Fix: Regression on removal of residual dirs (#388)

* ensure remove_residual_venv_dirs runs first * run test_charm_upgrade on every PR * update changelog * remove TODO (should be an item in the backlog)
canonical · Oct 7, 2024 · 3778f19 · 3778f19
1 parent 40c9b5d
commit 3778f19
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 29 deletions.
diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml
@@ -19,11 +19,11 @@ jobs:
       pre-run-script: scripts/pre-integration-test.sh
       provider: lxd
       test-tox-env: integration-juju3.1
-      # These important local LXD test has no OpenStack integration versions.
+      # These important local LXD test have no OpenStack integration versions.
       # test_charm_scheduled_events ensures reconcile events are fired on a schedule.
       # test_debug_ssh ensures tmate SSH actions works.
-      # TODO: Add OpenStack integration versions of these tests.
-      modules: '["test_charm_scheduled_events", "test_debug_ssh"]'
+      # The test test_charm_upgrade needs to run to ensure the charm can be upgraded.
+      modules: '["test_charm_scheduled_events", "test_debug_ssh", "test_charm_upgrade"]'
   openstack-interface-tests-private-endpoint:
     name: openstack interface test using private-endpoint
     uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -2,7 +2,8 @@
 
 ### 2024-10-07
 
-- Fix removal of proxy vars in `.env` file for LXD runners.
+- Fixed a regression in the removal of leftover directories.
+- Fixed the removal of proxy vars in `.env` file for LXD runners.
 
 ### 2024-09-18
 

diff --git a/src-docs/charm.md b/src-docs/charm.md
@@ -22,7 +22,7 @@ Charm for creating and managing GitHub self-hosted runner instances.
 
 ---
 
-<a href="../src/charm.py#L122"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L123"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `catch_charm_errors`
 
@@ -48,7 +48,7 @@ Catch common errors in charm.
 
 ---
 
-<a href="../src/charm.py#L163"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L164"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `catch_action_errors`
 
@@ -74,7 +74,7 @@ Catch common errors in actions.
 
 ---
 
-<a href="../src/charm.py#L115"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L116"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>class</kbd> `ReconcileRunnersEvent`
 Event representing a periodic check to ensure runners are ok. 
@@ -85,7 +85,7 @@ Event representing a periodic check to ensure runners are ok.
 
 ---
 
-<a href="../src/charm.py#L201"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L202"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>class</kbd> `GithubRunnerCharm`
 Charm for managing GitHub self-hosted runners. 
@@ -102,7 +102,7 @@ Charm for managing GitHub self-hosted runners.
  - <b>`ram_pool_path`</b>:  The path to memdisk storage. 
  - <b>`kernel_module_path`</b>:  The path to kernel modules. 
 
-<a href="../src/charm.py#L224"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm.py#L225"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>method</kbd> `__init__`
 

diff --git a/src/charm.py b/src/charm.py
@@ -7,29 +7,11 @@
 # pylint: disable=too-many-lines
 
 """Charm for creating and managing GitHub self-hosted runner instances."""
-from github_runner_manager.manager.cloud_runner_manager import (
-    GitHubRunnerConfig,
-    SupportServiceConfig,
-)
-from github_runner_manager.manager.runner_manager import (
-    FlushMode,
-    RunnerManager,
-    RunnerManagerConfig,
-)
-from github_runner_manager.manager.runner_scaler import RunnerScaler
-from github_runner_manager.openstack_cloud.openstack_runner_manager import (
-    OpenStackCloudConfig,
-    OpenStackRunnerManager,
-    OpenStackRunnerManagerConfig,
-    OpenStackServerConfig,
-)
-from github_runner_manager.reactive.types_ import QueueConfig as ReactiveQueueConfig
-from github_runner_manager.reactive.types_ import RunnerConfig as ReactiveRunnerConfig
-from github_runner_manager.types_.github import GitHubPath, GitHubRunnerStatus, parse_github_path
 
 from utilities import bytes_with_unit_to_kib, execute_command, remove_residual_venv_dirs, retry
 
 # This is a workaround for https://bugs.launchpad.net/juju/+bug/2058335
+# It is important that this is run before importation of any other modules.
 # pylint: disable=wrong-import-position,wrong-import-order
 # TODO: 2024-07-17 remove this once the issue has been fixed
 remove_residual_venv_dirs()
@@ -48,6 +30,25 @@
 import ops
 from charms.data_platform_libs.v0.data_interfaces import DatabaseRequires
 from charms.grafana_agent.v0.cos_agent import COSAgentProvider
+from github_runner_manager.manager.cloud_runner_manager import (
+    GitHubRunnerConfig,
+    SupportServiceConfig,
+)
+from github_runner_manager.manager.runner_manager import (
+    FlushMode,
+    RunnerManager,
+    RunnerManagerConfig,
+)
+from github_runner_manager.manager.runner_scaler import RunnerScaler
+from github_runner_manager.openstack_cloud.openstack_runner_manager import (
+    OpenStackCloudConfig,
+    OpenStackRunnerManager,
+    OpenStackRunnerManagerConfig,
+    OpenStackServerConfig,
+)
+from github_runner_manager.reactive.types_ import QueueConfig as ReactiveQueueConfig
+from github_runner_manager.reactive.types_ import RunnerConfig as ReactiveRunnerConfig
+from github_runner_manager.types_.github import GitHubPath, GitHubRunnerStatus, parse_github_path
 from ops.charm import (
     ActionEvent,
     CharmBase,

diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py
@@ -40,7 +40,7 @@ async def test_charm_upgrade(
     assert: the charm is upgraded successfully.
     """
     latest_stable_path = tmp_path / "github-runner.charm"
-    latest_stable_revision = 161  # update this value every release to stable.
+    latest_stable_revision = 256  # update this value every release to stable.
     # download the charm and inject lxd profile for testing
     retcode, stdout, stderr = await ops_test.juju(
         "download",