From 7fd24aa332e8f66ae00c78c262599b2dcdc4c0bc Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Tue, 26 Mar 2024 18:40:13 +0000 Subject: [PATCH 1/9] Fix availability zones and topology configuration --- charmcraft.yaml | 8 +++----- config.yaml | 6 ++++++ src/storage_manifests.py | 28 +++++++++++++++++++++++++--- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 9477a50..57a2d51 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -12,13 +12,11 @@ bases: - name: ubuntu channel: "20.04" architectures: - - amd64 - - arm64 + - "amd64" - name: ubuntu channel: "22.04" - architectures: - - amd64 - - arm64 + architectures: + - "amd64" parts: charm: build-packages: diff --git a/config.yaml b/config.yaml index 1da1345..2aceeb2 100644 --- a/config.yaml +++ b/config.yaml @@ -37,3 +37,9 @@ options: description: | Availability zone to use with Cinder CSI. This is passed through to the parameters.availability field of the csi-cinder-default StorageClass. + + topology: + type: boolean + default: True + description: | + Whether to enable the Cinder CSI topology awareness \ No newline at end of file diff --git a/src/storage_manifests.py b/src/storage_manifests.py index ec16599..b61c1bd 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -63,12 +63,12 @@ def __call__(self) -> Optional[AnyResource]: metadata=dict(name=storage_name), provisioner="cinder.csi.openstack.org", reclaimPolicy="Delete", - volumeBindingMode="WaitForFirstConsumer", + volumeBindingMode="WaitForFirstConsumer" ) ) - + if az := self.manifests.config.get("availability-zone"): - sc.parameters.availability = az + sc.parameters = dict(availability=az) return sc @@ -105,6 +105,7 @@ def __init__(self, charm, charm_config, kube_control, integrator): ConfigRegistry(self), CreateStorageClass(self, "default"), # creates csi-cinder-default UpdateSecrets(self), # update secrets + UpdateControllerPlugin, ], ) self.integrator = integrator @@ -142,3 +143,24 @@ def evaluate(self) -> Optional[str]: if not self.config.get(prop): return f"Storage manifests waiting for definition of {prop}" return None + +class UpdateControllerPlugin(Patch): + """Update the controller args in Deployments.""" + + def __call__(self, obj): + """Update the controller args in Deployments.""" + if not any( + [ + (obj.kind == "DaemonSet" and obj.metadata.name == "csi-cinder-nodeplugin"), + (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"), + ] + ): + return + + for container in obj.spec.template.spec.containers: + if container.name == "csi-provisioner": + for i, val in enumerate(container.args): + if "feature-gates" in val.lower(): + container.args[i] = f"feature-gates=Topology={self.manifests.config.get('topology')}" + log.info(f"Disabling cinder topology awareness") + From 071b46eed04cf3d275f38c9f89d6f2e19c51ed97 Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 08:07:02 +0000 Subject: [PATCH 2/9] Review batch 1 --- charmcraft.yaml | 8 +++++--- src/storage_manifests.py | 9 ++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 57a2d51..9477a50 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -12,11 +12,13 @@ bases: - name: ubuntu channel: "20.04" architectures: - - "amd64" + - amd64 + - arm64 - name: ubuntu channel: "22.04" - architectures: - - "amd64" + architectures: + - amd64 + - arm64 parts: charm: build-packages: diff --git a/src/storage_manifests.py b/src/storage_manifests.py index b61c1bd..8d792fe 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -105,7 +105,7 @@ def __init__(self, charm, charm_config, kube_control, integrator): ConfigRegistry(self), CreateStorageClass(self, "default"), # creates csi-cinder-default UpdateSecrets(self), # update secrets - UpdateControllerPlugin, + UpdateControllerPlugin(self), ], ) self.integrator = integrator @@ -151,7 +151,6 @@ def __call__(self, obj): """Update the controller args in Deployments.""" if not any( [ - (obj.kind == "DaemonSet" and obj.metadata.name == "csi-cinder-nodeplugin"), (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"), ] ): @@ -161,6 +160,6 @@ def __call__(self, obj): if container.name == "csi-provisioner": for i, val in enumerate(container.args): if "feature-gates" in val.lower(): - container.args[i] = f"feature-gates=Topology={self.manifests.config.get('topology')}" - log.info(f"Disabling cinder topology awareness") - + topology = str(self.manifests.config.get("topology")).lower() + container.args[i] = f"feature-gates=Topology={topology}" + log.info(f"Configuring cinder topology awareness=%s", topology) \ No newline at end of file From 74a1cfb19513295ecfb51f6ec904cdb8bad75183 Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 08:16:21 +0000 Subject: [PATCH 3/9] Re-add comma --- src/storage_manifests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage_manifests.py b/src/storage_manifests.py index 8d792fe..6e267ce 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -63,7 +63,7 @@ def __call__(self) -> Optional[AnyResource]: metadata=dict(name=storage_name), provisioner="cinder.csi.openstack.org", reclaimPolicy="Delete", - volumeBindingMode="WaitForFirstConsumer" + volumeBindingMode="WaitForFirstConsumer", ) ) From fa990fca1eb33e1bb99e0246951712e24cd3f391 Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 15:50:07 +0000 Subject: [PATCH 4/9] Review changes batch #2 --- src/storage_manifests.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/storage_manifests.py b/src/storage_manifests.py index 6e267ce..c882100 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -66,7 +66,6 @@ def __call__(self) -> Optional[AnyResource]: volumeBindingMode="WaitForFirstConsumer", ) ) - if az := self.manifests.config.get("availability-zone"): sc.parameters = dict(availability=az) return sc @@ -77,12 +76,7 @@ class UpdateSecrets(Patch): def __call__(self, obj): """Update the secret volume spec in daemonsets and deployments.""" - if not any( - [ - (obj.kind == "DaemonSet" and obj.metadata.name == "csi-cinder-nodeplugin"), - (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"), - ] - ): + if not (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"): return for volume in obj.spec.template.spec.volumes: From ebf6f29add26060095222618c0db25bacc065845 Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 16:18:12 +0000 Subject: [PATCH 5/9] Human Linter fixes --- src/storage_manifests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage_manifests.py b/src/storage_manifests.py index c882100..5db81be 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -138,6 +138,7 @@ def evaluate(self) -> Optional[str]: return f"Storage manifests waiting for definition of {prop}" return None + class UpdateControllerPlugin(Patch): """Update the controller args in Deployments.""" @@ -156,4 +157,4 @@ def __call__(self, obj): if "feature-gates" in val.lower(): topology = str(self.manifests.config.get("topology")).lower() container.args[i] = f"feature-gates=Topology={topology}" - log.info(f"Configuring cinder topology awareness=%s", topology) \ No newline at end of file + log.info("Configuring cinder topology awareness=%s", topology) From 72ea56e5db0c6dbc479e6f683bb7f2a530718e53 Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 16:53:19 +0000 Subject: [PATCH 6/9] Revert "Review changes batch #2" This reverts commit fa990fca1eb33e1bb99e0246951712e24cd3f391. --- src/storage_manifests.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/storage_manifests.py b/src/storage_manifests.py index 5db81be..b7d1ae5 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -66,6 +66,7 @@ def __call__(self) -> Optional[AnyResource]: volumeBindingMode="WaitForFirstConsumer", ) ) + if az := self.manifests.config.get("availability-zone"): sc.parameters = dict(availability=az) return sc @@ -76,7 +77,12 @@ class UpdateSecrets(Patch): def __call__(self, obj): """Update the secret volume spec in daemonsets and deployments.""" - if not (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"): + if not any( + [ + (obj.kind == "DaemonSet" and obj.metadata.name == "csi-cinder-nodeplugin"), + (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"), + ] + ): return for volume in obj.spec.template.spec.volumes: From a8fcede3e8cdb487be4ae7d1e60812b69e7d5711 Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 16:54:33 +0000 Subject: [PATCH 7/9] Update controller plugin --- src/storage_manifests.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/storage_manifests.py b/src/storage_manifests.py index b7d1ae5..c16ba1d 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -150,11 +150,7 @@ class UpdateControllerPlugin(Patch): def __call__(self, obj): """Update the controller args in Deployments.""" - if not any( - [ - (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"), - ] - ): + if not (obj.kind == "Deployment" and obj.metadata.name == "csi-cinder-controllerplugin"): return for container in obj.spec.template.spec.containers: From cb0336f17a51763d39c299a70a1459274cdd8dbd Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 16:55:28 +0000 Subject: [PATCH 8/9] Remove newline --- src/storage_manifests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage_manifests.py b/src/storage_manifests.py index c16ba1d..5a6a724 100644 --- a/src/storage_manifests.py +++ b/src/storage_manifests.py @@ -66,7 +66,6 @@ def __call__(self) -> Optional[AnyResource]: volumeBindingMode="WaitForFirstConsumer", ) ) - if az := self.manifests.config.get("availability-zone"): sc.parameters = dict(availability=az) return sc From faa4987d032e5154df54023708abeb25d7d3fcfb Mon Sep 17 00:00:00 2001 From: Peter Jose De Sousa Date: Wed, 27 Mar 2024 17:25:50 +0000 Subject: [PATCH 9/9] Add expected output for tests --- tests/unit/test_charm.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index b6c55af..8ef0308 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -164,6 +164,7 @@ def test_waits_for_kube_control(mock_create_kubeconfig, harness, caplog): "Creating storage class csi-cinder-default", "Setting secret for DaemonSet/csi-cinder-nodeplugin", "Setting secret for Deployment/csi-cinder-controllerplugin", + "Configuring cinder topology awareness=true", } caplog.clear()