Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/openstack connection creation #378

Merged
merged 41 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
a9dc05d
Update to use openstack credential directly
yhaliaw Sep 13, 2024
8d72fbb
Merge branch 'main' into fix/openstack-connection-creation
yhaliaw Sep 18, 2024
b989fc8
Remove test for removed code
yhaliaw Sep 18, 2024
0c26793
Fix the unit tests
yhaliaw Sep 19, 2024
9c3595a
Fix dict values type error
yhaliaw Sep 23, 2024
d01afb1
Merge branch 'main' into fix/openstack-connection-creation
yhaliaw Sep 23, 2024
d9eb66a
Fix dependency issues
yhaliaw Sep 23, 2024
1aa9819
Try new deps
yhaliaw Sep 23, 2024
c8edf13
Try new deps
yhaliaw Sep 23, 2024
4b5d6c3
Merge branch 'main' into fix/openstack-connection-creation
yhaliaw Sep 24, 2024
20fbd20
Fix merge issue
yhaliaw Sep 24, 2024
13da6e5
Remove a todo comment
yhaliaw Sep 24, 2024
2c9eeef
Add debugging
yhaliaw Sep 24, 2024
6100734
Fix merge issue
yhaliaw Sep 24, 2024
e0364f3
Merge branch 'main' into fix/openstack-connection-creation
yhaliaw Sep 25, 2024
89e827a
Remove debug
yhaliaw Sep 25, 2024
4d2ec32
Fix clouds.yaml access
yhaliaw Sep 26, 2024
b4b0dea
Fix clouds.yaml dict access
yhaliaw Sep 26, 2024
6ee44e4
Add comments on the auth fields of clouds.yaml
yhaliaw Sep 26, 2024
f9ad319
Fix fmt
yhaliaw Sep 26, 2024
c1e9149
Add reactive debug test
yhaliaw Sep 27, 2024
4236ed4
Debug reactive
yhaliaw Sep 27, 2024
c4be52e
Merge branch 'main' into fix/openstack-connection-creation
yhaliaw Sep 30, 2024
8b2ce60
Try if region_name
yhaliaw Sep 30, 2024
8dde4fe
Print deubgging info
yhaliaw Sep 30, 2024
6b40318
Merge branch 'main' into fix/openstack-connection-creation
cbartz Oct 16, 2024
4fef855
add region_name to _OpenStackAuth
cbartz Oct 16, 2024
949cdfb
add region_name to _OpenStackCloud
cbartz Oct 16, 2024
4750dbd
Merge branch 'main' into fix/openstack-connection-creation
cbartz Oct 16, 2024
852aaca
run other integration tests
cbartz Oct 16, 2024
76ba870
fix test_runner_manager_openstack
cbartz Oct 16, 2024
dce6256
fix unit tests
cbartz Oct 16, 2024
f64b7a7
revert integration_test.yaml
cbartz Oct 16, 2024
742b650
fix unit test
cbartz Oct 16, 2024
229442f
lint
cbartz Oct 16, 2024
206c3ec
update changelog
cbartz Oct 17, 2024
3d6cf9e
increase image-builder deploy timeout
cbartz Oct 17, 2024
7463e4f
pin correct commit
cbartz Oct 17, 2024
4d6a286
final new line
cbartz Oct 17, 2024
1c3aa06
remove todo
cbartz Oct 17, 2024
459e754
lint
cbartz Oct 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
34 changes: 17 additions & 17 deletions src-docs/charm_state.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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__`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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__`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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`

Expand Down Expand Up @@ -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__`

Expand Down
16 changes: 11 additions & 5 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
12 changes: 2 additions & 10 deletions src/charm_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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")
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
29 changes: 21 additions & 8 deletions tests/integration/test_runner_manager_openstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
12 changes: 0 additions & 12 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}
Expand Down
Loading
Loading