Merge ~hloeung/content-cache-charm:cleanup into content-cache-charm:master

[Haw Loeung]

Renamed config option to make it consistent - LP:1871259

This addresses review comments from https://code.launchpad.net/~hloeung/content-cache-charm/+git/content-cache-charm/+merge/388277

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Paul Collins (pjdc)]

review: Approve (lgtm)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 5152c2c47b7cf56ef26e37c8f9f88956a141a741

diff --git a/config.yaml b/config.yaml
index 0a81a0d..12560a6 100644
--- a/config.yaml
+++ b/config.yaml
@@ -25,14 +25,14 @@ options:
     default: "/var/lib/nginx/proxy"
     description: >
       Path or directory to store cached objects.
-  disable_cache_background_update:
-    default: false
+  enable_cache_background_update:
+    default: true
     type: boolean
     description: >
       Default is to enable serving contents from the cache with a
       background subrequest to update objects in cache. This works for
       large objects, but can cause issues with sites updated recently
-      to redirects that still have objects in the cache.
+      to redirect and still have objects in the cache.
   enable_prometheus_metrics:
     default: true
     type: boolean
diff --git a/lib/nginx.py b/lib/nginx.py
index 74f3e58..1b6289e 100644
--- a/lib/nginx.py
+++ b/lib/nginx.py
@@ -24,14 +24,14 @@ PROXY_CACHE_DEFAULTS = {
 
 
 class NginxConf:
-    def __init__(self, conf_path=None, unit='content-cache', disable_cache_bg_update=False):
+    def __init__(self, conf_path=None, unit='content-cache', enable_cache_bg_update=True):
         if not conf_path:
             conf_path = NGINX_BASE_PATH
         self.unit = unit
 
         self._base_path = conf_path
         self._conf_path = os.path.join(self.base_path, 'conf.d')
-        self._disable_cache_bg_update = disable_cache_bg_update
+        self._enable_cache_bg_update = enable_cache_bg_update
         self._sites_path = os.path.join(self.base_path, 'sites-available')
 
         script_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
@@ -113,7 +113,7 @@ class NginxConf:
                         continue
                     lc.setdefault(cache_key, v)
 
-                if self._disable_cache_bg_update:
+                if not self._enable_cache_bg_update:
                     lc['cache-background-update'] = 'off'
                     lc['cache-use-stale'] = lc['cache-use-stale'].replace('updating ', '')
 
diff --git a/reactive/content_cache.py b/reactive/content_cache.py
index 6c6eeef..e5967a8 100644
--- a/reactive/content_cache.py
+++ b/reactive/content_cache.py
@@ -138,10 +138,11 @@ def configure_nginx(conf_path=None):
         status.blocked('requires list of sites to configure')
         return
 
+    enable_cache_bg_update = config.get('enable_cache_background_update', True)
     enable_prometheus_metrics = config.get('enable_prometheus_metrics')
 
-    disable_cache_bg_update = config.get('disable_cache_background_update', False)
-    ngx_conf = nginx.NginxConf(conf_path, hookenv.local_unit(), disable_cache_bg_update=disable_cache_bg_update)
+    ngx_conf = nginx.NginxConf(conf_path, hookenv.local_unit(), enable_cache_bg_update=enable_cache_bg_update)
+
     sites_secrets = secrets_from_config(config.get('sites_secrets'))
     blacklist_ports = [int(x.strip()) for x in config.get('blacklist_ports', '').split(',') if x.strip()]
     sites = sites_from_config(config.get('sites'), sites_secrets, blacklist_ports=blacklist_ports)
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
index ad9964f..5b4e386 100644
--- a/tests/unit/test_content_cache.py
+++ b/tests/unit/test_content_cache.py
@@ -192,7 +192,7 @@ class TestCharm(unittest.TestCase):
             'cache_inactive_time': '2h',
             'cache_max_size': '1g',
             'cache_path': '/var/lib/nginx/proxy',
-            'disable_cache_background_update': False,
+            'enable_cache_background_update': True,
             'enable_prometheus_metrics': False,
             'sites': ngx_config,
             'worker_connections': 768,
@@ -1155,7 +1155,7 @@ site1.local:
             'cache_inactive_time': '2h',
             'cache_max_size': '1g',
             'cache_path': '/var/lib/nginx/proxy',
-            'disable_cache_background_update': True,
+            'enable_cache_background_update': False,
             'enable_prometheus_metrics': True,
             'sites': ngx_config,
             'worker_connections': 768,
diff --git a/tests/unit/test_nginx.py b/tests/unit/test_nginx.py
index 7f95ba8..c7bcfe3 100644
--- a/tests/unit/test_nginx.py
+++ b/tests/unit/test_nginx.py
@@ -124,7 +124,7 @@ class TestLibNginx(unittest.TestCase):
 
     def test_nginx_config_render_with_metrics(self):
         """Test rendering with metrics exposed."""
-        ngx_conf = nginx.NginxConf(unit='mock-content-cache/0', disable_cache_bg_update=True)
+        ngx_conf = nginx.NginxConf(unit='mock-content-cache/0', enable_cache_bg_update=False)
 
         with open('tests/unit/files/config_test_basic_config.txt', 'r', encoding='utf-8') as f:
             sites = yaml.safe_load(f.read())
@@ -133,7 +133,7 @@ class TestLibNginx(unittest.TestCase):
             'cache_inactive_time': '2h',
             'cache_max_size': '1g',
             'cache_path': '/var/lib/nginx/proxy',
-            'disable_cache_background_update': True,
+            'enable_cache_background_update': False,
             'enable_prometheus_metrics': True,
             'listen_address': '127.0.0.1',
         }