From 7e81ab83992fc074c24ecebb57e83917d3d6a512 Mon Sep 17 00:00:00 2001 From: Lena Voytek Date: Mon, 6 Oct 2025 18:07:45 -0400 Subject: [PATCH 1/5] Open controller port on charm --- src/charm.py | 22 ++++++++++++++++++++++ tests/integration/test_charm.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/charm.py b/src/charm.py index 84c6262..fa25901 100755 --- a/src/charm.py +++ b/src/charm.py @@ -110,6 +110,27 @@ def _lpuser_ssh_key(self) -> str | None: return None + def _open_controller_port(self) -> bool: + """Open the configured controller network port. + + Returns: + True if the port was opened, False otherwise. + """ + try: + port = self._controller_port + + if port > 0: + self.unit.set_ports(port) + logger.info("Opened controller port %d", port) + else: + self.unit.status = ops.BlockedStatus("Invalid controller port configuration.") + return False + except ops.ModelError: + self.unit.status = ops.BlockedStatus("Failed to open controller port.") + return False + + return True + def _refresh_importer_node(self) -> None: """Remove old and install new git-ubuntu services.""" self.unit.status = ops.MaintenanceStatus("Refreshing git-ubuntu services.") @@ -260,6 +281,7 @@ def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: not self._update_git_user_config() or not self._update_lpuser_config() or not self._update_git_ubuntu_snap() + or not self._open_controller_port() ): return diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 3735628..867c190 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -7,6 +7,7 @@ import json import logging +import socket from time import sleep import jubilant @@ -123,6 +124,34 @@ def test_installed_dump_files(app: str, juju: jubilant.Juju): assert debian_keyring_status == "0" +def test_controller_port_open(app: str, juju: jubilant.Juju): + """Confirm that the git-ubuntu controller network port opens. + + Args: + app: The app in charge of this unit. + juju: The juju model in charge of the app. + """ + juju.wait(jubilant.all_active) + + def is_port_open(host: str, port: int) -> bool: + """Check if a port is opened in a particular host. + + Args: + host: The host network location. + port: The port number to check. + + Returns: True if the port is open, False otherwise. + """ + try: + with socket.create_connection((host, port), timeout=5): + return True + except (ConnectionRefusedError, TimeoutError): + return False + + address = juju.status().apps[app].units[f"{app}/0"].public_address + assert is_port_open(address, 1692) + + def test_update_config_with_ssh_key(app: str, juju: jubilant.Juju): """Wait on other tests, then update config with an ssh key and test that it exists. From 18bb1bbe14df933dd49b6d4b66610525d5210333 Mon Sep 17 00:00:00 2001 From: Lena Voytek Date: Thu, 9 Oct 2025 16:51:43 -0400 Subject: [PATCH 2/5] Add replica relation and auto primary ip determination --- charmcraft.yaml | 10 ++++------ src/charm.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index d32e741..9e92ff0 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -41,14 +41,12 @@ charm-libs: - lib: operator_libs_linux.systemd version: "1" +peers: + replicas: + interface: git_ubuntu_work_assignments + config: options: - controller_ip: - description: | - The IP or network location of the primary node. This option is ignored - for the primary node. - default: "127.0.0.1" - type: string controller_port: description: | The network port on the primary node used for import assignments, must diff --git a/src/charm.py b/src/charm.py index fa25901..5968075 100755 --- a/src/charm.py +++ b/src/charm.py @@ -50,9 +50,15 @@ def __init__(self, framework: ops.Framework): self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.config_changed, self._on_config_changed) + self.framework.observe( + self.on.replicas_relation_changed, self._on_replicas_relation_changed + ) + self.framework.observe(self.on.replicas_relation_joined, self._on_replicas_relation_joined) + @property - def _controller_ip(self) -> str: - return str(self.config.get("controller_ip")) + def _peer_relation(self) -> ops.Relation | None: + """Get replica peer relation if available.""" + return self.model.get_relation("replicas") @property def _controller_port(self) -> int: @@ -131,6 +137,28 @@ def _open_controller_port(self) -> bool: return True + def _get_primary_node_ip(self) -> str | None: + """Get the primary node's network address - local if primary or juju binding if secondary. + + Returns: + The primary IP as a string if available, None otherwise. + """ + if self._is_primary: + return "127.0.0.1" + + bind_address = None + + if self._peer_relation is not None: + network_binding = self.model.get_binding(self._peer_relation) + if network_binding is not None: + bind_address = network_binding.network.bind_address + + if bind_address is None: + logger.warning("Failed to get primary IP, secondary node needs a relation.") + return None + + return str(bind_address) + def _refresh_importer_node(self) -> None: """Remove old and install new git-ubuntu services.""" self.unit.status = ops.MaintenanceStatus("Refreshing git-ubuntu services.") @@ -166,6 +194,12 @@ def _refresh_importer_node(self) -> None: return logger.info("Initialized importer node as primary.") else: + primary_ip = self._get_primary_node_ip() + + if primary_ip is None: + self.unit.status = ops.BlockedStatus("Secondary node requires a peer relation.") + return + if not node.setup_secondary_node( GIT_UBUNTU_USER_HOME_DIR, self._node_id, @@ -173,7 +207,7 @@ def _refresh_importer_node(self) -> None: GIT_UBUNTU_SYSTEM_USER_USERNAME, will_publish, self._controller_port, - self._controller_ip, + primary_ip, ): self.unit.status = ops.BlockedStatus("Failed to install git-ubuntu services.") return @@ -288,6 +322,16 @@ def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: # Initialize or re-install git-ubuntu services as needed. self._refresh_importer_node() + def _on_replicas_relation_changed(self, _: ops.RelationChangedEvent) -> None: + """Refresh services for secondary nodes when peer relations change.""" + if not self._is_primary: + self._refresh_importer_node() + + def _on_replicas_relation_joined(self, _: ops.RelationJoinedEvent) -> None: + """Refresh services for secondary nodes when joined with a peer.""" + if not self._is_primary: + self._refresh_importer_node() + if __name__ == "__main__": # pragma: nocover ops.main(GitUbuntuCharm) From 52e5cab6ceed19360a31bfca1761bcd2eaaea6bf Mon Sep 17 00:00:00 2001 From: Lena Voytek Date: Thu, 16 Oct 2025 15:31:10 -0400 Subject: [PATCH 3/5] Use multi-unit setup and auto-determine primary --- charmcraft.yaml | 9 +---- src/charm.py | 67 +++++++++++++++++++++++---------- tests/integration/conftest.py | 4 +- tests/integration/test_charm.py | 40 +++++++++++++------- 4 files changed, 77 insertions(+), 43 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 9e92ff0..8858376 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -43,7 +43,7 @@ charm-libs: peers: replicas: - interface: git_ubuntu_work_assignments + interface: git_ubuntu_primary_info config: options: @@ -74,13 +74,6 @@ config: The ID of this git-ubuntu operator node, must be unique in the network. default: 0 type: int - primary: - description: | - If this is the primary git-ubuntu importer node, containing the Broker - and Poller instances. Non-primary nodes will only contain Worker - instances. - default: True - type: boolean publish: description: | If updates should be pushed to Launchpad. Set to False for local diff --git a/src/charm.py b/src/charm.py index 5968075..c942663 100755 --- a/src/charm.py +++ b/src/charm.py @@ -14,6 +14,7 @@ import logging from pathlib import Path +from socket import getfqdn import ops @@ -50,10 +51,10 @@ def __init__(self, framework: ops.Framework): self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.config_changed, self._on_config_changed) + self.framework.observe(self.on.leader_elected, self._on_leader_elected) self.framework.observe( self.on.replicas_relation_changed, self._on_replicas_relation_changed ) - self.framework.observe(self.on.replicas_relation_joined, self._on_replicas_relation_joined) @property def _peer_relation(self) -> ops.Relation | None: @@ -84,9 +85,7 @@ def _node_id(self) -> int: @property def _is_primary(self) -> bool: - if self.config.get("primary"): - return True - return False + return self.unit.is_leader() @property def _is_publishing_active(self) -> bool: @@ -116,6 +115,15 @@ def _lpuser_ssh_key(self) -> str | None: return None + @property + def _git_ubuntu_primary_relation(self) -> ops.Relation | None: + """Get the peer relation that contains the primary node IP. + + Returns: + The peer relation or None if it does not exist. + """ + return self.model.get_relation("replicas") + def _open_controller_port(self) -> bool: """Open the configured controller network port. @@ -137,7 +145,23 @@ def _open_controller_port(self) -> bool: return True - def _get_primary_node_ip(self) -> str | None: + def _set_peer_primary_node_address(self) -> bool: + """Set the primary node's IP to this unit's in the peer relation databag. + + Returns: + True if the data was updated, False otherwise. + """ + relation = self._git_ubuntu_primary_relation + + if relation: + new_primary_address = getfqdn() + relation.data[self.app]["primary_address"] = new_primary_address + logger.info("Updated primary node address to %s", new_primary_address) + return True + + return False + + def _get_primary_node_address(self) -> str | None: """Get the primary node's network address - local if primary or juju binding if secondary. Returns: @@ -146,18 +170,17 @@ def _get_primary_node_ip(self) -> str | None: if self._is_primary: return "127.0.0.1" - bind_address = None + relation = self._git_ubuntu_primary_relation - if self._peer_relation is not None: - network_binding = self.model.get_binding(self._peer_relation) - if network_binding is not None: - bind_address = network_binding.network.bind_address + if relation: + primary_address = relation.data[self.app]["primary_address"] - if bind_address is None: - logger.warning("Failed to get primary IP, secondary node needs a relation.") - return None + if primary_address is not None and len(str(primary_address)) > 0: + logger.info("Found primary node address %s", primary_address) + return str(primary_address) - return str(bind_address) + logger.warning("No primary node address found.") + return None def _refresh_importer_node(self) -> None: """Remove old and install new git-ubuntu services.""" @@ -194,7 +217,7 @@ def _refresh_importer_node(self) -> None: return logger.info("Initialized importer node as primary.") else: - primary_ip = self._get_primary_node_ip() + primary_ip = self._get_primary_node_address() if primary_ip is None: self.unit.status = ops.BlockedStatus("Secondary node requires a peer relation.") @@ -322,13 +345,17 @@ def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: # Initialize or re-install git-ubuntu services as needed. self._refresh_importer_node() - def _on_replicas_relation_changed(self, _: ops.RelationChangedEvent) -> None: - """Refresh services for secondary nodes when peer relations change.""" - if not self._is_primary: + def _on_leader_elected(self, _: ops.LeaderElectedEvent) -> None: + """Refresh services and update peer data when the unit is elected as leader.""" + if self._set_peer_primary_node_address(): self._refresh_importer_node() + else: + self.unit.status = ops.BlockedStatus( + "Failed to update primary node IP in peer relation." + ) - def _on_replicas_relation_joined(self, _: ops.RelationJoinedEvent) -> None: - """Refresh services for secondary nodes when joined with a peer.""" + def _on_replicas_relation_changed(self, _: ops.RelationChangedEvent) -> None: + """Refresh services for secondary nodes when peer relations change.""" if not self._is_primary: self._refresh_importer_node() diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 6c16649..9ef580e 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -38,8 +38,8 @@ def charm(): @pytest.fixture(scope="module") def app(juju: jubilant.Juju, charm: Path): - """Deploy git-ubuntu charm with publishing off.""" - juju.deploy(f"./{charm}") + """Deploy git-ubuntu charm with a primary and worker unit.""" + juju.deploy(f"./{charm}", num_units=2) juju.wait(lambda status: jubilant.all_active(status, APP_NAME)) yield APP_NAME diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 867c190..f606269 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -37,7 +37,7 @@ def get_services_dict(app: str, juju: jubilant.Juju) -> dict[str, dict[str, bool A dict mapping unit name to if the service is active and its description. """ service_output = juju.ssh( - f"{app}/0", + f"{app}/leader", "systemctl list-units --type service --full --all --output json --no-pager | cat -v", "", ) @@ -85,11 +85,12 @@ def test_installed_apps(app: str, juju: jubilant.Juju): """ juju.wait(jubilant.all_active) - def check_deb_installed(app: str, juju: jubilant.Juju, package_name: str) -> bool: - """Check if a deb pkg is installed on the app's unit 0. + def check_deb_installed(app: str, unit: int, juju: jubilant.Juju, package_name: str) -> bool: + """Check if a deb pkg is installed on a specific unit. Args: app: The app in charge of this unit. + unit: The unit number to check juju: The juju model in charge of the app. package_name: The name of the deb package. @@ -97,16 +98,22 @@ def check_deb_installed(app: str, juju: jubilant.Juju, package_name: str) -> boo True if the package is installed, False otherwise. """ install_status = juju.ssh( - f"{app}/0", f"dpkg-query --show --showformat='${{Status}}' {package_name}" + f"{app}/{unit}", f"dpkg-query --show --showformat='${{Status}}' {package_name}" ) return "installed" in install_status - assert check_deb_installed(app, juju, "git") - assert check_deb_installed(app, juju, "sqlite3") + assert check_deb_installed(app, 0, juju, "git") + assert check_deb_installed(app, 0, juju, "sqlite3") + assert check_deb_installed(app, 1, juju, "git") + assert check_deb_installed(app, 1, juju, "sqlite3") - git_ubuntu_status = juju.ssh(f"{app}/0", "snap list | grep git-ubuntu", "") - assert "latest/beta" in git_ubuntu_status - assert "classic" in git_ubuntu_status + git_ubuntu_status_1 = juju.ssh(f"{app}/0", "snap list | grep git-ubuntu", "") + assert "latest/beta" in git_ubuntu_status_1 + assert "classic" in git_ubuntu_status_1 + + git_ubuntu_status_2 = juju.ssh(f"{app}/1", "snap list | grep git-ubuntu", "") + assert "latest/beta" in git_ubuntu_status_2 + assert "classic" in git_ubuntu_status_2 def test_installed_dump_files(app: str, juju: jubilant.Juju): @@ -119,13 +126,13 @@ def test_installed_dump_files(app: str, juju: jubilant.Juju): juju.wait(jubilant.all_active) debian_keyring_status = juju.ssh( - f"{app}/0", "test -f /etc/git-ubuntu/debian-archive-keyring.gpg | echo $?", "" + f"{app}/leader", "test -f /etc/git-ubuntu/debian-archive-keyring.gpg | echo $?", "" ).strip() assert debian_keyring_status == "0" def test_controller_port_open(app: str, juju: jubilant.Juju): - """Confirm that the git-ubuntu controller network port opens. + """Confirm that the git-ubuntu controller leader network port opens. Args: app: The app in charge of this unit. @@ -148,7 +155,12 @@ def is_port_open(host: str, port: int) -> bool: except (ConnectionRefusedError, TimeoutError): return False - address = juju.status().apps[app].units[f"{app}/0"].public_address + address = None + for unit in juju.status().apps[app].units.values(): + if unit.leader: + address = unit.public_address + + assert address is not None assert is_port_open(address, 1692) @@ -171,6 +183,8 @@ def test_update_config_with_ssh_key(app: str, juju: jubilant.Juju): juju.config(app, {"lpuser_ssh_key": secret_uri}) juju.wait(jubilant.all_active) - ssh_key = juju.ssh(f"{app}/0", "sudo -u git-ubuntu cat /var/local/git-ubuntu/.ssh/id", "") + ssh_key = juju.ssh( + f"{app}/leader", "sudo -u git-ubuntu cat /var/local/git-ubuntu/.ssh/id", "" + ) assert file_content == ssh_key From 128dd1d4360add0cc6f272172a8aebfffa63ace9 Mon Sep 17 00:00:00 2001 From: Lena Voytek Date: Fri, 17 Oct 2025 11:28:07 -0400 Subject: [PATCH 4/5] Automatically determine node id and start when replicas change --- charmcraft.yaml | 5 --- src/charm.py | 22 +++++++----- tests/integration/test_charm.py | 61 ++++++++++++++++++--------------- 3 files changed, 46 insertions(+), 42 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 8858376..f4f5041 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -69,11 +69,6 @@ config: An ssh private key that matches with a public key associated with the lpuser account on Launchpad. type: secret - node_id: - description: | - The ID of this git-ubuntu operator node, must be unique in the network. - default: 0 - type: int publish: description: | If updates should be pushed to Launchpad. Set to False for local diff --git a/src/charm.py b/src/charm.py index c942663..592e891 100755 --- a/src/charm.py +++ b/src/charm.py @@ -78,10 +78,7 @@ def _lp_username(self) -> str: @property def _node_id(self) -> int: - node_id = self.config.get("node_id") - if isinstance(node_id, int): - return node_id - return 0 + return int(self.unit.name.split("/")[-1]) @property def _is_primary(self) -> bool: @@ -130,6 +127,8 @@ def _open_controller_port(self) -> bool: Returns: True if the port was opened, False otherwise. """ + self.unit.status = ops.MaintenanceStatus("Opening controller port.") + try: port = self._controller_port @@ -151,6 +150,8 @@ def _set_peer_primary_node_address(self) -> bool: Returns: True if the data was updated, False otherwise. """ + self.unit.status = ops.MaintenanceStatus("Setting primary node address in peer relation.") + relation = self._git_ubuntu_primary_relation if relation: @@ -238,8 +239,8 @@ def _refresh_importer_node(self) -> None: self.unit.status = ops.ActiveStatus("Importer node install complete.") - def _on_start(self, _: ops.StartEvent) -> None: - """Handle start event.""" + def _start_services(self): + """Start the services and note the result through status.""" if node.start(GIT_UBUNTU_USER_HOME_DIR): node_type_str = "primary" if self._is_primary else "secondary" self.unit.status = ops.ActiveStatus( @@ -248,6 +249,10 @@ def _on_start(self, _: ops.StartEvent) -> None: else: self.unit.status = ops.BlockedStatus("Failed to start services.") + def _on_start(self, _: ops.StartEvent) -> None: + """Handle start event.""" + self._start_services() + def _update_git_user_config(self) -> bool: """Attempt to update git config with the default git-ubuntu user name and email.""" self.unit.status = ops.MaintenanceStatus("Updating git config for git-ubuntu user.") @@ -347,9 +352,7 @@ def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: def _on_leader_elected(self, _: ops.LeaderElectedEvent) -> None: """Refresh services and update peer data when the unit is elected as leader.""" - if self._set_peer_primary_node_address(): - self._refresh_importer_node() - else: + if not self._set_peer_primary_node_address(): self.unit.status = ops.BlockedStatus( "Failed to update primary node IP in peer relation." ) @@ -358,6 +361,7 @@ def _on_replicas_relation_changed(self, _: ops.RelationChangedEvent) -> None: """Refresh services for secondary nodes when peer relations change.""" if not self._is_primary: self._refresh_importer_node() + self._start_services() if __name__ == "__main__": # pragma: nocover diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index f606269..ca5dadb 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -26,18 +26,18 @@ def test_service_status(app: str, juju: jubilant.Juju): juju.wait(jubilant.all_active) sleep(30) - def get_services_dict(app: str, juju: jubilant.Juju) -> dict[str, dict[str, bool | str]]: - """Get a dictionary of running systemd services on the app's unit 0. + def get_services_dict(unit_name: str, juju: jubilant.Juju) -> dict[str, dict[str, bool | str]]: + """Get a dictionary of running systemd services on the app's unit. Args: - app: The app in charge of this unit. + unit_name: The name of the unit to check. juju: The juju model in charge of the app. Returns: A dict mapping unit name to if the service is active and its description. """ service_output = juju.ssh( - f"{app}/leader", + unit_name, "systemctl list-units --type service --full --all --output json --no-pager | cat -v", "", ) @@ -52,28 +52,33 @@ def get_services_dict(app: str, juju: jubilant.Juju) -> dict[str, dict[str, bool return service_dict - services = get_services_dict(app, juju) - - assert services["git-ubuntu-importer-service-broker.service"]["active"] - assert ( - services["git-ubuntu-importer-service-broker.service"]["description"] - == "git-ubuntu importer service broker" - ) - assert services["git-ubuntu-importer-service-poller.service"]["active"] - assert ( - services["git-ubuntu-importer-service-poller.service"]["description"] - == "git-ubuntu importer service poller" - ) - assert services["git-ubuntu-importer-service-worker0_0.service"]["active"] - assert ( - services["git-ubuntu-importer-service-worker0_0.service"]["description"] - == "git-ubuntu importer service worker" - ) - assert services["git-ubuntu-importer-service-worker0_1.service"]["active"] - assert ( - services["git-ubuntu-importer-service-worker0_1.service"]["description"] - == "git-ubuntu importer service worker" - ) + for unit_name, unit in juju.status().get_units(app).items(): + services = get_services_dict(unit_name, juju) + + if unit.leader: + assert services["git-ubuntu-importer-service-broker.service"]["active"] + assert ( + services["git-ubuntu-importer-service-broker.service"]["description"] + == "git-ubuntu importer service broker" + ) + assert services["git-ubuntu-importer-service-poller.service"]["active"] + assert ( + services["git-ubuntu-importer-service-poller.service"]["description"] + == "git-ubuntu importer service poller" + ) + + node_id = int(unit_name.split("/")[-1]) + + assert services[f"git-ubuntu-importer-service-worker{node_id}_0.service"]["active"] + assert ( + services[f"git-ubuntu-importer-service-worker{node_id}_0.service"]["description"] + == "git-ubuntu importer service worker" + ) + assert services[f"git-ubuntu-importer-service-worker{node_id}_1.service"]["active"] + assert ( + services[f"git-ubuntu-importer-service-worker{node_id}_1.service"]["description"] + == "git-ubuntu importer service worker" + ) def test_installed_apps(app: str, juju: jubilant.Juju): @@ -90,7 +95,7 @@ def check_deb_installed(app: str, unit: int, juju: jubilant.Juju, package_name: Args: app: The app in charge of this unit. - unit: The unit number to check + unit: The unit number to check. juju: The juju model in charge of the app. package_name: The name of the deb package. @@ -156,7 +161,7 @@ def is_port_open(host: str, port: int) -> bool: return False address = None - for unit in juju.status().apps[app].units.values(): + for unit in juju.status().get_units(app).values(): if unit.leader: address = unit.public_address From 356c24ed82c6648ca7c5ef63ccdb17931ed66776 Mon Sep 17 00:00:00 2001 From: Lena Voytek Date: Fri, 17 Oct 2025 11:36:49 -0400 Subject: [PATCH 5/5] Add return type --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 592e891..e7c2ebc 100755 --- a/src/charm.py +++ b/src/charm.py @@ -239,7 +239,7 @@ def _refresh_importer_node(self) -> None: self.unit.status = ops.ActiveStatus("Importer node install complete.") - def _start_services(self): + def _start_services(self) -> None: """Start the services and note the result through status.""" if node.start(GIT_UBUNTU_USER_HOME_DIR): node_type_str = "primary" if self._is_primary else "secondary"