Uses the controller's configuration socket to query the agent's ID.

This is used to set configuration at the appropriate path.

src/charm.py

@@ -6,9 +6,7 @@
 import configchangesocket
 import json
 import logging
-import re
 import secrets
-import subprocess
 import urllib.parse
 import yaml
 
@@ -25,6 +23,8 @@
 
 
 class JujuControllerCharm(CharmBase):
+    METRICS_SOCKET_PATH = '/var/lib/juju/control.socket'
+    CONFIG_SOCKET_PATH = '/var/lib/juju/configchange.socket'
     DB_BIND_ADDR_KEY = 'db-bind-address'
     ALL_BIND_ADDRS_KEY = 'db-bind-addresses'
 
@@ -33,30 +33,36 @@ class JujuControllerCharm(CharmBase):
     def __init__(self, *args):
         super().__init__(*args)
 
-        self.framework.observe(self.on.install, self._on_install)
-        self.framework.observe(self.on.collect_unit_status, self._on_collect_status)
-        self.framework.observe(self.on.config_changed, self._on_config_changed)
-        self.framework.observe(
-            self.on.dashboard_relation_joined, self._on_dashboard_relation_joined)
-        self.framework.observe(
-            self.on.website_relation_joined, self._on_website_relation_joined)
+        self._observe()
 
         self._stored.set_default(
             db_bind_address='',
             last_bind_addresses=[],
             all_bind_addresses=dict(),
         )
-        self.framework.observe(
-            self.on.dbcluster_relation_changed, self._on_dbcluster_relation_changed)
 
-        self.control_socket = controlsocket.ControlSocketClient(
-            socket_path='/var/lib/juju/control.socket')
-        self.config_change_socket = configchangesocket.ConfigChangeSocketClient(
-            socket_path='/var/lib/juju/configchange.socket')
+        # TODO (manadart 2024-03-05): Get these at need.
+        # No need to instantiate them for every invocatoin.
+        self._control_socket = controlsocket.ControlSocketClient(
+            socket_path=self.METRICS_SOCKET_PATH)
+        self._config_change_socket = configchangesocket.ConfigChangeSocketClient(
+            socket_path=self.CONFIG_SOCKET_PATH)
+
+    def _observe(self):
+        """Set up all framework event observers."""
+        self.framework.observe(self.on.install, self._on_install)
+        self.framework.observe(self.on.collect_unit_status, self._on_collect_status)
+        self.framework.observe(self.on.config_changed, self._on_config_changed)
+        self.framework.observe(
+            self.on.dashboard_relation_joined, self._on_dashboard_relation_joined)
+        self.framework.observe(
+            self.on.website_relation_joined, self._on_website_relation_joined)
         self.framework.observe(
             self.on.metrics_endpoint_relation_created, self._on_metrics_endpoint_relation_created)
         self.framework.observe(
             self.on.metrics_endpoint_relation_broken, self._on_metrics_endpoint_relation_broken)
+        self.framework.observe(
+            self.on.dbcluster_relation_changed, self._on_dbcluster_relation_changed)
 
     def _on_install(self, event: InstallEvent):
         """Ensure that the controller configuration file exists."""
@@ -116,7 +122,7 @@ def _on_website_relation_joined(self, event):
     def _on_metrics_endpoint_relation_created(self, event: RelationJoinedEvent):
         username = metrics_username(event.relation)
         password = generate_password()
-        self.control_socket.add_metrics_user(username, password)
+        self._control_socket.add_metrics_user(username, password)
 
         # Set up Prometheus scrape config
         try:
@@ -149,7 +155,7 @@ def _on_metrics_endpoint_relation_created(self, event: RelationJoinedEvent):
 
     def _on_metrics_endpoint_relation_broken(self, event: RelationDepartedEvent):
         username = metrics_username(event.relation)
-        self.control_socket.remove_metrics_user(username)
+        self._control_socket.remove_metrics_user(username)
 
     def _on_dbcluster_relation_changed(self, event):
         """Maintain our own bind address in relation data.
@@ -255,35 +261,12 @@ def _controller_config_path(self) -> str:
         """Interrogate the running controller jujud service to determine
         the local controller ID, then use it to construct a config path.
         """
-
-        # The option to jujud is controller-id on K8s and machine-id on others.
-        command = self._get_controller_process()[1]
-        print(command)
-        match = re.search(r'(?:--controller-id|--machine-id)\s+(\d+)', command)
-        if not match:
-            raise ControllerProcessException('Unable to determine ID for running controller')
-
-        controller_id = match.group(1)
+        controller_id = self._config_change_socket.get_controller_agent_id()
         return f'/var/lib/juju/agents/controller-{controller_id}/agent.conf'
 
-    def _get_controller_process(self):
-        """Use pgrep to get the controller's jujud process ID and full command."""
-
-        # This wild card accommodates the different paths for jujud across
-        # K8s and machines. It is safe - we ensure one and only one match.
-        res = subprocess.check_output(['pgrep', '-af', '/var/lib/juju/tools.*jujud'], text=True)
-
-        lines = res.strip().split('\n')
-        if len(lines) != 1:
-            raise ControllerProcessException(
-                'Unable to determine process for running controller')
-
-        parts = lines[0].split(' ', 1)
-        return (int(parts[0]), parts[1])
-
     def _request_config_reload(self):
-        """Set reload request to the config reload socket"""
-        self.config_change_socket.reload_config()
+        """Send a reload request to the config reload socket"""
+        self._config_change_socket.reload_config()
 
 
 def metrics_username(relation: Relation) -> str:

src/configchangesocket.py

@@ -18,9 +18,9 @@ def __init__(self, socket_path: str,
                  opener: Optional[urllib.request.OpenerDirector] = None):
         super().__init__(socket_path, opener=opener)
 
+    def get_controller_agent_id(self):
+        resp = self.request_raw(path='/agent-id', method='GET')
+        return resp.read().decode('utf-8')
+
     def reload_config(self):
-        resp = self.request_raw(
-            method='POST',
-            path='/reload',
-        )
-        logger.debug('result of reload request: %r', resp)
+        self.request_raw(path='/reload', method='POST')

tests/test_charm.py

@@ -150,20 +150,15 @@ def test_apiaddresses_ipv6(self, _):
         self.assertEqual(self.harness.charm.api_port(), 17070)
 
     @patch("builtins.open", new_callable=mock_open, read_data=agent_conf)
-    @patch('subprocess.check_output')
+    @patch("configchangesocket.ConfigChangeSocketClient.get_controller_agent_id")
     @patch("ops.model.Model.get_binding")
-    @patch("charm.JujuControllerCharm._request_config_reload")
+    @patch("configchangesocket.ConfigChangeSocketClient.reload_config")
     def test_dbcluster_relation_changed_single_addr(
-            self, mock_reload_config, mock_get_binding, mock_check_out, *__):
+            self, mock_reload_config, mock_get_binding, mock_get_agent_id, *__):
         harness = self.harness
         mock_get_binding.return_value = mockBinding(['192.168.1.17'])
 
-        # This is an example of how the jujud invocation looks on machines/VMs.
-        pgrep = ('666 /var/lib/juju/tools/machine-0/jujud machine '
-                 '--data-dir /var/lib/juju --machine-id 0 --debug')
-
-        # First call is to get the controller service name; last is for its PID.
-        mock_check_out.side_effect = [pgrep, pgrep]
+        mock_get_agent_id.return_value = '0'
 
         harness.set_leader()
 
@@ -173,6 +168,7 @@ def test_dbcluster_relation_changed_single_addr(
         harness.add_relation_unit(relation_id, 'juju-controller/1')
         self.harness.update_relation_data(
             relation_id, 'juju-controller/1', {'db-bind-address': '192.168.1.100'})
+
         mock_reload_config.assert_called_once()
 
         unit_data = harness.get_relation_data(relation_id, 'juju-controller/0')
@@ -200,22 +196,17 @@ def test_dbcluster_relation_changed_multi_addr_error(self, binding, _):
         harness.evaluate_status()
         self.assertIsInstance(harness.charm.unit.status, BlockedStatus)
 
-    @patch('subprocess.check_output')
+    @patch("configchangesocket.ConfigChangeSocketClient.get_controller_agent_id")
     @patch("builtins.open", new_callable=mock_open)
     @patch("ops.model.Model.get_binding")
-    @patch("charm.JujuControllerCharm._request_config_reload")
+    @patch("configchangesocket.ConfigChangeSocketClient.reload_config")
     def test_dbcluster_relation_changed_write_file(
-            self, mock_reload_config, mock_get_binding, mock_open, mock_check_out):
+            self, mock_reload_config, mock_get_binding, mock_open, mock_get_agent_id):
 
         harness = self.harness
         mock_get_binding.return_value = mockBinding(['192.168.1.17'])
 
-        # This is an example of how the jujud invocation looks on K8s.
-        pgrep = ('12345 /var/lib/juju/tools/jujud machine --data-dir '
-                 '/var/lib/juju --controller-id 0 --log-to-stderr')
-
-        # First call is to get the controller service name; last is for its PID.
-        mock_check_out.side_effect = [pgrep, pgrep]
+        mock_get_agent_id.return_value = '0'
 
         relation_id = harness.add_relation('dbcluster', harness.charm.app)
         harness.add_relation_unit(relation_id, 'juju-controller/1')
@@ -242,7 +233,7 @@ def test_dbcluster_relation_changed_write_file(
 
         self.assertEqual(yaml.safe_load(written), {'db-bind-addresses': bound})
 
-        # The last thing we should have done is sent SIGHUP to Juju to reload the config.
+        # The last thing we should have done is send a reload request via the socket..
         mock_reload_config.assert_called_once()
 
 

tests/test_sockets.py

@@ -19,7 +19,6 @@ def test_add_metrics_user_success(self):
             url='http://localhost/metrics-users',
             method='POST',
             body=r'{"username": "juju-metrics-r0", "password": "passwd"}',
-
             response=MockResponse(
                 headers=MockHeaders(content_type='application/json'),
                 body=r'{"message":"created user \"juju-metrics-r0\""}'
@@ -35,7 +34,6 @@ def test_add_metrics_user_fail(self):
             url='http://localhost/metrics-users',
             method='POST',
             body=r'{"username": "juju-metrics-r0", "password": "passwd"}',
-
             error=urllib.error.HTTPError(
                 url='http://localhost/metrics-users',
                 code=409,
@@ -60,7 +58,6 @@ def test_remove_metrics_user_success(self):
             url='http://localhost/metrics-users/juju-metrics-r0',
             method='DELETE',
             body=None,
-
             response=MockResponse(
                 headers=MockHeaders(content_type='application/json'),
                 body=r'{"message":"deleted user \"juju-metrics-r0\""}'
@@ -76,7 +73,6 @@ def test_remove_metrics_user_fail(self):
             url='http://localhost/metrics-users/juju-metrics-r0',
             method='DELETE',
             body=None,
-
             error=urllib.error.HTTPError(
                 url='http://localhost/metrics-users/juju-metrics-r0',
                 code=404,
@@ -101,13 +97,29 @@ def test_connection_error(self):
             url='http://localhost/metrics-users',
             method='POST',
             body=r'{"username": "juju-metrics-r0", "password": "passwd"}',
-
             error=urllib.error.URLError('could not connect to socket')
         )
 
         with self.assertRaisesRegex(ConnectionError, 'could not connect to socket'):
             control_socket.add_metrics_user('juju-metrics-r0', 'passwd')
 
+    def test_get_controller_agent_id(self):
+        mock_opener = MockOpener(self)
+        config_reload_socket = ConfigChangeSocketClient('fake_socket_path', opener=mock_opener)
+
+        mock_opener.expect(
+            url='http://localhost/agent-id',
+            method='GET',
+            body=None,
+            response=MockResponse(
+                headers=MockHeaders(content_type='application/text'),
+                body=b'666'
+            )
+        )
+
+        id = config_reload_socket.get_controller_agent_id()
+        self.assertEqual(id, '666')
+
     def test_reload_config(self):
         mock_opener = MockOpener(self)
         config_reload_socket = ConfigChangeSocketClient('fake_socket_path', opener=mock_opener)
@@ -118,6 +130,7 @@ def test_reload_config(self):
             body=None,
             response=None,
         )
+
         config_reload_socket.reload_config()