Merge ~hloeung/content-cache-charm:sites-persistent-ports into content-cache-charm:master

[Haw Loeung]

Don't shuffling sites and port-pair mappings - LP:1865945

[Haw Loeung (hloeung)]

Rejected in favor of keeping the mappings in unitdata.kv(). See:

https://code.launchpad.net/~hloeung/content-cache-charm/+git/content-cache-charm/+merge/380871

Unmerged commits

e0fa10b... by Haw Loeung on 2020-03-18

WIP: Don't shuffling sites and port-pair mappings - LP:1865945

3b55472... by Haw Loeung on 2020-03-18

Fixed based on reviews

ff8557e... by Haw Loeung on 2020-03-18

Add map_sites_ports for persistent ports - LP:1865945

map_sites_ports() to parse the on disk HAProxy config and build a list
of site to port-pair mappings.

diff --git a/reactive/content_cache.py b/reactive/content_cache.py
index 450c63f..4311d78 100644
--- a/reactive/content_cache.py
+++ b/reactive/content_cache.py
@@ -506,18 +506,29 @@ def sites_from_config(sites_yaml, sites_secrets=None, blacklist_ports=None):
     cache_port = 0
     backend_port = 0
     new_sites = {}
+    existing_site_map = HAProxy.HAProxyConf().map_sites_ports()
+    if not blacklist_ports:
+        blacklist_ports = []
+    blacklist_ports += list(existing_site_map['cache'].values())
+    blacklist_ports += list(existing_site_map['backend'].values())
     for site, site_conf in sites.items():
         if not site_conf:
             continue
-        (cache_port, unused_backend_port) = utils.next_port_pair(
-            cache_port, backend_port, blacklist_ports=blacklist_ports
-        )
+        if site in existing_site_map['cache']:
+            cache_port = existing_site_map['cache'][site]
+        else:
+            (cache_port, unused_backend_port) = utils.next_port_pair(
+                cache_port, backend_port, blacklist_ports=blacklist_ports
+            )
         site_conf['cache_port'] = cache_port
         for location, loc_conf in site_conf.get('locations', {}).items():
             if loc_conf and loc_conf.get('backends'):
-                (unused_cache_port, backend_port) = utils.next_port_pair(
-                    cache_port, backend_port, blacklist_ports=blacklist_ports
-                )
+                if site in existing_site_map['backend']:
+                    backend_port = existing_site_map['backend'][site]
+                else:
+                    (unused_cache_port, backend_port) = utils.next_port_pair(
+                        cache_port, backend_port, blacklist_ports=blacklist_ports
+                    )
                 loc_conf['backend_port'] = backend_port
         new_sites[site] = site_conf
     return new_sites
diff --git a/tests/unit/files/config_test_sites_map.txt b/tests/unit/files/config_test_sites_map.txt
new file mode 100644
index 0000000..7158e24
--- /dev/null
+++ b/tests/unit/files/config_test_sites_map.txt
@@ -0,0 +1,127 @@
+site1.local:
+    cache_port: 6080
+    locations:
+        /:
+            backend_port: 8080
+            backends:
+            - 127.0.1.10:80
+            - 127.0.1.11:80
+            - 127.0.1.12:80
+            origin-headers:
+            -   X-Origin-Key: Sae6oob2aethuosh
+            -   X-Some-Header-1: something one two three
+            -   X-Some-Header-2: something:one:two:three
+            signed-url-hmac-key: SrRorTsImr92B6FfSKBFrSIiR5NYGS+gdjd8oGoVH44=
+    port: 80
+site2.local:
+    cache_port: 6081
+    locations:
+        /:
+            backend-check-method: GET
+            backend-check-path: /check/
+            backend-maxconn: 1024
+            backend-tls: true
+            backend_port: 8081
+            backends:
+            - 127.0.1.10:443
+            - 127.0.1.11:443
+            - 127.0.1.12:443
+        /my-local-content/:
+            extra-config:
+            - root /var/www/html
+        /my-local-content2/:
+            extra-config:
+            - root /var/www/html
+    redirect-http-to-https: true
+    tls-cert-bundle-path: /etc/haproxy/site2-bundle.crt
+site3.local:
+    cache_port: 6082
+    locations:
+        /:
+            backend-options:
+            - forwardfor except 127.0.0.1
+            - forceclose
+            backend_port: 8082
+            backends:
+            - 127.0.1.10:80
+            - 127.0.1.11:80
+            - 127.0.1.12:80
+            cache-maxconn: 4096
+site4.local:
+    cache_port: 6083
+    locations:
+        /:
+            extra-config:
+            - autoindex on
+        /ubuntu/pool/:
+            extra-config:
+            - autoindex on
+            - auth_request /auth
+site5:
+    cache_port: 6084
+    locations:
+        /:
+            backend_port: 8083
+            backends:
+            - 127.0.1.10:80
+        /auth:
+            backend-path: /auth-check/
+            backend_port: 8084
+            backends:
+            - 127.0.1.11:80
+            cache-background-update: None
+            cache-min-uses: None
+            cache-validity: 200 401 1h
+            modifier: '='
+    site-name: site5.local
+site6.local:
+    cache_port: 6085
+    locations:
+        /:
+            backend-tls: true
+            backend_port: 8085
+            backends:
+            - 127.0.1.10:443
+site7.local:
+    cache_port: 6086
+    locations:
+        /:
+            backend-tls: false
+            backend_port: 8086
+            backends:
+            - 127.0.1.10:80
+    port: 444
+    tls-cert-bundle-path: /etc/haproxy/site7-bundle.crt
+site8.local:
+    cache_port: 6087
+    locations:
+        /:
+            backend-tls: false
+            backend_port: 8087
+            backends:
+            - 127.0.1.10:80
+        /auth:
+            backend-tls: true
+            backend_port: 8088
+            backends:
+            - 127.0.1.10:443
+            site-name: auth.site8.local
+    port: 444
+    tls-cert-bundle-path: /etc/haproxy/site8-bundle.crt
+site9.local:
+    cache_port: 6088
+    locations:
+        /:
+            backend-inter-time: 1m
+            backend-tls: false
+            backend_port: 8089
+            backends:
+            - 127.0.1.15:80
+            extra-config:
+            - proxy_force_ranges off
+        /private/content:
+            extra-config:
+            - root /srv/example1.com/content/
+            - autoindex on
+            - auth_request /auth
+            nagios-expect: 401 Unauthorized
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
index 4cbbf05..f506063 100644
--- a/tests/unit/test_content_cache.py
+++ b/tests/unit/test_content_cache.py
@@ -7,6 +7,7 @@ from unittest import mock
 
 import freezegun
 import jinja2
+import yaml
 
 # We also need to mock up charms.layer so we can run unit tests without having
 # to build the charm and pull in layers such as layer-status.
@@ -774,6 +775,48 @@ site5.local:
         }
         self.assertEqual(want, content_cache.sites_from_config(config_yaml))
 
+    def test_sites_from_config_no_reshuffling(self):
+        haproxy_cfg = os.path.join(self.tmpdir, 'haproxy.cfg')
+        with mock.patch('lib.haproxy.HAProxyConf.conf_file', new_callable=mock.PropertyMock) as mock_conf_file:
+            mock_conf_file.return_value = haproxy_cfg
+            shutil.copyfile('tests/unit/files/content_cache_rendered_haproxy_test_output.txt', haproxy_cfg)
+
+            with open('tests/unit/files/config_test_sites_map.txt', 'r', encoding='utf-8') as f:
+                config_yaml = f.read()
+            want = yaml.safe_load(config_yaml)
+            self.assertEqual(want, content_cache.sites_from_config(config_yaml))
+
+            sites = yaml.safe_load(config_yaml)
+            sites_list = list(sites.keys())
+
+            # Remove all except second and last site for testing. Check to
+            # make sure it's correct and ports aren't reshuffled.
+            new = {}
+            new[sites_list[1]] = sites[sites_list[1]]
+            new[sites_list[len(sites_list) - 1]] = sites[sites_list[len(sites_list) - 1]]
+            want = new
+            config_yaml = yaml.safe_dump(new, indent=4, default_flow_style=False)
+            self.assertEqual(want, content_cache.sites_from_config(config_yaml))
+
+            # Add two sites back and make sure the existing two aren't reshuffled.
+            new = {}
+            new[sites_list[1]] = sites[sites_list[1]]
+            new[sites_list[len(sites_list) - 1]] = sites[sites_list[len(sites_list) - 1]]
+            new[sites_list[0]] = sites[sites_list[0]]
+            new[sites_list[2]] = sites[sites_list[2]]
+            want = new
+            config_yaml = yaml.safe_dump(new, indent=4, default_flow_style=False)
+            self.assertEqual(want, content_cache.sites_from_config(config_yaml))
+
+            # Add new site somewhere in the middle
+            new = {}
+            new[sites_list[1]] = sites[sites_list[1]]
+            new[sites_list[len(sites_list) - 1]] = sites[sites_list[len(sites_list) - 1]]
+            new[sites_list[0]] = sites[sites_list[0]]
+            new[sites_list[2]] = sites[sites_list[2]]
+            new['site11'] = {'locations': {'/': {'backend-tls': True, 'backends': ['127.0.1.10:443']}}}
+            self.assertEqual(want, content_cache.sites_from_config(config_yaml))
+
     def test_sites_from_config_blacklist_ports(self):
         blacklist_ports = [6080, 8080]
         config_yaml = '''