Fix/openstack connection creation (#378)

* Update to use openstack credential directly * Remove test for removed code * Fix the unit tests * Fix dict values type error * Fix dependency issues * Try new deps * Try new deps * Fix merge issue * Remove a todo comment * Add debugging * Fix merge issue * Remove debug * Fix clouds.yaml access * Fix clouds.yaml dict access * Add comments on the auth fields of clouds.yaml * Fix fmt * Add reactive debug test * Debug reactive * Try if region_name * Print deubgging info * add region_name to _OpenStackAuth * add region_name to _OpenStackCloud * run other integration tests * fix test_runner_manager_openstack * fix unit tests * revert integration_test.yaml * fix unit test * lint * update changelog * increase image-builder deploy timeout * pin correct commit * final new line * remove todo * lint --------- Co-authored-by: Christopher Bartz <[email protected]>
canonical · Oct 17, 2024 · 3168532 · 3168532
1 parent 5a31f10
commit 3168532
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 79 deletions.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -1,4 +1,12 @@
 # Changelog
+
+### 2024-10-17
+
+- Use in-memory authentication instead of clouds.yaml on disk for OpenStack. This prevents
+the multi-processing fighting over the file handle for the clouds.yaml file in the github-runner-manager.
+
+- Fixed a bug where metrics storage for unmatched runners could not get cleaned up.
+
 ### 2024-10-11
 
 - Added support for COS integration with reactive runners.

diff --git a/requirements.txt b/requirements.txt
@@ -12,4 +12,4 @@ cosl ==0.0.15
 # juju 3.1.2.0 depends on pyyaml<=6.0 and >=5.1.2
 PyYAML ==6.0.*
 pyOpenSSL==24.2.1
-github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@601dbb70a983b2902566503e462a8567f80758e7
+github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@6b136f4e915c7dfec22e801981bcc0a6af581df1
diff --git a/src-docs/charm_state.py.md b/src-docs/charm_state.py.md
@@ -110,7 +110,7 @@ Some charm configurations are grouped into other configuration models.
 
 ---
 
-<a href="../src/charm_state.py#L448"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L440"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `check_reconcile_interval`
 
@@ -139,7 +139,7 @@ Validate the general charm configuration.
 
 ---
 
-<a href="../src/charm_state.py#L475"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L467"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -178,7 +178,7 @@ Raised when charm config is invalid.
 
  - <b>`msg`</b>:  Explanation of the error. 
 
-<a href="../src/charm_state.py#L196"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L194"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>function</kbd> `__init__`
 
@@ -221,7 +221,7 @@ The charm state.
 
 ---
 
-<a href="../src/charm_state.py#L1146"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L1138"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -267,7 +267,7 @@ Charm configuration related to GitHub.
 
 ---
 
-<a href="../src/charm_state.py#L109"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L107"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -300,7 +300,7 @@ Get github related charm configuration values from charm.
 ## <kbd>class</kbd> `ImmutableConfigChangedError`
 Represents an error when changing immutable charm state. 
 
-<a href="../src/charm_state.py#L1014"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L1006"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>function</kbd> `__init__`
 
@@ -355,7 +355,7 @@ Runner configurations for local LXD instances.
 
 ---
 
-<a href="../src/charm_state.py#L745"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L737"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `check_virtual_machine_resources`
 
@@ -386,7 +386,7 @@ Validate the virtual_machine_resources field values.
 
 ---
 
-<a href="../src/charm_state.py#L723"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L715"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `check_virtual_machines`
 
@@ -415,7 +415,7 @@ Validate the virtual machines configuration value.
 
 ---
 
-<a href="../src/charm_state.py#L671"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L663"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -475,7 +475,7 @@ OpenstackImage from image builder relation data.
 
 ---
 
-<a href="../src/charm_state.py#L581"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L573"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -518,7 +518,7 @@ Runner configuration for OpenStack Instances.
 
 ---
 
-<a href="../src/charm_state.py#L623"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L615"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -572,7 +572,7 @@ Return the aproxy address.
 
 ---
 
-<a href="../src/charm_state.py#L815"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L807"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `check_use_aproxy`
 
@@ -602,7 +602,7 @@ Validate the proxy configuration.
 
 ---
 
-<a href="../src/charm_state.py#L843"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L835"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -640,7 +640,7 @@ Represents the configuration for reactive scheduling.
 
 ---
 
-<a href="../src/charm_state.py#L977"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L969"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_database`
 
@@ -685,7 +685,7 @@ Configuration for the repo policy compliance service.
 
 ---
 
-<a href="../src/charm_state.py#L263"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L261"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -748,7 +748,7 @@ SSH connection information for debug workflow.
 
 ---
 
-<a href="../src/charm_state.py#L929"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L921"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>classmethod</kbd> `from_charm`
 
@@ -781,7 +781,7 @@ Raised when given machine charm architecture is unsupported.
 
  - <b>`arch`</b>:  The current machine architecture. 
 
-<a href="../src/charm_state.py#L886"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/charm_state.py#L878"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ### <kbd>function</kbd> `__init__`
 

diff --git a/src/charm.py b/src/charm.py
@@ -41,7 +41,7 @@
 )
 from github_runner_manager.manager.runner_scaler import RunnerScaler
 from github_runner_manager.openstack_cloud.openstack_runner_manager import (
-    OpenStackCloudConfig,
+    OpenStackCredentials,
     OpenStackRunnerManager,
     OpenStackRunnerManagerConfig,
     OpenStackServerConfig,
@@ -1346,9 +1346,15 @@ def _create_openstack_runner_manager_config(
             logger.warning(
                 "Multiple clouds defined in clouds.yaml. Using the first one to connect."
             )
-        cloud_config = OpenStackCloudConfig(
-            clouds_config=state.charm_config.openstack_clouds_yaml,
-            cloud=clouds[0],
+        first_cloud_config = state.charm_config.openstack_clouds_yaml["clouds"][clouds[0]]
+        credentials = OpenStackCredentials(
+            auth_url=first_cloud_config["auth"]["auth_url"],
+            project_name=first_cloud_config["auth"]["project_name"],
+            username=first_cloud_config["auth"]["username"],
+            password=first_cloud_config["auth"]["password"],
+            user_domain_name=first_cloud_config["auth"]["user_domain_name"],
+            project_domain_name=first_cloud_config["auth"]["project_domain_name"],
+            region_name=first_cloud_config["region_name"],
         )
         server_config = None
         image = state.runner_config.openstack_image
@@ -1370,7 +1376,7 @@ def _create_openstack_runner_manager_config(
             name=self.app.name,
             # The prefix is set to f"{application_name}-{unit number}"
             prefix=self.unit.name.replace("/", "-"),
-            cloud_config=cloud_config,
+            credentials=credentials,
             server_config=server_config,
             runner_config=runner_config,
             service_config=service_config,

diff --git a/src/charm_state.py b/src/charm_state.py
@@ -19,8 +19,6 @@
 
 import yaml
 from charms.data_platform_libs.v0.data_interfaces import DatabaseRequires
-from github_runner_manager import openstack_cloud
-from github_runner_manager.errors import OpenStackInvalidConfigError
 from github_runner_manager.types_.github import GitHubPath, parse_github_path
 from ops import CharmBase
 from pydantic import (
@@ -315,9 +313,11 @@ class _OpenStackCloud(TypedDict):
 
     Attributes:
         auth: The connection authentication info.
+        region_name: The OpenStack region to authenticate to.
     """
 
     auth: _OpenStackAuth
+    region_name: str
 
 
 class OpenStackCloudsYAML(TypedDict):
@@ -435,14 +435,6 @@ def _parse_openstack_clouds_config(cls, charm: CharmBase) -> OpenStackCloudsYAML
                 f"Invalid {OPENSTACK_CLOUDS_YAML_CONFIG_NAME} config. Invalid yaml."
             ) from exc
 
-        try:
-            openstack_cloud.initialize(openstack_clouds_yaml)
-        except OpenStackInvalidConfigError as exc:
-            logger.error("Invalid openstack config, %s.", exc)
-            raise CharmConfigInvalidError(
-                "Invalid openstack config. Not able to initialize openstack integration."
-            ) from exc
-
         return openstack_clouds_yaml
 
     @validator("reconcile_interval")

diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
@@ -48,6 +48,8 @@
 from tests.integration.helpers.openstack import OpenStackInstanceHelper, PrivateEndpointConfigs
 from tests.status_name import ACTIVE
 
+IMAGE_BUILDER_DEPLOY_TIMEOUT_IN_SECONDS = 30 * 60
+
 # The following line is required because we are using request.getfixturevalue in conjunction
 # with pytest-asyncio. See https://github.com/pytest-dev/pytest-asyncio/issues/112
 nest_asyncio.apply()
@@ -397,7 +399,9 @@ async def image_builder_fixture(
                 "openstack-user-name": private_endpoint_config["username"],
             },
         )
-        await model.wait_for_idle(apps=[app.name], wait_for_active=True, timeout=15 * 60)
+        await model.wait_for_idle(
+            apps=[app.name], wait_for_active=True, timeout=IMAGE_BUILDER_DEPLOY_TIMEOUT_IN_SECONDS
+        )
     else:
         app = model.applications["github-runner-image-builder"]
     return app

diff --git a/tests/integration/test_runner_manager_openstack.py b/tests/integration/test_runner_manager_openstack.py
@@ -31,9 +31,8 @@
 )
 from github_runner_manager.metrics import events
 from github_runner_manager.openstack_cloud import health_checks
-from github_runner_manager.openstack_cloud.openstack_cloud import _CLOUDS_YAML_PATH
 from github_runner_manager.openstack_cloud.openstack_runner_manager import (
-    OpenStackCloudConfig,
+    OpenStackCredentials,
     OpenStackRunnerManager,
     OpenStackRunnerManagerConfig,
     OpenStackServerConfig,
@@ -113,13 +112,27 @@ async def openstack_runner_manager_fixture(
     The prefix args of OpenstackRunnerManager set to app_name to let openstack_connection_fixture
     perform the cleanup of openstack resources.
     """
-    _CLOUDS_YAML_PATH.unlink(missing_ok=True)
     clouds_config = yaml.safe_load(private_endpoint_clouds_yaml)
 
-    cloud_config = OpenStackCloudConfig(
-        clouds_config=clouds_config,
-        cloud="testcloud",
-    )
+    try:
+        # Pick the first cloud in the clouds.yaml
+        cloud = tuple(clouds_config["clouds"].values())[0]
+        print("============================================")
+        print(cloud)
+        print("============================================")
+
+        credentials = OpenStackCredentials(
+            auth_url=cloud["auth"]["auth_url"],
+            project_name=cloud["auth"]["project_name"],
+            username=cloud["auth"]["username"],
+            password=cloud["auth"]["password"],
+            user_domain_name=cloud["auth"]["user_domain_name"],
+            project_domain_name=cloud["auth"]["project_domain_name"],
+            region_name=cloud["region_name"],
+        )
+    except KeyError as err:
+        raise AssertionError("Issue with the format of the clouds.yaml used in test") from err
+
     server_config = OpenStackServerConfig(
         image=openstack_test_image,
         flavor=flavor_name,
@@ -139,7 +152,7 @@ async def openstack_runner_manager_fixture(
     openstack_runner_manager_config = OpenStackRunnerManagerConfig(
         name=app_name,
         prefix=f"{app_name}-0",
-        cloud_config=cloud_config,
+        credentials=credentials,
         server_config=server_config,
         runner_config=runner_config,
         service_config=service_config,

diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py
@@ -102,18 +102,6 @@ def cloud_name_fixture() -> str:
     return "microstack"
 
 
-@pytest.fixture(autouse=True, name="clouds_yaml_path")
-def clouds_yaml_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path:
-    """Mocked clouds.yaml path.
-
-    Returns:
-        Path: Mocked clouds.yaml path.
-    """
-    clouds_yaml_path = tmp_path / "clouds.yaml"
-    monkeypatch.setattr("github_runner_manager.openstack_cloud.CLOUDS_YAML_PATH", clouds_yaml_path)
-    return clouds_yaml_path
-
-
 @pytest.fixture(name="clouds_yaml")
 def clouds_yaml_fixture(cloud_name: str) -> dict:
     """Testing clouds.yaml."""

diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -681,7 +681,8 @@ def test_on_config_changed_openstack_clouds_yaml(self, run, wt, mkdir, orm, rm):
                         "username": secrets.token_hex(16),
                         "user_domain_name": secrets.token_hex(16),
                         "password": secrets.token_hex(16),
-                    }
+                    },
+                    "region_name": secrets.token_hex(16),
                 }
             }
         }