Merge pull request juju#57 from manadart/3.3-into-main

Merge 3.3 into main

run_tests

@@ -11,6 +11,6 @@ if [ -n "$PYTHONPATH" ]; then
 fi
 export PYTHONPATH="src:lib$PYTHONPATH"
 
-flake8
 coverage run --source=src -m unittest -v "$@"
 coverage report -m
+flake8

src/charm.py

@@ -5,14 +5,16 @@
 import controlsocket
 import logging
 import secrets
+import urllib.parse
 import yaml
 
 from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider
 from ops.charm import CharmBase, CollectStatusEvent
 from ops.framework import StoredState
 from ops.charm import RelationJoinedEvent, RelationDepartedEvent
 from ops.main import main
-from ops.model import ActiveStatus, BlockedStatus, Relation
+from ops.model import ActiveStatus, BlockedStatus, ErrorStatus, Relation
+from typing import List
 
 logger = logging.getLogger(__name__)
 
@@ -36,7 +38,7 @@ def __init__(self, *args):
             self.on.dbcluster_relation_changed, self._on_dbcluster_relation_changed)
 
         self.control_socket = controlsocket.Client(
-            socket_path="/var/lib/juju/control.socket")
+            socket_path='/var/lib/juju/control.socket')
         self.framework.observe(
             self.on.metrics_endpoint_relation_created, self._on_metrics_endpoint_relation_created)
         self.framework.observe(
@@ -47,9 +49,11 @@ def _on_collect_status(self, event: CollectStatusEvent):
             event.add_status(BlockedStatus(
                 'multiple possible DB bind addresses; set a suitable dbcluster network binding'))
 
-        if self.api_port() is None:
-            event.add_status(BlockedStatus(
-                'charm does not appear to be running on a controller node'))
+        try:
+            self.api_port()
+        except AgentConfException as e:
+            event.add_status(ErrorStatus(
+                f"cannot read controller API port from agent configuration: {e}"))
 
         event.add_status(ActiveStatus())
 
@@ -71,9 +75,11 @@ def _on_dashboard_relation_joined(self, event):
     def _on_website_relation_joined(self, event):
         """Connect a website relation."""
         logger.info('got a new website relation: %r', event)
-        port = self.api_port()
-        if port is None:
-            logger.error('charm does not appear to be running on a controller node')
+
+        try:
+            api_port = self.api_port()
+        except AgentConfException as e:
+            logger.error("cannot read controller API port from agent configuration: {}", e)
             return
 
         address = None
@@ -84,7 +90,7 @@ def _on_website_relation_joined(self, event):
                 event.relation.data[self.unit].update({
                     'hostname': str(address),
                     'private-address': str(address),
-                    'port': str(port)
+                    'port': str(api_port)
                 })
 
     def _on_metrics_endpoint_relation_created(self, event: RelationJoinedEvent):
@@ -93,14 +99,20 @@ def _on_metrics_endpoint_relation_created(self, event: RelationJoinedEvent):
         self.control_socket.add_metrics_user(username, password)
 
         # Set up Prometheus scrape config
+        try:
+            api_port = self.api_port()
+        except AgentConfException as e:
+            logger.error("cannot read controller API port from agent configuration: {}", e)
+            return
+
         metrics_endpoint = MetricsEndpointProvider(
             self,
             jobs=[{
                 "metrics_path": "/introspection/metrics",
                 "scheme": "https",
                 "static_configs": [{
                     "targets": [
-                        f'*:{self.api_port()}'
+                        f'*:{api_port}'
                     ]
                 }],
                 "basic_auth": {
@@ -135,6 +147,23 @@ def _on_dbcluster_relation_changed(self, event):
         self._stored.db_bind_address = ip
         event.relation.data[self.unit].update({'db-bind-address': ip})
 
+    def api_port(self) -> str:
+        """Return the port on which the controller API server is listening."""
+        api_addresses = self._agent_conf('apiaddresses')
+        if not api_addresses:
+            raise AgentConfException("agent.conf key 'apiaddresses' missing")
+        if not isinstance(api_addresses, List):
+            raise AgentConfException("agent.conf key 'apiaddresses' is not a list")
+
+        parsed_url = urllib.parse.urlsplit('//' + api_addresses[0])
+        if not parsed_url.port:
+            raise AgentConfException("API address does not include port")
+        return parsed_url.port
+
+    def ca_cert(self) -> str:
+        """Return the controller's CA certificate."""
+        return self._agent_conf('cacert')
+
     def _agent_conf(self, key: str):
         """Read a value (by key) from the agent.conf file on disk."""
         unit_name = self.unit.name.replace('/', '-')
@@ -144,14 +173,6 @@ def _agent_conf(self, key: str):
             agent_conf = yaml.safe_load(agent_conf_file)
             return agent_conf.get(key)
 
-    def api_port(self) -> str:
-        """Return the port on which the controller API server is listening."""
-        return self._agent_conf('apiport')
-
-    def ca_cert(self) -> str:
-        """Return the controller's CA certificate."""
-        return self._agent_conf('cacert')
-
 
 def metrics_username(relation: Relation) -> str:
     """
@@ -166,5 +187,9 @@ def generate_password() -> str:
     return secrets.token_urlsafe(16)
 
 
+class AgentConfException(Exception):
+    """Raised when there are errors reading info from agent.conf."""
+
+
 if __name__ == "__main__":
     main(JujuControllerCharm)

tests/test_charm.py

@@ -3,13 +3,36 @@
 
 import os
 import unittest
-from charm import JujuControllerCharm
-from ops.model import BlockedStatus, ActiveStatus
+from charm import JujuControllerCharm, AgentConfException
+from ops.model import BlockedStatus, ActiveStatus, ErrorStatus
 from ops.testing import Harness
 from unittest.mock import mock_open, patch
 
 agent_conf = '''
-apiport: 17070
+apiaddresses:
+- localhost:17070
+cacert: fake
+'''
+
+agent_conf_apiaddresses_missing = '''
+cacert: fake
+'''
+
+agent_conf_apiaddresses_not_list = '''
+apiaddresses:
+  foo: bar
+cacert: fake
+'''
+
+agent_conf_ipv4 = '''
+apiaddresses:
+- "127.0.0.1:17070"
+cacert: fake
+'''
+
+agent_conf_ipv6 = '''
+apiaddresses:
+- "[::1]:17070"
 cacert: fake
 '''
 
@@ -88,12 +111,58 @@ def test_metrics_endpoint_relation(self, mock_remove_user, mock_add_user,
         harness.remove_relation(relation_id)
         mock_remove_user.assert_called_once_with(f'juju-metrics-r{relation_id}')
 
+    @patch("builtins.open", new_callable=mock_open, read_data=agent_conf_apiaddresses_missing)
+    def test_apiaddresses_missing(self, _):
+        harness = Harness(JujuControllerCharm)
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        with self.assertRaisesRegex(AgentConfException, "agent.conf key 'apiaddresses' missing"):
+            harness.charm.api_port()
+
+    @patch("builtins.open", new_callable=mock_open, read_data=agent_conf_apiaddresses_not_list)
+    def test_apiaddresses_not_list(self, _):
+        harness = Harness(JujuControllerCharm)
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        with self.assertRaisesRegex(
+            AgentConfException, "agent.conf key 'apiaddresses' is not a list"
+        ):
+            harness.charm.api_port()
+
+    @patch("builtins.open", new_callable=mock_open, read_data=agent_conf_apiaddresses_missing)
+    @patch("controlsocket.Client.add_metrics_user")
+    def test_apiaddresses_missing_status(self, *_):
+        harness = Harness(JujuControllerCharm)
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        harness.add_relation('metrics-endpoint', 'prometheus-k8s')
+        harness.evaluate_status()
+        self.assertIsInstance(harness.charm.unit.status, ErrorStatus)
+
+    @patch("builtins.open", new_callable=mock_open, read_data=agent_conf_ipv4)
+    def test_apiaddresses_ipv4(self, _):
+        harness = Harness(JujuControllerCharm)
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        self.assertEqual(harness.charm.api_port(), 17070)
+
+    @patch("builtins.open", new_callable=mock_open, read_data=agent_conf_ipv6)
+    def test_apiaddresses_ipv6(self, _):
+        harness = Harness(JujuControllerCharm)
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        self.assertEqual(harness.charm.api_port(), 17070)
+
     @patch("builtins.open", new_callable=mock_open, read_data=agent_conf)
     @patch("ops.model.Model.get_binding")
     def test_dbcluster_relation_changed_single_addr(self, binding, _):
         harness = self.harness
         binding.return_value = mockBinding(["192.168.1.17"])
-
         relation_id = harness.add_relation('dbcluster', 'controller')
         harness.add_relation_unit(relation_id, 'juju-controller/1')