Merge ~hloeung/content-cache-charm:nginx-config into content-cache-charm:master

[Haw Loeung]

Allow overriding Nginx' proxy_cache_lock

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

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

[Barry Price (barryprice)]

Sure

review: Approve

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

Change successfully merged at revision 6df560833781f16a441a0b42aa0e298671a24a33

diff --git a/config.yaml b/config.yaml
index e634fe2..a7cd0e9 100644
--- a/config.yaml
+++ b/config.yaml
@@ -33,6 +33,13 @@ options:
       background subrequest to update objects in cache. This works for
       large objects, but can cause issues with sites updated recently
       to redirect and still have objects in the cache.
+  enable_cache_lock:
+    default: true
+    type: boolean
+    description: >
+      Default is to restrict to only one request at a time to populate
+      new cache elements from the backends. See Nginx'
+      proxy_cache_lock.
   enable_prometheus_metrics:
     default: true
     type: boolean
diff --git a/lib/nginx.py b/lib/nginx.py
index 1b6289e..d880d72 100644
--- a/lib/nginx.py
+++ b/lib/nginx.py
@@ -24,7 +24,7 @@ PROXY_CACHE_DEFAULTS = {
 
 
 class NginxConf:
-    def __init__(self, conf_path=None, unit='content-cache', enable_cache_bg_update=True):
+    def __init__(self, conf_path=None, unit='content-cache', enable_cache_bg_update=True, enable_cache_lock=True):
         if not conf_path:
             conf_path = NGINX_BASE_PATH
         self.unit = unit
@@ -32,6 +32,7 @@ class NginxConf:
         self._base_path = conf_path
         self._conf_path = os.path.join(self.base_path, 'conf.d')
         self._enable_cache_bg_update = enable_cache_bg_update
+        self._enable_cache_lock = enable_cache_lock
         self._sites_path = os.path.join(self.base_path, 'sites-available')
 
         script_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
@@ -117,6 +118,9 @@ class NginxConf:
                     lc['cache-background-update'] = 'off'
                     lc['cache-use-stale'] = lc['cache-use-stale'].replace('updating ', '')
 
+                if not self._enable_cache_lock:
+                    lc['cache-lock'] = 'off'
+
                 cache_val = self.proxy_cache_configs['valid']
                 # Backwards compatibility
                 if 'cache-validity' in lc:
diff --git a/reactive/content_cache.py b/reactive/content_cache.py
index 8003f24..ddd0bfb 100644
--- a/reactive/content_cache.py
+++ b/reactive/content_cache.py
@@ -139,9 +139,15 @@ def configure_nginx(conf_path=None):
         return
 
     enable_cache_bg_update = config.get('enable_cache_background_update', True)
+    enable_cache_lock = config.get('enable_cache_lock', True)
     enable_prometheus_metrics = config.get('enable_prometheus_metrics')
 
-    ngx_conf = nginx.NginxConf(conf_path, hookenv.local_unit(), enable_cache_bg_update=enable_cache_bg_update)
+    ngx_conf = nginx.NginxConf(
+        conf_path,
+        hookenv.local_unit(),
+        enable_cache_bg_update=enable_cache_bg_update,
+        enable_cache_lock=enable_cache_lock,
+    )
 
     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()]
diff --git a/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
index 98a3042..1e97ac9 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-basic_site.txt
@@ -21,7 +21,7 @@ server {
         proxy_force_ranges on;
         proxy_cache e176ab9d1f16-cache;
         proxy_cache_background_update off;
-        proxy_cache_lock on;
+        proxy_cache_lock off;
         proxy_cache_min_uses 1;
         proxy_cache_revalidate on;
         proxy_cache_use_stale error timeout http_500 http_502 http_503 http_504;
diff --git a/tests/unit/files/nginx_config_rendered_test_output-token_site.txt b/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
index 21638c3..c265d8a 100644
--- a/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
+++ b/tests/unit/files/nginx_config_rendered_test_output-token_site.txt
@@ -21,7 +21,7 @@ server {
         proxy_force_ranges on;
         proxy_cache c27ba6753fee-cache;
         proxy_cache_background_update off;
-        proxy_cache_lock on;
+        proxy_cache_lock off;
         proxy_cache_min_uses 1;
         proxy_cache_revalidate on;
         proxy_cache_use_stale error timeout http_500 http_502 http_503 http_504;
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
index 0d7e39f..6f52f08 100644
--- a/tests/unit/test_content_cache.py
+++ b/tests/unit/test_content_cache.py
@@ -203,6 +203,7 @@ class TestCharm(unittest.TestCase):
             'cache_max_size': '1g',
             'cache_path': '/var/lib/nginx/proxy',
             'enable_cache_background_update': True,
+            'enable_cache_lock': True,
             'enable_prometheus_metrics': False,
             'sites': ngx_config,
             'worker_connections': 768,
@@ -1287,6 +1288,7 @@ site1.local:
             'cache_max_size': '1g',
             'cache_path': '/var/lib/nginx/proxy',
             'enable_cache_background_update': False,
+            'enable_cache_lock': 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 5ec7fbe..b76c674 100644
--- a/tests/unit/test_nginx.py
+++ b/tests/unit/test_nginx.py
@@ -126,7 +126,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', enable_cache_bg_update=False)
+        ngx_conf = nginx.NginxConf(unit='mock-content-cache/0', enable_cache_bg_update=False, enable_cache_lock=False)
 
         with open('tests/unit/files/config_test_basic_config.txt', 'r', encoding='utf-8') as f:
             sites = yaml.safe_load(f.read())
@@ -137,7 +137,6 @@ class TestLibNginx(unittest.TestCase):
             'cache_inactive_time': '2h',
             'cache_max_size': '1g',
             'cache_path': '/var/lib/nginx/proxy',
-            'enable_cache_background_update': False,
             'enable_prometheus_metrics': True,
             'listen_address': '127.0.0.1',
         }