Merge ~moon127/charm-k8s-unifi-poller/+git/charm-k8s-unifi-poller:multi-controller-support into charm-k8s-unifi-poller:master

[Gareth Woolridge]

Add support for multiple controllers and other options/overrides supported by the upstream unifi-poller via extra_configs. Add tests and lint fixes.

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

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

[Tom Haddon (mthaddon)]

Discussed IRL the question of up_ - there are other valid values we'd want to pass in besides up_unifi_controller_$blah (e.g. up_influxdb_interval), so checking for up_ makes sense. Agreed to switch to urlparse to avoid hard to read regex for the URLs though.

[Tom Haddon (mthaddon)]

I'd have a small preference for `url.scheme not in ("http", "https")` over `url.scheme != ("https" or "http")` but other than that looks good, and I think this is ready for charmcrafters review.

[John Lenton (chipaca)]

FWIW,

url.scheme != ("https" or "http")

is *not* the same as

url.scheme not in ("http", "https")

The former is True unless url.scheme is "https", because ("https" or "http") evaluates to "https", and then you're comparing url.scheme to that. You _probably_ were thinking

url.scheme != "https" and url.scheme != "http"

at which point it becomes a matter of personal preference; I find the 'not in' variant much easier to read, and a lot easier to expand if needed.

[John Lenton (chipaca)]

review: Approve

[Tom Haddon (mthaddon)]

Lgtm, thx

review: Approve

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

Change successfully merged at revision c4cd11792f372ee0a72379ee19c692ebb08092f8

diff --git a/README.md b/README.md
index a7792f3..e42a63b 100644
--- a/README.md
+++ b/README.md
@@ -65,12 +65,8 @@ The charm optionally supports an externally deployed InfluxDB instance which may
     juju config unifi-poller influxdb_username=unifipoller
     juju config unifi=poller influxdb_password=unifipoller
 
-### Scale Out Usage
+### Additional Controllers / Config Options
 
-Currently the Unifi Poller charm supports only a single Unifi Network Controller per instance.
+The Unifi Poller charm supports configuring additional controllers and additional options,  which can be added via the extra_configs charm option.
 
-For now to poll multiple Unifi Network Controllers we would deploy multiple instances of unifi-poller charm with unique application names, and configure the controller_* config options on each for a specific controller:
-
-    juju deploy unifi-poller.charm --config image_path=localhost:32000/unifi-poller:focal-latest unifi-poller-controller1
-    juju deploy unifi-poller.charm --config image_path=localhost:32000/unifi-poller:focal-latest unifi-poller-controller2
-    ...
+See config.yaml and the [full list of supported env vars](https://github.com/unifi-poller/unifi-poller/wiki/Configuration#config-file-and-environment-variables) for details.
diff --git a/config.yaml b/config.yaml
index 6ad6366..e17157f 100644
--- a/config.yaml
+++ b/config.yaml
@@ -59,3 +59,26 @@ options:
         description: |
           The password associated with influxdb_user for access the InfluxDB specified in influxdb_url.
         default: ''
+    extra_configs:
+        type: string
+        description: |
+          Additional config options, as yaml, to configure additional options and controllers.
+          Full list of options from https://github.com/unifi-poller/unifi-poller/wiki/Configuration#config-file-and-environment-variables
+
+          The controller_0 URL, user and password are set via the main charm options, although additional options can be set here.
+          To add additional controllers start from controller_1.
+
+          Example 1:
+          - up_unifi_controller_1_url=https://<server_ip_1>:8443
+          - up_unifi_controller_1_user=unifipoller
+          - up_unifi_controller_1_pass=unifipoller
+          - up_unifi_controller_2_url=https://<server_ip_2>:8443
+          - up_unifi_controller_2_user=unifipoller
+          - up_unifi_controller_2_pass=unifipoller
+          - up_influxdb_interval=120s
+
+          Example 2:
+          ['up_unifi_controller_1_url=https://<server_1_ip>:8443', up_unifi_controller_1_user=unifipoller, \
+          up_unifi_controller_1_pass=unifipoller, 'up_unifi_controller_2_url=<server_2_ip>', \
+          up_unifi_controller_2_user=unifipoller, up_unifi_controller_2_pass=unifipoller, up_influxdb_interval=120s]
+        default: ''
diff --git a/src/charm.py b/src/charm.py
index 61d25aa..20a530a 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -3,6 +3,9 @@
 # See LICENSE file for licensing details.
 
 import logging
+import re
+from urllib.parse import urlparse
+import yaml
 
 from ops.charm import CharmBase
 from ops.main import main
@@ -43,6 +46,10 @@ class UnifiPollerCharm(CharmBase):
         if missing:
             problems.append('required setting(s) missing: {}'.format(', '.join(sorted(missing))))
 
+        extra_config_errors = self._bad_extra_config_charm_settings()
+        if extra_config_errors:
+            problems.append('invalid extra_config setting(s): {}'.format(', '.join(sorted(extra_config_errors))))
+
         return '; '.join(filter(None, problems))
 
     def _missing_charm_settings(self):
@@ -60,6 +67,23 @@ class UnifiPollerCharm(CharmBase):
 
         return sorted(missing)
 
+    def _bad_extra_config_charm_settings(self):
+        """Check extra_config configuration_settings."""
+        config = self.model.config
+
+        errors = []
+        if config['extra_configs']:
+            for c in yaml.safe_load(config['extra_configs']):
+                k, value = c.split('=', 1)
+                if not re.match(r'^UP_', k.upper()):
+                    errors.append(k)
+                if re.match(r'.*_URL', k.upper()):
+                    url = urlparse(value)
+                    if url.scheme not in ("http", "https") or not url.netloc:
+                        errors.append(k)
+
+        return sorted(errors)
+
     def _make_pod_spec(self):
         """Return a pod spec with some core configuration."""
         config = self.model.config
@@ -78,7 +102,9 @@ class UnifiPollerCharm(CharmBase):
                     'imageDetails': image_details,
                     'ports': [{'containerPort': METRICS_PORT, 'protocol': 'TCP'}],
                     'envConfig': pod_config,
-                    'kubernetes': {'readinessProbe': {'httpGet': {'path': '/check', 'port': METRICS_PORT}},},
+                    'kubernetes': {
+                        'readinessProbe': {'httpGet': {'path': '/check', 'port': METRICS_PORT}},
+                    },
                 }
             ],
         }
@@ -86,19 +112,14 @@ class UnifiPollerCharm(CharmBase):
     def _make_pod_config(self):
         """Return an envConfig with some core configuration."""
         config = self.model.config
-        pod_config = {}
-
-        if config['controller_url']:
-            pod_config['UP_UNIFI_DEFAULT_URL'] = config['controller_url']
-
-        if config['controller_username']:
-            pod_config['UP_UNIFI_DEFAULT_USER'] = config['controller_username']
-
-        if config['controller_password']:
-            pod_config['UP_UNIFI_DEFAULT_PASS'] = config['controller_password']
+        pod_config = {
+            'UP_UNIFI_CONTROLLER_0_URL': config['controller_url'],
+            'UP_UNIFI_CONTROLLER_0_USER': config['controller_username'],
+            'UP_UNIFI_CONTROLLER_0_PASS': config['controller_password'],
+        }
 
         if config['controller_dpi']:
-            pod_config['UP_UNIFI_DEFAULT_SAVE_DPI'] = config['controller_dpi']
+            pod_config['UP_UNIFI_CONTROLLER_0_SAVE_DPI'] = config['controller_dpi']
 
         if config['influxdb_url']:
             pod_config['UP_INFLUXDB_URL'] = config['influxdb_url']
@@ -112,6 +133,11 @@ class UnifiPollerCharm(CharmBase):
         if config['influxdb_password']:
             pod_config['UP_INFLUXDB_PASS'] = config['influxdb_password']
 
+        if config['extra_configs']:
+            for c in yaml.safe_load(config['extra_configs']):
+                k, value = c.split('=', 1)
+                pod_config[k.upper()] = value
+
         return pod_config
 
     def _configure_pod(self, event):
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 98a0b83..d3e787a 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -10,56 +10,79 @@ CONFIG_ALL = {
     'image_path': 'example.com/unifi-poller:latest',
     'image_username': 'image_user',
     'image_password': 'image_pass',
-    'controller_url': 'https://192.168.88.1:8443/',
+    'controller_url': 'https://192.168.88.1:8443',
     'controller_username': 'unifipoller',
     'controller_password': 'unifipoller',
     'controller_dpi': 'true',
-    'influxdb_url': 'http://192.168.88.2:8086/',
+    'influxdb_url': 'http://192.168.88.2:8086',
     'influxdb_dbname': 'unifi-k8s',
     'influxdb_username': 'unifipoller',
     'influxdb_password': 'unifipoller',
+    'extra_configs': "['up_unifi_controller_1_url=https://192.168.88.253:8443', up_unifi_controller_1_user=unifipoller, \
+                      up_unifi_controller_1_pass=unifipoller, 'up_unifi_controller_2_url=https://192.168.88.254:8443', \
+                      up_unifi_controller_2_user=unifipoller, up_unifi_controller_2_pass=unifipoller]",
 }
 
 CONFIG_NO_IMAGE = {
     'image_path': '',
     'image_username': '',
     'image_password': '',
-    'controller_url': 'https://192.168.88.1:8443/',
+    'controller_url': 'https://192.168.88.1:8443',
     'controller_username': 'unifipoller',
     'controller_password': 'unifipoller',
     'controller_dpi': 'true',
-    'influxdb_url': 'http://192.168.88.2:8086/',
+    'influxdb_url': 'http://192.168.88.2:8086',
     'influxdb_dbname': 'unifi-k8s',
     'influxdb_username': 'unifipoller',
     'influxdb_password': 'unifipoller',
+    'extra_configs': '',
 }
 
-CONFIG_IMAGE_NO_CREDS = {
+CONFIG_NO_CONTROLLER_CONFIG = {
     'image_path': 'example.com/unifi-poller:latest',
-    'image_username': '',
-    'image_password': '',
-    'controller_url': 'https://192.168.88.1:8443/',
+    'image_username': 'image_user',
+    'image_password': 'image_pass',
+    'controller_url': '',
+    'controller_username': '',
+    'controller_password': '',
+    'controller_dpi': '',
+    'influxdb_url': 'http://192.168.88.2:8086',
+    'influxdb_dbname': 'unifi-k8s',
+    'influxdb_username': 'unifipoller',
+    'influxdb_password': 'unifipoller',
+    'extra_configs': '',
+}
+
+CONFIG_INVALID_EXTRA_CONFIG = {
+    'image_path': 'example.com/unifi-poller:latest',
+    'image_username': 'image_user',
+    'image_password': 'image_pass',
+    'controller_url': 'https://192.168.88.1:8443',
     'controller_username': 'unifipoller',
     'controller_password': 'unifipoller',
     'controller_dpi': 'true',
-    'influxdb_url': 'http://192.168.88.2:8086/',
+    'influxdb_url': 'http://192.168.88.2:8086',
     'influxdb_dbname': 'unifi-k8s',
     'influxdb_username': 'unifipoller',
     'influxdb_password': 'unifipoller',
+    'extra_configs': "['up_unifi_controller_1_url=this:not-a-url', up_unifi_controller_1_user=unifipoller, \
+                      up_unifi_controller_1_pass=unifipoller, influx_interval=60s]",
 }
 
-CONFIG_NO_CONTROLLER_CONFIG = {
-    'image_path': 'example.com/unifi-poller:latest',
+CONFIG_INVALID_EXTRA_CONFIG_NO_IMAGE = {
+    'image_path': '',
     'image_username': 'image_user',
     'image_password': 'image_pass',
-    'controller_url': '',
-    'controller_username': '',
-    'controller_password': '',
-    'controller_dpi': '',
-    'influxdb_url': 'http://192.168.88.2:8086/',
+    'controller_url': 'https://192.168.88.1:8443',
+    'controller_username': 'unifipoller',
+    'controller_password': 'unifipoller',
+    'controller_dpi': 'true',
+    'influxdb_url': 'http://192.168.88.2:8086',
     'influxdb_dbname': 'unifi-k8s',
     'influxdb_username': 'unifipoller',
     'influxdb_password': 'unifipoller',
+    'extra_configs': "['up_unifi_controller_1_url=this:not-a-url', up_unifi_controller_1_user=unifipoller, \
+                      up_unifi_controller_1_pass=unifipoller, influx_interval=60s]",
 }
 
 
@@ -83,18 +106,37 @@ class TestCharm(unittest.TestCase):
         expected = 'required setting(s) missing: controller_password, controller_url, controller_username'
         self.assertEqual(self.harness.charm._check_for_config_problems(), expected)
 
+    def test_check_for_invalid_extra_config(self):
+        """Check for correctly reports invalid extra_configs options."""
+        self.harness.update_config(CONFIG_INVALID_EXTRA_CONFIG)
+        expected = 'invalid extra_config setting(s): influx_interval, up_unifi_controller_1_url'
+        self.assertEqual(self.harness.charm._check_for_config_problems(), expected)
+
+    def test_check_for_missing_image_invalid_extra_config(self):
+        """Check for correctly reports invalid extra_configs options."""
+        self.harness.update_config(CONFIG_INVALID_EXTRA_CONFIG_NO_IMAGE)
+        expected = 'required setting(s) missing: image_path; \
+invalid extra_config setting(s): influx_interval, up_unifi_controller_1_url'
+        self.assertEqual(self.harness.charm._check_for_config_problems(), expected)
+
     def test_make_pod_config(self):
         """Make basic, correct pod config."""
-        self.harness.update_config(CONFIG_IMAGE_NO_CREDS)
+        self.harness.update_config(CONFIG_ALL)
         expected = {
             'UP_INFLUXDB_DB': 'unifi-k8s',
             'UP_INFLUXDB_PASS': 'unifipoller',
-            'UP_INFLUXDB_URL': 'http://192.168.88.2:8086/',
+            'UP_INFLUXDB_URL': 'http://192.168.88.2:8086',
             'UP_INFLUXDB_USER': 'unifipoller',
-            'UP_UNIFI_DEFAULT_PASS': 'unifipoller',
-            'UP_UNIFI_DEFAULT_SAVE_DPI': 'true',
-            'UP_UNIFI_DEFAULT_URL': 'https://192.168.88.1:8443/',
-            'UP_UNIFI_DEFAULT_USER': 'unifipoller',
+            'UP_UNIFI_CONTROLLER_0_PASS': 'unifipoller',
+            'UP_UNIFI_CONTROLLER_0_SAVE_DPI': 'true',
+            'UP_UNIFI_CONTROLLER_0_URL': 'https://192.168.88.1:8443',
+            'UP_UNIFI_CONTROLLER_0_USER': 'unifipoller',
+            'UP_UNIFI_CONTROLLER_1_PASS': 'unifipoller',
+            'UP_UNIFI_CONTROLLER_1_URL': 'https://192.168.88.253:8443',
+            'UP_UNIFI_CONTROLLER_1_USER': 'unifipoller',
+            'UP_UNIFI_CONTROLLER_2_USER': 'unifipoller',
+            'UP_UNIFI_CONTROLLER_2_PASS': 'unifipoller',
+            'UP_UNIFI_CONTROLLER_2_URL': 'https://192.168.88.254:8443',
         }
         self.assertEqual(self.harness.charm._make_pod_config(), expected)
 
@@ -113,7 +155,9 @@ class TestCharm(unittest.TestCase):
                     },
                     'ports': [{'containerPort': 9130, 'protocol': 'TCP'}],
                     'envConfig': self.harness.charm._make_pod_config(),
-                    'kubernetes': {'readinessProbe': {'httpGet': {'path': '/check', 'port': 9130}},},
+                    'kubernetes': {
+                        'readinessProbe': {'httpGet': {'path': '/check', 'port': 9130}},
+                    },
                 }
             ],
         }