From fd12c7a5e4830475d5e1259e32fa7c8dbecce89e Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Thu, 18 Apr 2024 11:36:01 +0200 Subject: [PATCH 1/7] Integrate runtime updates for etcd --- .../k8s/lib/charms/k8s/v0/k8sd_api_manager.py | 26 +++++++++++++-- charms/worker/k8s/src/charm.py | 32 +++++++++++++++++++ charms/worker/k8s/tests/unit/test_base.py | 1 + .../k8s/tests/unit/test_k8sd_api_manager.py | 21 ++++++++++-- tests/integration/test_etcd.py | 4 ++- 5 files changed, 78 insertions(+), 6 deletions(-) diff --git a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py index 633c6381..20e3a695 100644 --- a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py +++ b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py @@ -290,6 +290,24 @@ class UserFacingClusterConfig(BaseModel): metrics_server: MetricsServerConfig = Field(None, alias="metrics-server") +class UserFacingDatastoreConfig(BaseModel): + """Aggregated configuration model for the user-facing datastore aspects of a cluster. + + Attributes: + type: Type of the datastore. For runtime updates, this needs to be "external". + servers: Server addresses of the external datastore. + ca_crt: CA certificate of the external datastore cluster in PEM format. + client_crt: client certificate of the external datastore cluster in PEM format. + client_key: client key of the external datastore cluster in PEM format. + """ + + type: str = Field(None) + servers: List[str] = Field(None) + ca_crt: str = Field(None, alias="ca-crt") + client_crt: str = Field(None, alias="client-crt") + client_key: str = Field(None, alias="client-key") + + class BootstrapConfig(BaseModel): """Configuration model for bootstrapping a Canonical K8s cluster. @@ -341,9 +359,11 @@ class UpdateClusterConfigRequest(BaseModel): Attributes: config (Optional[UserFacingClusterConfig]): The cluster configuration. + datastore (Optional[UserFacingDatastoreConfig]): The clusters datastore configuration. """ - config: UserFacingClusterConfig + config: UserFacingClusterConfig = Field(None) + datastore: UserFacingDatastoreConfig = Field(None) class DatastoreStatus(BaseModel): @@ -351,11 +371,11 @@ class DatastoreStatus(BaseModel): Attributes: datastore_type (str): external or k8s-dqlite datastore - external_url: (str): list of external_urls + servers: (List(str)): list of server addresses of the external datastore cluster. """ datastore_type: str = Field(None, alias="type") - external_url: str = Field(None, alias="external-url") + servers: List[str] = Field(None, alias="servers") class ClusterStatus(BaseModel): diff --git a/charms/worker/k8s/src/charm.py b/charms/worker/k8s/src/charm.py index c346cc04..3ac1f31e 100755 --- a/charms/worker/k8s/src/charm.py +++ b/charms/worker/k8s/src/charm.py @@ -43,6 +43,7 @@ UnixSocketConnectionFactory, UpdateClusterConfigRequest, UserFacingClusterConfig, + UserFacingDatastoreConfig, ) from charms.kubernetes_libs.v0.etcd import EtcdReactiveRequires from charms.node_base import LabelMaker @@ -361,6 +362,36 @@ def _enable_functionalities(self): self.api_manager.update_cluster_config(update_request) + @on_error( + WaitingStatus("Ensure that the cluster configuration is up-to-date"), + InvalidResponseError, + K8sdConnectionError, + ) + def _ensure_cluster_config(self): + """Ensure that the cluster configuration is up-to-date + + The snap will detect any changes and only perform necessary steps. + There is no need to track changes in the charm. + """ + status.add(ops.MaintenanceStatus("Ensure cluster config")) + log.info("Ensure cluster-config") + + update_request = UpdateClusterConfigRequest() + + # TODO: Ensure other configs here as well. + + if self.config.get("datastore") == "etcd": + etcd_config = self.etcd.get_client_credentials() + update_request.datastore = UserFacingDatastoreConfig( + type="external", + servers=self.etcd.get_connection_string().split(","), + ca_crt=etcd_config.get("client_ca", ""), + client_crt=etcd_config.get("client_cert", ""), + client_key=etcd_config.get("client_key", ""), + ) + + self.api_manager.update_cluster_config(update_request) + def _get_scrape_jobs(self): """Retrieve the Prometheus Scrape Jobs. @@ -474,6 +505,7 @@ def _reconcile(self, event): self._create_cos_tokens() self._apply_cos_requirements() self._revoke_cluster_tokens() + self._ensure_cluster_config() self._join_cluster() self._configure_cos_integration() self._update_status() diff --git a/charms/worker/k8s/tests/unit/test_base.py b/charms/worker/k8s/tests/unit/test_base.py index 1317aa09..af021035 100644 --- a/charms/worker/k8s/tests/unit/test_base.py +++ b/charms/worker/k8s/tests/unit/test_base.py @@ -59,6 +59,7 @@ def mock_reconciler_handlers(harness): "_apply_cos_requirements", "_copy_internal_kubeconfig", "_revoke_cluster_tokens", + "_ensure_cluster_config", "_expose_ports", } diff --git a/charms/worker/k8s/tests/unit/test_k8sd_api_manager.py b/charms/worker/k8s/tests/unit/test_k8sd_api_manager.py index 875a5e9d..f8f62b82 100644 --- a/charms/worker/k8s/tests/unit/test_k8sd_api_manager.py +++ b/charms/worker/k8s/tests/unit/test_k8sd_api_manager.py @@ -25,6 +25,7 @@ UnixSocketHTTPConnection, UpdateClusterConfigRequest, UserFacingClusterConfig, + UserFacingDatastoreConfig, ) @@ -262,13 +263,29 @@ def test_update_cluster_config(self, mock_send_request): dns_config = DNSConfig(enabled=True) user_config = UserFacingClusterConfig(dns=dns_config) - request = UpdateClusterConfigRequest(config=user_config) + datastore = UserFacingDatastoreConfig( + type="external", + servers=["localhost:123"], + ca_crt="ca-crt", + client_crt="client-crt", + client_key="client-key", + ) + request = UpdateClusterConfigRequest(config=user_config, datastore=datastore) self.api_manager.update_cluster_config(request) mock_send_request.assert_called_once_with( "/1.0/k8sd/cluster/config", "PUT", EmptyResponse, - {"config": {"dns": {"enabled": True}}}, + { + "config": {"dns": {"enabled": True}}, + "datastore": { + "type": "external", + "servers": ["localhost:123"], + "ca-crt": "ca-crt", + "client-crt": "client-crt", + "client-key": "client-key", + }, + }, ) @patch("lib.charms.k8s.v0.k8sd_api_manager.K8sdAPIManager._send_request") diff --git a/tests/integration/test_etcd.py b/tests/integration/test_etcd.py index f2936d7d..6955eff1 100644 --- a/tests/integration/test_etcd.py +++ b/tests/integration/test_etcd.py @@ -40,4 +40,6 @@ async def test_etcd_datastore(kubernetes_cluster: model.Model): status = json.loads(result.results["stdout"]) assert status["ready"], "Cluster isn't ready" assert status["datastore"]["type"] == "external", "Not bootstrapped against etcd" - assert status["datastore"]["external-url"] == f"https://{etcd.public_address}:{etcd_port}" + assert status["datastore"]["servers"] == [ + f"https://{etcd.public_address}:{etcd_port}" + ] From adb04782a1c8947568dfb8c501bbe2fb92f1b67f Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Fri, 19 Apr 2024 10:45:02 +0200 Subject: [PATCH 2/7] fix tests --- .../k8s/lib/charms/k8s/v0/k8sd_api_manager.py | 4 +-- tests/integration/test_etcd.py | 26 ++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py index 20e3a695..c5c2e731 100644 --- a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py +++ b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py @@ -290,7 +290,7 @@ class UserFacingClusterConfig(BaseModel): metrics_server: MetricsServerConfig = Field(None, alias="metrics-server") -class UserFacingDatastoreConfig(BaseModel): +class UserFacingDatastoreConfig(BaseModel, allow_population_by_field_name=True): """Aggregated configuration model for the user-facing datastore aspects of a cluster. Attributes: @@ -647,7 +647,7 @@ def update_cluster_config(self, config: UpdateClusterConfigRequest): config (UpdateClusterConfigRequest): The cluster configuration. """ endpoint = "/1.0/k8sd/cluster/config" - body = config.dict(exclude_none=True) + body = config.dict(exclude_none=True, by_alias=True) self._send_request(endpoint, "PUT", EmptyResponse, body) def get_cluster_status(self) -> GetClusterStatusResponse: diff --git a/tests/integration/test_etcd.py b/tests/integration/test_etcd.py index 6955eff1..c0870c9a 100644 --- a/tests/integration/test_etcd.py +++ b/tests/integration/test_etcd.py @@ -40,6 +40,26 @@ async def test_etcd_datastore(kubernetes_cluster: model.Model): status = json.loads(result.results["stdout"]) assert status["ready"], "Cluster isn't ready" assert status["datastore"]["type"] == "external", "Not bootstrapped against etcd" - assert status["datastore"]["servers"] == [ - f"https://{etcd.public_address}:{etcd_port}" - ] + assert status["datastore"]["servers"] == [f"https://{etcd.public_address}:{etcd_port}"] + + +@pytest.mark.abort_on_fail +async def test_update_etcd_cluster(kubernetes_cluster: model.Model): + """Test that adding etcd clusters are propagated to the k8s cluster.""" + k8s: unit.Unit = kubernetes_cluster.applications["k8s"].units[0] + etcd = kubernetes_cluster.applications["etcd"] + + await etcd.add_unit() + await etcd.add_unit() + + expected_servers = [] + for u in etcd.units: + etcd_port = u.safe_data["ports"][0]["number"] + expected_servers.append(f"https://{u.public_address}:{etcd_port}") + + event = await k8s.run("k8s status --output-format json") + result = await event.wait() + status = json.loads(result.results["stdout"]) + assert status["ready"], "Cluster isn't ready" + assert status["datastore"]["type"] == "external", "Not bootstrapped against etcd" + assert set(status["datastore"]["servers"]) == set(expected_servers) From d956342d706f6db5869aed821a49dd37dc20f373 Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Fri, 19 Apr 2024 11:08:08 +0200 Subject: [PATCH 3/7] disable mypy check for unknown keyword --- charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py | 2 +- charms/worker/k8s/src/charm.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py index c5c2e731..938bc4c6 100644 --- a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py +++ b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py @@ -290,7 +290,7 @@ class UserFacingClusterConfig(BaseModel): metrics_server: MetricsServerConfig = Field(None, alias="metrics-server") -class UserFacingDatastoreConfig(BaseModel, allow_population_by_field_name=True): +class UserFacingDatastoreConfig(BaseModel, allow_population_by_field_name=True): # type: ignore[call-arg] """Aggregated configuration model for the user-facing datastore aspects of a cluster. Attributes: diff --git a/charms/worker/k8s/src/charm.py b/charms/worker/k8s/src/charm.py index 3ac1f31e..a226e5b7 100755 --- a/charms/worker/k8s/src/charm.py +++ b/charms/worker/k8s/src/charm.py @@ -368,7 +368,7 @@ def _enable_functionalities(self): K8sdConnectionError, ) def _ensure_cluster_config(self): - """Ensure that the cluster configuration is up-to-date + """Ensure that the cluster configuration is up-to-date. The snap will detect any changes and only perform necessary steps. There is no need to track changes in the charm. From 41716026c13306c2646b1474cdeec0022d214917 Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Mon, 22 Apr 2024 10:01:51 +0200 Subject: [PATCH 4/7] Update tests/integration/test_etcd.py Co-authored-by: Adam Dyess --- tests/integration/test_etcd.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_etcd.py b/tests/integration/test_etcd.py index c0870c9a..545d96ab 100644 --- a/tests/integration/test_etcd.py +++ b/tests/integration/test_etcd.py @@ -48,9 +48,10 @@ async def test_update_etcd_cluster(kubernetes_cluster: model.Model): """Test that adding etcd clusters are propagated to the k8s cluster.""" k8s: unit.Unit = kubernetes_cluster.applications["k8s"].units[0] etcd = kubernetes_cluster.applications["etcd"] - - await etcd.add_unit() - await etcd.add_unit() + count = 3 - len(etcd.units) + if count > 0: + await etcd.add_unit(count=count) + await kubernetes_cluster.wait_for_idle(status="active", timeout=20*60) expected_servers = [] for u in etcd.units: From b02522d50a843ce784a15e4808ff5a792f899950 Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Mon, 22 Apr 2024 10:02:28 +0200 Subject: [PATCH 5/7] Update charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py Co-authored-by: Adam Dyess --- .../worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py index 9a31eafa..833301a5 100644 --- a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py +++ b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py @@ -304,11 +304,11 @@ class UserFacingDatastoreConfig(BaseModel, allow_population_by_field_name=True): client_key: client key of the external datastore cluster in PEM format. """ - type: str = Field(None) - servers: List[str] = Field(None) - ca_crt: str = Field(None, alias="ca-crt") - client_crt: str = Field(None, alias="client-crt") - client_key: str = Field(None, alias="client-key") + type: Optional[str]] = Field(None) + servers: Optional[List[str]] = Field(None) + ca_crt: Optional[str] = Field(None, alias="ca-crt") + client_crt: Optional[str] = Field(None, alias="client-crt") + client_key: Optional[str] = Field(None, alias="client-key") class BootstrapConfig(BaseModel): From 08b0187387656e097332f58c7fea6eb50dd47a02 Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Mon, 22 Apr 2024 10:02:36 +0200 Subject: [PATCH 6/7] Update charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py Co-authored-by: Adam Dyess --- charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py index 833301a5..12352a87 100644 --- a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py +++ b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py @@ -367,8 +367,8 @@ class UpdateClusterConfigRequest(BaseModel): datastore (Optional[UserFacingDatastoreConfig]): The clusters datastore configuration. """ - config: UserFacingClusterConfig = Field(None) - datastore: UserFacingDatastoreConfig = Field(None) + config: Optional[UserFacingClusterConfig] = Field(None) + datastore: Optional[UserFacingDatastoreConfig] = Field(None) class NodeJoinConfig(BaseModel, allow_population_by_field_name=True): From ea65aa20f586519da3cd8ddd9693fb8af8d0111e Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Mon, 22 Apr 2024 11:22:03 +0200 Subject: [PATCH 7/7] fixup linting --- charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py | 2 +- tests/integration/test_etcd.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py index 12352a87..d928ceae 100644 --- a/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py +++ b/charms/worker/k8s/lib/charms/k8s/v0/k8sd_api_manager.py @@ -304,7 +304,7 @@ class UserFacingDatastoreConfig(BaseModel, allow_population_by_field_name=True): client_key: client key of the external datastore cluster in PEM format. """ - type: Optional[str]] = Field(None) + type: Optional[str] = Field(None) servers: Optional[List[str]] = Field(None) ca_crt: Optional[str] = Field(None, alias="ca-crt") client_crt: Optional[str] = Field(None, alias="client-crt") diff --git a/tests/integration/test_etcd.py b/tests/integration/test_etcd.py index 545d96ab..d0e3c5f0 100644 --- a/tests/integration/test_etcd.py +++ b/tests/integration/test_etcd.py @@ -51,7 +51,7 @@ async def test_update_etcd_cluster(kubernetes_cluster: model.Model): count = 3 - len(etcd.units) if count > 0: await etcd.add_unit(count=count) - await kubernetes_cluster.wait_for_idle(status="active", timeout=20*60) + await kubernetes_cluster.wait_for_idle(status="active", timeout=20 * 60) expected_servers = [] for u in etcd.units: