From 3abe3377c148785343ab42ec81c82573aea0e707 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Thu, 29 Jan 2026 01:12:35 +0700 Subject: [PATCH 01/15] feat: add debug juju action for enabling/disabling flavor --- charms/planner-operator/charmcraft.yaml | 12 ++++ charms/planner-operator/src/charm.py | 43 +++++++++++++ charms/tests/integration/test_planner.py | 78 ++++++++++++++++++++++++ cmd/planner/main.go | 57 +++++++++++++++++ 4 files changed, 190 insertions(+) diff --git a/charms/planner-operator/charmcraft.yaml b/charms/planner-operator/charmcraft.yaml index 646a4f31..29100e57 100644 --- a/charms/planner-operator/charmcraft.yaml +++ b/charms/planner-operator/charmcraft.yaml @@ -43,3 +43,15 @@ config: Planner admin token used to create/delete general auth tokens. It must start with 'planner_v0_' followed by exactly 20 characters that can be letters, numbers, hyphens (-), or underscores (_). To generate the token, you can run `echo "planner_v0_$(uuidgen | tr -d '-' | tr '[:upper:]' '[:lower:]' | head -c 20)"` + +actions: + update-flavor: + description: Enable or disable a specific flavor + params: + flavor: + type: string + description: Name of the flavor to update + disable: + type: boolean + description: True to disable the flavor, false to enable it + required: [flavor, disable] diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index f59b30ca..5c173de7 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -6,6 +6,7 @@ import logging import pathlib +import subprocess import typing import ops @@ -25,6 +26,9 @@ def __init__(self, *args: typing.Any) -> None: args: passthrough to CharmBase. """ super().__init__(*args) + self.framework.observe( + self.on.update_flavor_action, self._on_update_flavor_action + ) def get_cos_dir(self) -> str: """Get the COS directory for this charm. @@ -59,6 +63,45 @@ def gen_environment() -> dict[str, str]: setattr(app, "gen_environment", gen_environment) return app + def _on_update_flavor_action(self, event: ops.ActionEvent) -> None: + """Handle the update-flavor action. + + Args: + event: The action event. + """ + flavor = event.params["flavor"] + disable = event.params["disable"] + + try: + result = subprocess.run( + [ + "/charm/bin/planner", + "update-flavor", + "--flavor", + flavor, + "--disable" if disable else "--enable", + ], + capture_output=True, + text=True, + check=True, + timeout=30, + ) + event.set_results( + { + "message": f"Flavor '{flavor}' {'disabled' if disable else 'enabled'} successfully", + "output": result.stdout, + } + ) + except subprocess.CalledProcessError as e: + event.fail(f"Failed to update flavor: {e.stderr}") + logger.error("Failed to update flavor %s: %s", flavor, e.stderr) + except subprocess.TimeoutExpired: + event.fail("Command timed out") + logger.error("Update flavor command timed out for %s", flavor) + except Exception as e: + event.fail(f"Unexpected error: {str(e)}") + logger.exception("Unexpected error updating flavor %s", flavor) + if __name__ == "__main__": ops.main(GithubRunnerPlannerCharm) diff --git a/charms/tests/integration/test_planner.py b/charms/tests/integration/test_planner.py index 9820a1ef..51fa589f 100644 --- a/charms/tests/integration/test_planner.py +++ b/charms/tests/integration/test_planner.py @@ -54,3 +54,81 @@ def test_planner_prometheus_metrics( response = requests.get(f"http://{unit_ip}:{METRICS_PORT}/metrics") assert response.status_code == requests.status_codes.codes.OK + + +@pytest.mark.usefixtures("planner_with_integrations") +def test_planner_update_flavor_action( + juju: jubilant.Juju, + planner_app: str, + user_token: str, +): + """ + arrange: The planner app is deployed with required integrations and a flavor exists. + act: Run update-flavor action to disable and then enable the flavor. + assert: Flavor is disabled and enabled correctly as verified via API. + """ + status = juju.status() + unit_ip = status.apps[planner_app].units[planner_app + "/0"].address + flavor_name = "test-action-flavor" + + # Create a test flavor + response = requests.post( + f"http://{unit_ip}:{APP_PORT}/api/v1/flavors/{flavor_name}", + json={ + "platform": "github", + "labels": ["self-hosted", "linux"], + "priority": 50, + "minimum_pressure": 0, + }, + headers={ + "Content-Type": "application/json", + "Authorization": f"Bearer {user_token}", + }, + ) + assert response.status_code == requests.status_codes.codes.created + + # Verify flavor is initially enabled + response = requests.get( + f"http://{unit_ip}:{APP_PORT}/api/v1/flavors/{flavor_name}", + headers={"Authorization": f"Bearer {user_token}"}, + ) + assert response.status_code == requests.status_codes.codes.OK + flavor_data = response.json() + assert flavor_data["is_disabled"] is False, "Flavor should be enabled initially" + + # Run action to disable the flavor + unit_name = f"{planner_app}/0" + result = juju.run_action( + unit_name, + "update-flavor", + params={"flavor": flavor_name, "disable": True}, + ) + assert result.status == "completed", f"Action failed: {result.results}" + assert "successfully" in result.results.get("message", "").lower() + + # Verify flavor is now disabled + response = requests.get( + f"http://{unit_ip}:{APP_PORT}/api/v1/flavors/{flavor_name}", + headers={"Authorization": f"Bearer {user_token}"}, + ) + assert response.status_code == requests.status_codes.codes.OK + flavor_data = response.json() + assert flavor_data["is_disabled"] is True, "Flavor should be disabled after action" + + # Run action to enable the flavor + result = juju.run_action( + unit_name, + "update-flavor", + params={"flavor": flavor_name, "disable": False}, + ) + assert result.status == "completed", f"Action failed: {result.results}" + assert "successfully" in result.results.get("message", "").lower() + + # Verify flavor is enabled again + response = requests.get( + f"http://{unit_ip}:{APP_PORT}/api/v1/flavors/{flavor_name}", + headers={"Authorization": f"Bearer {user_token}"}, + ) + assert response.status_code == requests.status_codes.codes.OK + flavor_data = response.json() + assert flavor_data["is_disabled"] is False, "Flavor should be enabled after action" diff --git a/cmd/planner/main.go b/cmd/planner/main.go index 3d5433b0..747f523d 100644 --- a/cmd/planner/main.go +++ b/cmd/planner/main.go @@ -13,6 +13,8 @@ package main import ( "context" "errors" + "flag" + "fmt" "log" "net/http" "os" @@ -36,6 +38,7 @@ const ( adminTokenEnvVar = "APP_ADMIN_TOKEN_VALUE" serviceName = "github-runner-planner" rabbitMQUriEnvVar = "RABBITMQ_CONNECT_STRING" + platform = "github" shutdownTimeout = 30 * time.Second heartbeatInterval = 30 * time.Second ) @@ -53,6 +56,11 @@ type config struct { } func main() { + if len(os.Args) > 1 && os.Args[1] == "update-flavor" { + handleUpdateFlavor() + return + } + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() @@ -160,3 +168,52 @@ func shutdown(server *http.Server, consumer *planner.JobConsumer, consumerWg *sy log.Printf("failed to shutdown telemetry: %v", err) } } + +func handleUpdateFlavor() { + updateCmd := flag.NewFlagSet("update-flavor", flag.ExitOnError) + flavor := updateCmd.String("flavor", "", "Name of the flavor to update") + disable := updateCmd.Bool("disable", false, "Disable the flavor") + enable := updateCmd.Bool("enable", false, "Enable the flavor") + + if err := updateCmd.Parse(os.Args[2:]); err != nil { + log.Fatalf("Failed to parse flags: %v", err) + } + + if *flavor == "" { + log.Fatalln("--flavor is required") + } + + if *enable == *disable { + log.Fatalln("exactly one of --enable or --disable must be specified") + } + + dbURI, found := os.LookupEnv(dbURI) + if !found { + log.Fatalln(dbURI, "environment variable not set.") + } + + ctx := context.Background() + db, err := database.New(ctx, dbURI) + if err != nil { + log.Fatalf("Failed to connect to database: %v", err) + } + defer db.Close() + + if *enable { + if err := db.EnableFlavor(ctx, platform, *flavor); err != nil { + if errors.Is(err, database.ErrNotExist) { + log.Fatalf("Flavor '%s' not found", *flavor) + } + log.Fatalf("Failed to enable flavor: %v", err) + } + fmt.Printf("Flavor '%s' enabled successfully\n", *flavor) + } else { + if err := db.DisableFlavor(ctx, platform, *flavor); err != nil { + if errors.Is(err, database.ErrNotExist) { + log.Fatalf("Flavor '%s' not found", *flavor) + } + log.Fatalf("Failed to disable flavor: %v", err) + } + fmt.Printf("Flavor '%s' disabled successfully\n", *flavor) + } +} From a9227752668ffb3b489d1e373448c78fd1b620c9 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Thu, 29 Jan 2026 01:54:18 +0700 Subject: [PATCH 02/15] decrease cyclop --- charms/tests/integration/test_planner.py | 4 +-- cmd/planner/main.go | 45 ++++++++++++++++-------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/charms/tests/integration/test_planner.py b/charms/tests/integration/test_planner.py index 51fa589f..cca26ca7 100644 --- a/charms/tests/integration/test_planner.py +++ b/charms/tests/integration/test_planner.py @@ -98,7 +98,7 @@ def test_planner_update_flavor_action( # Run action to disable the flavor unit_name = f"{planner_app}/0" - result = juju.run_action( + result = juju.run( unit_name, "update-flavor", params={"flavor": flavor_name, "disable": True}, @@ -116,7 +116,7 @@ def test_planner_update_flavor_action( assert flavor_data["is_disabled"] is True, "Flavor should be disabled after action" # Run action to enable the flavor - result = juju.run_action( + result = juju.run( unit_name, "update-flavor", params={"flavor": flavor_name, "disable": False}, diff --git a/cmd/planner/main.go b/cmd/planner/main.go index 747f523d..4ad8a589 100644 --- a/cmd/planner/main.go +++ b/cmd/planner/main.go @@ -170,6 +170,11 @@ func shutdown(server *http.Server, consumer *planner.JobConsumer, consumerWg *sy } func handleUpdateFlavor() { + flavorName, shouldEnable := parseUpdateFlavorFlags() + executeFlavorUpdate(flavorName, shouldEnable) +} + +func parseUpdateFlavorFlags() (string, bool) { updateCmd := flag.NewFlagSet("update-flavor", flag.ExitOnError) flavor := updateCmd.String("flavor", "", "Name of the flavor to update") disable := updateCmd.Bool("disable", false, "Disable the flavor") @@ -187,6 +192,10 @@ func handleUpdateFlavor() { log.Fatalln("exactly one of --enable or --disable must be specified") } + return *flavor, *enable +} + +func executeFlavorUpdate(flavorName string, enable bool) { dbURI, found := os.LookupEnv(dbURI) if !found { log.Fatalln(dbURI, "environment variable not set.") @@ -199,21 +208,29 @@ func handleUpdateFlavor() { } defer db.Close() - if *enable { - if err := db.EnableFlavor(ctx, platform, *flavor); err != nil { - if errors.Is(err, database.ErrNotExist) { - log.Fatalf("Flavor '%s' not found", *flavor) - } - log.Fatalf("Failed to enable flavor: %v", err) - } - fmt.Printf("Flavor '%s' enabled successfully\n", *flavor) + if enable { + enableFlavor(ctx, db, flavorName) } else { - if err := db.DisableFlavor(ctx, platform, *flavor); err != nil { - if errors.Is(err, database.ErrNotExist) { - log.Fatalf("Flavor '%s' not found", *flavor) - } - log.Fatalf("Failed to disable flavor: %v", err) + disableFlavor(ctx, db, flavorName) + } +} + +func enableFlavor(ctx context.Context, db *database.Database, flavorName string) { + if err := db.EnableFlavor(ctx, platform, flavorName); err != nil { + if errors.Is(err, database.ErrNotExist) { + log.Fatalf("Flavor '%s' not found", flavorName) + } + log.Fatalf("Failed to enable flavor: %v", err) + } + fmt.Printf("Flavor '%s' enabled successfully\n", flavorName) +} + +func disableFlavor(ctx context.Context, db *database.Database, flavorName string) { + if err := db.DisableFlavor(ctx, platform, flavorName); err != nil { + if errors.Is(err, database.ErrNotExist) { + log.Fatalf("Flavor '%s' not found", flavorName) } - fmt.Printf("Flavor '%s' disabled successfully\n", *flavor) + log.Fatalf("Failed to disable flavor: %v", err) } + fmt.Printf("Flavor '%s' disabled successfully\n", flavorName) } From 430f2f781259a1880b6927f1aa1105ab79124b33 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Thu, 29 Jan 2026 02:39:12 +0700 Subject: [PATCH 03/15] fix path --- charms/planner-operator/charmcraft.yaml | 1 + charms/planner-operator/src/charm.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/charms/planner-operator/charmcraft.yaml b/charms/planner-operator/charmcraft.yaml index 29100e57..37c5aec6 100644 --- a/charms/planner-operator/charmcraft.yaml +++ b/charms/planner-operator/charmcraft.yaml @@ -55,3 +55,4 @@ actions: type: boolean description: True to disable the flavor, false to enable it required: [flavor, disable] + additionalProperties: false diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index 5c173de7..ee9335e3 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -75,7 +75,7 @@ def _on_update_flavor_action(self, event: ops.ActionEvent) -> None: try: result = subprocess.run( [ - "/charm/bin/planner", + "/usr/local/bin/planner", "update-flavor", "--flavor", flavor, From 9d7cd7944bed62ae1f8456f4e3a6f96768c0925a Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Thu, 29 Jan 2026 10:06:27 +0700 Subject: [PATCH 04/15] use container --- charms/planner-operator/src/charm.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index ee9335e3..5fb369f0 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -6,7 +6,6 @@ import logging import pathlib -import subprocess import typing import ops @@ -73,7 +72,7 @@ def _on_update_flavor_action(self, event: ops.ActionEvent) -> None: disable = event.params["disable"] try: - result = subprocess.run( + process = self._container.exec( [ "/usr/local/bin/planner", "update-flavor", @@ -81,21 +80,20 @@ def _on_update_flavor_action(self, event: ops.ActionEvent) -> None: flavor, "--disable" if disable else "--enable", ], - capture_output=True, - text=True, - check=True, timeout=30, + combine_stderr=True, ) + stdout, _ = process.wait_output() event.set_results( { "message": f"Flavor '{flavor}' {'disabled' if disable else 'enabled'} successfully", - "output": result.stdout, + "output": stdout, } ) - except subprocess.CalledProcessError as e: - event.fail(f"Failed to update flavor: {e.stderr}") - logger.error("Failed to update flavor %s: %s", flavor, e.stderr) - except subprocess.TimeoutExpired: + except ops.pebble.ExecError as e: + event.fail(f"Failed to update flavor: {e.stdout}") + logger.error("Failed to update flavor %s: %s", flavor, e.stdout) + except ops.pebble.TimeoutError: event.fail("Command timed out") logger.error("Update flavor command timed out for %s", flavor) except Exception as e: From 5102d118f75d87bb1a375735a37b6125d247b519 Mon Sep 17 00:00:00 2001 From: florentianayuwono <76247368+florentianayuwono@users.noreply.github.com> Date: Thu, 29 Jan 2026 10:44:07 +0700 Subject: [PATCH 05/15] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cmd/planner/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/planner/main.go b/cmd/planner/main.go index 4ad8a589..6e492146 100644 --- a/cmd/planner/main.go +++ b/cmd/planner/main.go @@ -196,13 +196,13 @@ func parseUpdateFlavorFlags() (string, bool) { } func executeFlavorUpdate(flavorName string, enable bool) { - dbURI, found := os.LookupEnv(dbURI) + dbConnURI, found := os.LookupEnv(dbURI) if !found { log.Fatalln(dbURI, "environment variable not set.") } ctx := context.Background() - db, err := database.New(ctx, dbURI) + db, err := database.New(ctx, dbConnURI) if err != nil { log.Fatalf("Failed to connect to database: %v", err) } From 00d918f06ff2cdff3ad44b7beae3c271c294fcd8 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Thu, 29 Jan 2026 13:51:45 +0700 Subject: [PATCH 06/15] add env --- charms/planner-operator/src/charm.py | 1 + 1 file changed, 1 insertion(+) diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index 5fb369f0..3ff06dff 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -80,6 +80,7 @@ def _on_update_flavor_action(self, event: ops.ActionEvent) -> None: flavor, "--disable" if disable else "--enable", ], + environment=self._gen_environment(), timeout=30, combine_stderr=True, ) From 126296a2db0d4b22018fc3d8c2bb334280b08504 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Fri, 30 Jan 2026 13:18:28 +0700 Subject: [PATCH 07/15] feat: update action command --- charms/planner-operator/charmcraft.yaml | 19 ++-- charms/planner-operator/src/charm.py | 139 +++++++++++++++++------ charms/tests/integration/test_planner.py | 12 +- cmd/planner/main.go | 71 ------------ 4 files changed, 125 insertions(+), 116 deletions(-) diff --git a/charms/planner-operator/charmcraft.yaml b/charms/planner-operator/charmcraft.yaml index 37c5aec6..8cafb62c 100644 --- a/charms/planner-operator/charmcraft.yaml +++ b/charms/planner-operator/charmcraft.yaml @@ -45,14 +45,19 @@ config: numbers, hyphens (-), or underscores (_). To generate the token, you can run `echo "planner_v0_$(uuidgen | tr -d '-' | tr '[:upper:]' '[:lower:]' | head -c 20)"` actions: - update-flavor: - description: Enable or disable a specific flavor + enable-flavor: + description: Enable a specific flavor params: flavor: type: string - description: Name of the flavor to update - disable: - type: boolean - description: True to disable the flavor, false to enable it - required: [flavor, disable] + description: Name of the flavor to enable + required: [flavor] + additionalProperties: false + disable-flavor: + description: Disable a specific flavor + params: + flavor: + type: string + description: Name of the flavor to disable + required: [flavor] additionalProperties: false diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index 3ff06dff..e3a6b0f0 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -4,9 +4,12 @@ """Go Charm entrypoint.""" +import json import logging import pathlib import typing +import urllib.request +import urllib.error import ops @@ -26,7 +29,10 @@ def __init__(self, *args: typing.Any) -> None: """ super().__init__(*args) self.framework.observe( - self.on.update_flavor_action, self._on_update_flavor_action + self.on.enable_flavor_action, self._on_enable_flavor_action + ) + self.framework.observe( + self.on.disable_flavor_action, self._on_disable_flavor_action ) def get_cos_dir(self) -> str: @@ -62,44 +68,113 @@ def gen_environment() -> dict[str, str]: setattr(app, "gen_environment", gen_environment) return app - def _on_update_flavor_action(self, event: ops.ActionEvent) -> None: - """Handle the update-flavor action. + def _update_flavor_via_api( + self, flavor_name: str, is_disabled: bool + ) -> tuple[bool, str]: + """Update flavor via REST API. Args: - event: The action event. + flavor_name: The name of the flavor to update. + is_disabled: Whether to disable (True) or enable (False) the flavor. + + Returns: + A tuple of (success, message). """ - flavor = event.params["flavor"] - disable = event.params["disable"] + env = self._gen_environment() + port = env.get("APP_PORT", "8080") + admin_token = env.get("APP_ADMIN_TOKEN_VALUE") + + if not admin_token: + return False, "Admin token not configured" + + url = f"http://127.0.0.1:{port}/api/v1/flavors/{flavor_name}" try: - process = self._container.exec( - [ - "/usr/local/bin/planner", - "update-flavor", - "--flavor", - flavor, - "--disable" if disable else "--enable", - ], - environment=self._gen_environment(), - timeout=30, - combine_stderr=True, + current_flavor = self._get_flavor(url, admin_token) + if not current_flavor: + return False, f"Flavor '{flavor_name}' not found" + + current_flavor["is_disabled"] = is_disabled + + data = json.dumps(current_flavor).encode("utf-8") + req = urllib.request.Request( + url, + data=data, + method="PATCH", + headers={ + "Authorization": f"Bearer {admin_token}", + "Content-Type": "application/json", + }, ) - stdout, _ = process.wait_output() - event.set_results( - { - "message": f"Flavor '{flavor}' {'disabled' if disable else 'enabled'} successfully", - "output": stdout, - } - ) - except ops.pebble.ExecError as e: - event.fail(f"Failed to update flavor: {e.stdout}") - logger.error("Failed to update flavor %s: %s", flavor, e.stdout) - except ops.pebble.TimeoutError: - event.fail("Command timed out") - logger.error("Update flavor command timed out for %s", flavor) + + with urllib.request.urlopen(req, timeout=10) as response: + if response.status == 200: + action = "disabled" if is_disabled else "enabled" + return True, f"Flavor '{flavor_name}' {action} successfully" + return False, f"Unexpected status code: {response.status}" + + except urllib.error.HTTPError as e: + error_body = e.read().decode("utf-8") if e.fp else "" + return False, f"HTTP error {e.code}: {error_body}" + except urllib.error.URLError as e: + return False, f"Connection error: {e.reason}" except Exception as e: - event.fail(f"Unexpected error: {str(e)}") - logger.exception("Unexpected error updating flavor %s", flavor) + return False, f"Unexpected error: {str(e)}" + + def _get_flavor(self, url: str, admin_token: str) -> dict[str, typing.Any] | None: + """Get current flavor configuration from API. + + Args: + url: The API URL for the flavor. + admin_token: The admin authentication token. + + Returns: + The flavor configuration dict, or None if not found. + """ + try: + req = urllib.request.Request( + url, + method="GET", + headers={"Authorization": f"Bearer {admin_token}"}, + ) + with urllib.request.urlopen(req, timeout=10) as response: + if response.status == 200: + return json.loads(response.read().decode("utf-8")) + return None + except urllib.error.HTTPError: + return None + except Exception: + return None + + def _on_enable_flavor_action(self, event: ops.ActionEvent) -> None: + """Handle the enable-flavor action. + + Args: + event: The action event. + """ + flavor = event.params["flavor"] + success, message = self._update_flavor_via_api(flavor, is_disabled=False) + + if success: + event.set_results({"message": message}) + else: + event.fail(message) + logger.error("Failed to enable flavor %s: %s", flavor, message) + + def _on_disable_flavor_action(self, event: ops.ActionEvent) -> None: + """Handle the disable-flavor action. + + Args: + event: The action event. + """ + flavor = event.params["flavor"] + success, message = self._update_flavor_via_api(flavor, is_disabled=True) + + if success: + event.set_results({"message": message}) + else: + event.fail(message) + logger.error("Failed to disable flavor %s: %s", flavor, message) if __name__ == "__main__": diff --git a/charms/tests/integration/test_planner.py b/charms/tests/integration/test_planner.py index cca26ca7..a0f801e1 100644 --- a/charms/tests/integration/test_planner.py +++ b/charms/tests/integration/test_planner.py @@ -57,14 +57,14 @@ def test_planner_prometheus_metrics( @pytest.mark.usefixtures("planner_with_integrations") -def test_planner_update_flavor_action( +def test_planner_enable_disable_flavor_actions( juju: jubilant.Juju, planner_app: str, user_token: str, ): """ arrange: The planner app is deployed with required integrations and a flavor exists. - act: Run update-flavor action to disable and then enable the flavor. + act: Run disable-flavor and enable-flavor actions. assert: Flavor is disabled and enabled correctly as verified via API. """ status = juju.status() @@ -100,8 +100,8 @@ def test_planner_update_flavor_action( unit_name = f"{planner_app}/0" result = juju.run( unit_name, - "update-flavor", - params={"flavor": flavor_name, "disable": True}, + "disable-flavor", + params={"flavor": flavor_name}, ) assert result.status == "completed", f"Action failed: {result.results}" assert "successfully" in result.results.get("message", "").lower() @@ -118,8 +118,8 @@ def test_planner_update_flavor_action( # Run action to enable the flavor result = juju.run( unit_name, - "update-flavor", - params={"flavor": flavor_name, "disable": False}, + "enable-flavor", + params={"flavor": flavor_name}, ) assert result.status == "completed", f"Action failed: {result.results}" assert "successfully" in result.results.get("message", "").lower() diff --git a/cmd/planner/main.go b/cmd/planner/main.go index 6e492146..e6dd3deb 100644 --- a/cmd/planner/main.go +++ b/cmd/planner/main.go @@ -56,11 +56,6 @@ type config struct { } func main() { - if len(os.Args) > 1 && os.Args[1] == "update-flavor" { - handleUpdateFlavor() - return - } - ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() @@ -168,69 +163,3 @@ func shutdown(server *http.Server, consumer *planner.JobConsumer, consumerWg *sy log.Printf("failed to shutdown telemetry: %v", err) } } - -func handleUpdateFlavor() { - flavorName, shouldEnable := parseUpdateFlavorFlags() - executeFlavorUpdate(flavorName, shouldEnable) -} - -func parseUpdateFlavorFlags() (string, bool) { - updateCmd := flag.NewFlagSet("update-flavor", flag.ExitOnError) - flavor := updateCmd.String("flavor", "", "Name of the flavor to update") - disable := updateCmd.Bool("disable", false, "Disable the flavor") - enable := updateCmd.Bool("enable", false, "Enable the flavor") - - if err := updateCmd.Parse(os.Args[2:]); err != nil { - log.Fatalf("Failed to parse flags: %v", err) - } - - if *flavor == "" { - log.Fatalln("--flavor is required") - } - - if *enable == *disable { - log.Fatalln("exactly one of --enable or --disable must be specified") - } - - return *flavor, *enable -} - -func executeFlavorUpdate(flavorName string, enable bool) { - dbConnURI, found := os.LookupEnv(dbURI) - if !found { - log.Fatalln(dbURI, "environment variable not set.") - } - - ctx := context.Background() - db, err := database.New(ctx, dbConnURI) - if err != nil { - log.Fatalf("Failed to connect to database: %v", err) - } - defer db.Close() - - if enable { - enableFlavor(ctx, db, flavorName) - } else { - disableFlavor(ctx, db, flavorName) - } -} - -func enableFlavor(ctx context.Context, db *database.Database, flavorName string) { - if err := db.EnableFlavor(ctx, platform, flavorName); err != nil { - if errors.Is(err, database.ErrNotExist) { - log.Fatalf("Flavor '%s' not found", flavorName) - } - log.Fatalf("Failed to enable flavor: %v", err) - } - fmt.Printf("Flavor '%s' enabled successfully\n", flavorName) -} - -func disableFlavor(ctx context.Context, db *database.Database, flavorName string) { - if err := db.DisableFlavor(ctx, platform, flavorName); err != nil { - if errors.Is(err, database.ErrNotExist) { - log.Fatalf("Flavor '%s' not found", flavorName) - } - log.Fatalf("Failed to disable flavor: %v", err) - } - fmt.Printf("Flavor '%s' disabled successfully\n", flavorName) -} From 81eb481aa66b4a65260194bafa4bfb38f8494d3b Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Fri, 30 Jan 2026 13:21:22 +0700 Subject: [PATCH 08/15] remove unused imports --- cmd/planner/main.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/planner/main.go b/cmd/planner/main.go index e6dd3deb..3d5433b0 100644 --- a/cmd/planner/main.go +++ b/cmd/planner/main.go @@ -13,8 +13,6 @@ package main import ( "context" "errors" - "flag" - "fmt" "log" "net/http" "os" @@ -38,7 +36,6 @@ const ( adminTokenEnvVar = "APP_ADMIN_TOKEN_VALUE" serviceName = "github-runner-planner" rabbitMQUriEnvVar = "RABBITMQ_CONNECT_STRING" - platform = "github" shutdownTimeout = 30 * time.Second heartbeatInterval = 30 * time.Second ) From 55b4683c68e0a1326be3a7bf11d137da20604806 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Fri, 30 Jan 2026 16:34:27 +0700 Subject: [PATCH 09/15] address code reviews --- charms/planner-operator/src/charm.py | 128 ++++++++++++--------------- 1 file changed, 58 insertions(+), 70 deletions(-) diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index e3a6b0f0..1b733e36 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -4,14 +4,12 @@ """Go Charm entrypoint.""" -import json import logging import pathlib import typing -import urllib.request -import urllib.error import ops +import requests import paas_charm.go @@ -68,9 +66,37 @@ def gen_environment() -> dict[str, str]: setattr(app, "gen_environment", gen_environment) return app - def _update_flavor_via_api( - self, flavor_name: str, is_disabled: bool - ) -> tuple[bool, str]: + def _on_enable_flavor_action(self, event: ops.ActionEvent) -> None: + """Handle the enable-flavor action. + + Args: + event: The action event. + """ + flavor = event.params["flavor"] + try: + message = self._update_flavor_via_api(flavor, is_disabled=False) + event.set_results({"message": message}) + except RuntimeError as e: + error_msg = str(e) + event.fail(error_msg) + logger.error("Failed to enable flavor %s: %s", flavor, error_msg) + + def _on_disable_flavor_action(self, event: ops.ActionEvent) -> None: + """Handle the disable-flavor action. + + Args: + event: The action event. + """ + flavor = event.params["flavor"] + try: + message = self._update_flavor_via_api(flavor, is_disabled=True) + event.set_results({"message": message}) + except RuntimeError as e: + error_msg = str(e) + event.fail(error_msg) + logger.error("Failed to disable flavor %s: %s", flavor, error_msg) + + def _update_flavor_via_api(self, flavor_name: str, is_disabled: bool) -> str: """Update flavor via REST API. Args: @@ -78,48 +104,44 @@ def _update_flavor_via_api( is_disabled: Whether to disable (True) or enable (False) the flavor. Returns: - A tuple of (success, message). + Success message. + + Raises: + RuntimeError: If admin token is not configured or API call fails. """ env = self._gen_environment() port = env.get("APP_PORT", "8080") admin_token = env.get("APP_ADMIN_TOKEN_VALUE") if not admin_token: - return False, "Admin token not configured" + raise RuntimeError("Admin token not configured") url = f"http://127.0.0.1:{port}/api/v1/flavors/{flavor_name}" try: current_flavor = self._get_flavor(url, admin_token) if not current_flavor: - return False, f"Flavor '{flavor_name}' not found" + raise RuntimeError(f"Flavor '{flavor_name}' not found") current_flavor["is_disabled"] = is_disabled - data = json.dumps(current_flavor).encode("utf-8") - req = urllib.request.Request( + response = requests.patch( url, - data=data, - method="PATCH", - headers={ - "Authorization": f"Bearer {admin_token}", - "Content-Type": "application/json", - }, + json=current_flavor, + headers={"Authorization": f"Bearer {admin_token}"}, + timeout=10, ) - - with urllib.request.urlopen(req, timeout=10) as response: - if response.status == 200: - action = "disabled" if is_disabled else "enabled" - return True, f"Flavor '{flavor_name}' {action} successfully" - return False, f"Unexpected status code: {response.status}" - - except urllib.error.HTTPError as e: - error_body = e.read().decode("utf-8") if e.fp else "" - return False, f"HTTP error {e.code}: {error_body}" - except urllib.error.URLError as e: - return False, f"Connection error: {e.reason}" - except Exception as e: - return False, f"Unexpected error: {str(e)}" + response.raise_for_status() + action = "disabled" if is_disabled else "enabled" + return f"Flavor '{flavor_name}' {action} successfully" + + except requests.exceptions.HTTPError as e: + error_body = e.response.text if e.response else "" + raise RuntimeError( + f"HTTP error {e.response.status_code}: {error_body}" + ) from e + except requests.exceptions.RequestException as e: + raise RuntimeError(f"Connection error: {str(e)}") from e def _get_flavor(self, url: str, admin_token: str) -> dict[str, typing.Any] | None: """Get current flavor configuration from API. @@ -132,50 +154,16 @@ def _get_flavor(self, url: str, admin_token: str) -> dict[str, typing.Any] | Non The flavor configuration dict, or None if not found. """ try: - req = urllib.request.Request( + response = requests.get( url, - method="GET", headers={"Authorization": f"Bearer {admin_token}"}, + timeout=10, ) - with urllib.request.urlopen(req, timeout=10) as response: - if response.status == 200: - return json.loads(response.read().decode("utf-8")) - return None - except urllib.error.HTTPError: - return None - except Exception: + response.raise_for_status() + return response.json() + except requests.exceptions.RequestException: return None - def _on_enable_flavor_action(self, event: ops.ActionEvent) -> None: - """Handle the enable-flavor action. - - Args: - event: The action event. - """ - flavor = event.params["flavor"] - success, message = self._update_flavor_via_api(flavor, is_disabled=False) - - if success: - event.set_results({"message": message}) - else: - event.fail(message) - logger.error("Failed to enable flavor %s: %s", flavor, message) - - def _on_disable_flavor_action(self, event: ops.ActionEvent) -> None: - """Handle the disable-flavor action. - - Args: - event: The action event. - """ - flavor = event.params["flavor"] - success, message = self._update_flavor_via_api(flavor, is_disabled=True) - - if success: - event.set_results({"message": message}) - else: - event.fail(message) - logger.error("Failed to disable flavor %s: %s", flavor, message) - if __name__ == "__main__": ops.main(GithubRunnerPlannerCharm) From 19dff2ede045e5ee4b20f5ca64cb947ea45be9d3 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Fri, 30 Jan 2026 21:19:04 +0700 Subject: [PATCH 10/15] add req library --- charms/planner-operator/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/charms/planner-operator/requirements.txt b/charms/planner-operator/requirements.txt index ce52318f..363df41e 100644 --- a/charms/planner-operator/requirements.txt +++ b/charms/planner-operator/requirements.txt @@ -1,2 +1,3 @@ ops==3.5.1 paas-charm==1.9.2 +requests==2.32.5 From 46cfb0e7633ad930b4d827b4e8829e6a8add01e4 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Tue, 3 Feb 2026 00:02:03 +0700 Subject: [PATCH 11/15] refactor to planner client class --- charms/planner-operator/src/charm.py | 172 +++++-------------------- charms/planner-operator/src/planner.py | 125 ++++++++++++++++++ 2 files changed, 157 insertions(+), 140 deletions(-) create mode 100644 charms/planner-operator/src/planner.py diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index a781a086..7d6c340b 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -8,11 +8,10 @@ import pathlib import typing -import requests import ops -import requests import paas_charm.go +from planner import PlannerClient, PlannerError logger = logging.getLogger(__name__) @@ -26,10 +25,6 @@ class ConfigError(Exception): """Error for configuration issues.""" -class PlannerError(Exception): - """Error for planner application issues.""" - - class JujuError(Exception): """Error for Juju-related issues.""" @@ -89,6 +84,20 @@ def gen_environment() -> dict[str, str]: setattr(app, "gen_environment", gen_environment) return app + def _create_planner_client(self, admin_token: str) -> PlannerClient: + """Create a planner client instance. + + Args: + admin_token: Admin token for authentication. + + Returns: + PlannerClient instance. + """ + env = self._gen_environment() + port = env.get("APP_PORT", "8080") + base_url = f"http://127.0.0.1:{port}" + return PlannerClient(base_url, admin_token) + def _on_enable_flavor_action(self, event: ops.ActionEvent) -> None: """Handle the enable-flavor action. @@ -97,9 +106,11 @@ def _on_enable_flavor_action(self, event: ops.ActionEvent) -> None: """ flavor = event.params["flavor"] try: - message = self._update_flavor_via_api(flavor, is_disabled=False) - event.set_results({"message": message}) - except RuntimeError as e: + admin_token = self._get_admin_token() + client = self._create_planner_client(admin_token) + client.update_flavor(flavor, is_disabled=False) + event.set_results({"message": f"Flavor '{flavor}' enabled successfully"}) + except (ConfigError, PlannerError, RuntimeError) as e: error_msg = str(e) event.fail(error_msg) logger.error("Failed to enable flavor %s: %s", flavor, error_msg) @@ -112,80 +123,15 @@ def _on_disable_flavor_action(self, event: ops.ActionEvent) -> None: """ flavor = event.params["flavor"] try: - message = self._update_flavor_via_api(flavor, is_disabled=True) - event.set_results({"message": message}) - except RuntimeError as e: + admin_token = self._get_admin_token() + client = self._create_planner_client(admin_token) + client.update_flavor(flavor, is_disabled=True) + event.set_results({"message": f"Flavor '{flavor}' disabled successfully"}) + except (ConfigError, PlannerError, RuntimeError) as e: error_msg = str(e) event.fail(error_msg) logger.error("Failed to disable flavor %s: %s", flavor, error_msg) - def _update_flavor_via_api(self, flavor_name: str, is_disabled: bool) -> str: - """Update flavor via REST API. - - Args: - flavor_name: The name of the flavor to update. - is_disabled: Whether to disable (True) or enable (False) the flavor. - - Returns: - Success message. - - Raises: - RuntimeError: If admin token is not configured or API call fails. - """ - env = self._gen_environment() - port = env.get("APP_PORT", "8080") - admin_token = env.get("APP_ADMIN_TOKEN_VALUE") - - if not admin_token: - raise RuntimeError("Admin token not configured") - - url = f"http://127.0.0.1:{port}/api/v1/flavors/{flavor_name}" - - try: - current_flavor = self._get_flavor(url, admin_token) - if not current_flavor: - raise RuntimeError(f"Flavor '{flavor_name}' not found") - - current_flavor["is_disabled"] = is_disabled - - response = requests.patch( - url, - json=current_flavor, - headers={"Authorization": f"Bearer {admin_token}"}, - timeout=10, - ) - response.raise_for_status() - action = "disabled" if is_disabled else "enabled" - return f"Flavor '{flavor_name}' {action} successfully" - - except requests.exceptions.HTTPError as e: - error_body = e.response.text if e.response else "" - raise RuntimeError( - f"HTTP error {e.response.status_code}: {error_body}" - ) from e - except requests.exceptions.RequestException as e: - raise RuntimeError(f"Connection error: {str(e)}") from e - - def _get_flavor(self, url: str, admin_token: str) -> dict[str, typing.Any] | None: - """Get current flavor configuration from API. - - Args: - url: The API URL for the flavor. - admin_token: The admin authentication token. - - Returns: - The flavor configuration dict, or None if not found. - """ - try: - response = requests.get( - url, - headers={"Authorization": f"Bearer {admin_token}"}, - timeout=10, - ) - response.raise_for_status() - return response.json() - except requests.exceptions.RequestException: - return None def _on_manager_relation_changed(self, _: ops.RelationChangedEvent) -> None: """Handle changes to the github-runner-manager relation.""" self._setup() @@ -215,31 +161,18 @@ def _setup_planner_relation(self, admin_token: str) -> None: if not self.unit.is_leader(): return - auth_token_names = None - try: - response = requests.get( - f"http://localhost:{HTTP_PORT}/api/v1/auth/token", - headers={"Authorization": f"Bearer {admin_token}"}, - ) - response.raise_for_status() - auth_token_names = [ - token - for token in response.json()["names"] - if self._check_name_fit_auth_token(token) - ] - except requests.RequestException as err: - logger.exception("Failed to list the names of auth tokens") - raise PlannerError("Failed to list the names of auth tokens") from err - - if auth_token_names is None: - auth_token_names = [] + client = self._create_planner_client(admin_token) + all_token_names = client.list_auth_token_names() + auth_token_names = [ + token for token in all_token_names if self._check_name_fit_auth_token(token) + ] auth_token_set = set(auth_token_names) relations = self.model.relations[PLANNER_RELATION_NAME] for relation in relations: auth_token_name = self._get_auth_token_name(relation.id) if auth_token_name not in auth_token_set: - auth_token = self._create_auth_token(admin_token, auth_token_name) + auth_token = client.create_auth_token(auth_token_name) try: secret = self.app.add_secret( {"token": auth_token}, label=auth_token_name @@ -275,48 +208,7 @@ def _setup_planner_relation(self, admin_token: str) -> None: logger.debug( "Secret with label %s not found during cleanup", token_name ) - self._remove_auth_token(admin_token, token_name) - - @staticmethod - def _create_auth_token(admin_token: str, name: str) -> str: - """Create an auth token secret in the planner application. - - Args: - admin_token: The admin token for making API requests to planner. - name: The name of the auth token. - - Returns: - The auth token. - """ - try: - response = requests.post( - f"http://localhost:{HTTP_PORT}/api/v1/auth/token/{name}", - headers={"Authorization": f"Bearer {admin_token}"}, - ) - response.raise_for_status() - return response.json()["token"] - except requests.RequestException as err: - logger.exception("Failed to create auth token %s", name) - raise PlannerError(f"Failed to create auth token {name}") from err - - @staticmethod - def _remove_auth_token(admin_token: str, name: str) -> None: - """Remove an auth token secret in the planner application. - - Args: - admin_token: The admin token for making API requests to planner. - name: The name of the auth token. - """ - try: - # If the token does not exist, the response code will be 204 No Content. - response = requests.delete( - f"http://localhost:{HTTP_PORT}/api/v1/auth/token/{name}", - headers={"Authorization": f"Bearer {admin_token}"}, - ) - response.raise_for_status() - except requests.RequestException as err: - logger.exception("Failed to remove auth token %s", name) - raise PlannerError(f"Failed to remove auth token {name}") from err + client.delete_auth_token(token_name) def _get_admin_token(self) -> str: admin_token_secret_id = self.config.get(ADMIN_TOKEN_CONFIG_NAME) diff --git a/charms/planner-operator/src/planner.py b/charms/planner-operator/src/planner.py new file mode 100644 index 00000000..bc68ec69 --- /dev/null +++ b/charms/planner-operator/src/planner.py @@ -0,0 +1,125 @@ +# Copyright 2026 Ubuntu +# See LICENSE file for licensing details. + +"""Planner API client.""" + +import typing + +import requests + + +class PlannerError(Exception): + """Error for planner application issues.""" + + +class PlannerClient: + """Client for interacting with the planner API.""" + + def __init__(self, base_url: str, admin_token: str, timeout: int = 10) -> None: + """Initialize the planner client. + + Args: + base_url: Base URL for the planner API. + admin_token: Admin token for authentication. + timeout: Request timeout in seconds. + """ + self._base_url = base_url.rstrip("/") + self._admin_token = admin_token + self._timeout = timeout + + def _request( + self, + method: str, + path: str, + json_data: dict[str, typing.Any] | None = None, + ) -> requests.Response: + """Make an HTTP request to the planner API. + + Args: + method: HTTP method (GET, POST, PATCH, DELETE). + path: API path (e.g., "/api/v1/flavors/small"). + json_data: Optional JSON payload. + + Returns: + Response object. + + Raises: + PlannerError: If API returns non-2xx status code. + RuntimeError: If connection fails. + """ + url = f"{self._base_url}{path}" + headers = {"Authorization": f"Bearer {self._admin_token}"} + + try: + response = requests.request( + method, + url, + json=json_data, + headers=headers, + timeout=self._timeout, + ) + response.raise_for_status() + return response + except requests.exceptions.HTTPError as e: + error_body = e.response.text if e.response else "" + raise PlannerError( + f"HTTP error {e.response.status_code}: {error_body}" + ) from e + except requests.exceptions.RequestException as e: + raise RuntimeError(f"Connection error: {str(e)}") from e + + def update_flavor(self, flavor_name: str, is_disabled: bool) -> None: + """Update flavor disabled status. + + Args: + flavor_name: The name of the flavor to update. + is_disabled: Whether to disable (True) or enable (False) the flavor. + + Raises: + PlannerError: If API returns non-2xx status code. + RuntimeError: If connection fails. + """ + self._request( + "PATCH", f"/api/v1/flavors/{flavor_name}", {"is_disabled": is_disabled} + ) + + def list_auth_token_names(self) -> list[str]: + """List all auth token names. + + Returns: + List of auth token names. + + Raises: + PlannerError: If API returns non-2xx status code. + RuntimeError: If connection fails. + """ + response = self._request("GET", "/api/v1/auth/token") + return response.json()["names"] + + def create_auth_token(self, name: str) -> str: + """Create an auth token. + + Args: + name: The name of the auth token. + + Returns: + The auth token value. + + Raises: + PlannerError: If API returns non-2xx status code. + RuntimeError: If connection fails. + """ + response = self._request("POST", f"/api/v1/auth/token/{name}") + return response.json()["token"] + + def delete_auth_token(self, name: str) -> None: + """Delete an auth token. + + Args: + name: The name of the auth token. + + Raises: + PlannerError: If API returns non-2xx status code. + RuntimeError: If connection fails. + """ + self._request("DELETE", f"/api/v1/auth/token/{name}") From 40076553c0ad684a1de3a9950a8f08f6aa9f1f41 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Thu, 5 Feb 2026 10:27:20 +0700 Subject: [PATCH 12/15] address code reviews --- charms/planner-operator/src/charm.py | 32 ++++++++++---------------- charms/planner-operator/src/planner.py | 14 ++++++----- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index 7d6c340b..e2d60696 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -84,19 +84,17 @@ def gen_environment() -> dict[str, str]: setattr(app, "gen_environment", gen_environment) return app - def _create_planner_client(self, admin_token: str) -> PlannerClient: + def _create_planner_client(self) -> PlannerClient: """Create a planner client instance. - Args: - admin_token: Admin token for authentication. - Returns: PlannerClient instance. """ + admin_token = self._get_admin_token() env = self._gen_environment() port = env.get("APP_PORT", "8080") base_url = f"http://127.0.0.1:{port}" - return PlannerClient(base_url, admin_token) + return PlannerClient(base_url=base_url, admin_token=admin_token) def _on_enable_flavor_action(self, event: ops.ActionEvent) -> None: """Handle the enable-flavor action. @@ -106,9 +104,8 @@ def _on_enable_flavor_action(self, event: ops.ActionEvent) -> None: """ flavor = event.params["flavor"] try: - admin_token = self._get_admin_token() - client = self._create_planner_client(admin_token) - client.update_flavor(flavor, is_disabled=False) + client = self._create_planner_client() + client.update_flavor(flavor_name=flavor, is_disabled=False) event.set_results({"message": f"Flavor '{flavor}' enabled successfully"}) except (ConfigError, PlannerError, RuntimeError) as e: error_msg = str(e) @@ -123,9 +120,8 @@ def _on_disable_flavor_action(self, event: ops.ActionEvent) -> None: """ flavor = event.params["flavor"] try: - admin_token = self._get_admin_token() - client = self._create_planner_client(admin_token) - client.update_flavor(flavor, is_disabled=True) + client = self._create_planner_client() + client.update_flavor(flavor_name=flavor, is_disabled=True) event.set_results({"message": f"Flavor '{flavor}' disabled successfully"}) except (ConfigError, PlannerError, RuntimeError) as e: error_msg = str(e) @@ -140,7 +136,7 @@ def _setup(self) -> None: """Setup the planner application.""" self.unit.status = ops.MaintenanceStatus("Setting up planner application") try: - admin_token = self._get_admin_token() + self._get_admin_token() except ConfigError: logger.exception("Missing %s configuration", ADMIN_TOKEN_CONFIG_NAME) self.unit.status = ops.BlockedStatus( @@ -149,19 +145,15 @@ def _setup(self) -> None: return self.unit.status = ops.MaintenanceStatus("Setup planner integrations") - self._setup_planner_relation(admin_token) + self._setup_planner_relation() self.unit.status = ops.ActiveStatus() - def _setup_planner_relation(self, admin_token: str) -> None: - """Setup the planner relations if this unit is the leader. - - Args: - admin_token: The admin token for making API requests to planner. - """ + def _setup_planner_relation(self) -> None: + """Setup the planner relations if this unit is the leader.""" if not self.unit.is_leader(): return - client = self._create_planner_client(admin_token) + client = self._create_planner_client() all_token_names = client.list_auth_token_names() auth_token_names = [ token for token in all_token_names if self._check_name_fit_auth_token(token) diff --git a/charms/planner-operator/src/planner.py b/charms/planner-operator/src/planner.py index bc68ec69..28c046d0 100644 --- a/charms/planner-operator/src/planner.py +++ b/charms/planner-operator/src/planner.py @@ -52,8 +52,8 @@ def _request( try: response = requests.request( - method, - url, + method=method, + url=url, json=json_data, headers=headers, timeout=self._timeout, @@ -80,7 +80,9 @@ def update_flavor(self, flavor_name: str, is_disabled: bool) -> None: RuntimeError: If connection fails. """ self._request( - "PATCH", f"/api/v1/flavors/{flavor_name}", {"is_disabled": is_disabled} + method="PATCH", + path=f"/api/v1/flavors/{flavor_name}", + json_data={"is_disabled": is_disabled}, ) def list_auth_token_names(self) -> list[str]: @@ -93,7 +95,7 @@ def list_auth_token_names(self) -> list[str]: PlannerError: If API returns non-2xx status code. RuntimeError: If connection fails. """ - response = self._request("GET", "/api/v1/auth/token") + response = self._request(method="GET", path="/api/v1/auth/token") return response.json()["names"] def create_auth_token(self, name: str) -> str: @@ -109,7 +111,7 @@ def create_auth_token(self, name: str) -> str: PlannerError: If API returns non-2xx status code. RuntimeError: If connection fails. """ - response = self._request("POST", f"/api/v1/auth/token/{name}") + response = self._request(method="POST", path=f"/api/v1/auth/token/{name}") return response.json()["token"] def delete_auth_token(self, name: str) -> None: @@ -122,4 +124,4 @@ def delete_auth_token(self, name: str) -> None: PlannerError: If API returns non-2xx status code. RuntimeError: If connection fails. """ - self._request("DELETE", f"/api/v1/auth/token/{name}") + self._request(method="DELETE", path=f"/api/v1/auth/token/{name}") From a1a8a3cf6183151737ec416d1eb082fc3c4e28ba Mon Sep 17 00:00:00 2001 From: florentianayuwono <76247368+florentianayuwono@users.noreply.github.com> Date: Thu, 5 Feb 2026 19:46:11 +0700 Subject: [PATCH 13/15] Update charms/planner-operator/src/planner.py Co-authored-by: Christopher Bartz --- charms/planner-operator/src/planner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charms/planner-operator/src/planner.py b/charms/planner-operator/src/planner.py index 28c046d0..2dff88d8 100644 --- a/charms/planner-operator/src/planner.py +++ b/charms/planner-operator/src/planner.py @@ -1,4 +1,4 @@ -# Copyright 2026 Ubuntu +# Copyright 2026 Canonical # See LICENSE file for licensing details. """Planner API client.""" From ccb7e60f982efe4f1b17aa539ba39b6e20ecde1e Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Fri, 6 Feb 2026 10:36:40 +0700 Subject: [PATCH 14/15] add unit tests --- charms/planner-operator/src/charm.py | 2 +- .../tests/unit/test_planner.py | 97 +++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 charms/planner-operator/tests/unit/test_planner.py diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index e2d60696..5cef1dd3 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -9,8 +9,8 @@ import typing import ops - import paas_charm.go + from planner import PlannerClient, PlannerError logger = logging.getLogger(__name__) diff --git a/charms/planner-operator/tests/unit/test_planner.py b/charms/planner-operator/tests/unit/test_planner.py new file mode 100644 index 00000000..4c723457 --- /dev/null +++ b/charms/planner-operator/tests/unit/test_planner.py @@ -0,0 +1,97 @@ +# Copyright 2026 Canonical +# See LICENSE file for licensing details. + +"""Unit test for planner client.""" + +import requests + +from planner import PlannerClient, PlannerError + +base_url = "http://127.0.0.1:8080" + + +def test_list_auth_token_names_returns_names(requests_mock: object) -> None: + """ + arrange: A requests mock for GET request to the /api/v1/auth/token endpoint. + act: The list_auth_token_names method of the PlannerClient is called. + assert: The returned list of token names matches the expected list. + """ + requests_mock.get(f"{base_url}/api/v1/auth/token", json={"names": ["one", "two"]}) + client = PlannerClient(base_url=base_url, admin_token="token") + + assert client.list_auth_token_names() == ["one", "two"] + + +def test_create_auth_token_returns_token(requests_mock: object) -> None: + """ + arrange: A requests mock for POST request to the /api/v1/auth/token/{name} endpoint. + act: The create_auth_token method of the PlannerClient is called. + assert: The returned token matches the expected token. + """ + requests_mock.post(f"{base_url}/api/v1/auth/token/runner", json={"token": "secret"}) + client = PlannerClient(base_url=base_url, admin_token="token") + + assert client.create_auth_token("runner") == "secret" + + +def test_update_flavor_completes_successfully(requests_mock: object) -> None: + """ + arrange: A requests mock for PATCH request to the /api/v1/flavors/{name} endpoint. + act: The update_flavor method of the PlannerClient is called. + assert: The method completes successfully without raising an exception. + """ + requests_mock.patch(f"{base_url}/api/v1/flavors/small", status_code=200) + client = PlannerClient(base_url=base_url, admin_token="token") + + client.update_flavor("small", True) + + +def test_delete_auth_token_completes_successfully(requests_mock: object) -> None: + """ + arrange: A requests mock for DELETE request to the /api/v1/auth/token/{name} endpoint. + act: The delete_auth_token method of the PlannerClient is called. + assert: The method completes successfully without raising an exception. + """ + requests_mock.delete(f"{base_url}/api/v1/auth/token/runner", status_code=204) + client = PlannerClient(base_url=base_url, admin_token="token") + + client.delete_auth_token("runner") + + +def test_http_error_raises_planner_error(requests_mock: object) -> None: + """ + arrange: A requests mock that returns 400. + act: The list_auth_token_names method of the PlannerClient is called. + assert: A PlannerError is raised with the expected message. + """ + requests_mock.get( + f"{base_url}/api/v1/auth/token", status_code=400, text="bad request" + ) + client = PlannerClient(base_url=base_url, admin_token="token") + + try: + client.list_auth_token_names() + except PlannerError as exc: + assert "HTTP error 400" in str(exc) + else: + raise AssertionError("PlannerError not raised") + + +def test_connection_error_raises_runtime_error(requests_mock: object) -> None: + """ + arrange: A requests mock that raises a connection error. + act: The list_auth_token_names method of the PlannerClient is called. + assert: A RuntimeError is raised with the expected message. + """ + requests_mock.get( + f"{base_url}/api/v1/auth/token", + exc=requests.exceptions.ConnectionError("boom"), + ) + client = PlannerClient(base_url=base_url, admin_token="token") + + try: + client.list_auth_token_names() + except RuntimeError as exc: + assert "Connection error" in str(exc) + else: + raise AssertionError("RuntimeError not raised") From d86e8a281e982da1dccfa03e0cc59ae795f71008 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Fri, 6 Feb 2026 15:17:32 +0700 Subject: [PATCH 15/15] use pytest raise --- charms/planner-operator/src/charm.py | 2 +- charms/planner-operator/src/planner.py | 2 +- .../planner-operator/tests/unit/test_planner.py | 15 ++++----------- charms/planner-operator/tox.ini | 3 ++- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/charms/planner-operator/src/charm.py b/charms/planner-operator/src/charm.py index 5cef1dd3..0fda01f7 100755 --- a/charms/planner-operator/src/charm.py +++ b/charms/planner-operator/src/charm.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright 2025 Ubuntu +# Copyright 2025 Canonical Ltd. # See LICENSE file for licensing details. """Go Charm entrypoint.""" diff --git a/charms/planner-operator/src/planner.py b/charms/planner-operator/src/planner.py index 2dff88d8..9502aa8c 100644 --- a/charms/planner-operator/src/planner.py +++ b/charms/planner-operator/src/planner.py @@ -1,4 +1,4 @@ -# Copyright 2026 Canonical +# Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. """Planner API client.""" diff --git a/charms/planner-operator/tests/unit/test_planner.py b/charms/planner-operator/tests/unit/test_planner.py index 4c723457..0b2f6f07 100644 --- a/charms/planner-operator/tests/unit/test_planner.py +++ b/charms/planner-operator/tests/unit/test_planner.py @@ -1,8 +1,9 @@ -# Copyright 2026 Canonical +# Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. """Unit test for planner client.""" +import pytest import requests from planner import PlannerClient, PlannerError @@ -69,12 +70,8 @@ def test_http_error_raises_planner_error(requests_mock: object) -> None: ) client = PlannerClient(base_url=base_url, admin_token="token") - try: + with pytest.raises(PlannerError, match="HTTP error 400"): client.list_auth_token_names() - except PlannerError as exc: - assert "HTTP error 400" in str(exc) - else: - raise AssertionError("PlannerError not raised") def test_connection_error_raises_runtime_error(requests_mock: object) -> None: @@ -89,9 +86,5 @@ def test_connection_error_raises_runtime_error(requests_mock: object) -> None: ) client = PlannerClient(base_url=base_url, admin_token="token") - try: + with pytest.raises(RuntimeError, match="Connection error"): client.list_auth_token_names() - except RuntimeError as exc: - assert "Connection error" in str(exc) - else: - raise AssertionError("RuntimeError not raised") diff --git a/charms/planner-operator/tox.ini b/charms/planner-operator/tox.ini index 1e8a7610..d63f8155 100644 --- a/charms/planner-operator/tox.ini +++ b/charms/planner-operator/tox.ini @@ -1,4 +1,4 @@ -# Copyright 2025 Ubuntu +# Copyright 2025 Canonical Ltd. # See LICENSE file for licensing details. [tox] @@ -48,6 +48,7 @@ commands = description = Run unit tests deps = pytest + requests-mock coverage[toml] -r {tox_root}/requirements.txt commands =