From 4c32390bf88742e31c65b20d2719991803105035 Mon Sep 17 00:00:00 2001 From: Travis Holton Date: Tue, 17 Oct 2023 12:30:03 +1300 Subject: [PATCH] Retrieve volume types from cinder and use to make storageclasses Query cinder to find volume types and return set of storageclass dictionaries. CSI arguments as labels defaulting to config variables --- magnum_capi_helm/conf.py | 43 +++++++- magnum_capi_helm/driver.py | 83 ++++++++++++++- magnum_capi_helm/tests/test_driver.py | 139 +++++++++++++++++++++++++- 3 files changed, 262 insertions(+), 3 deletions(-) diff --git a/magnum_capi_helm/conf.py b/magnum_capi_helm/conf.py index c589443..99a4d32 100644 --- a/magnum_capi_helm/conf.py +++ b/magnum_capi_helm/conf.py @@ -58,7 +58,7 @@ ), cfg.StrOpt( "default_helm_chart_version", - default="0.1.4", + default="0.1.7", help=( "Version of the helm chart specified " "by the config: capi_driver.helm_chart_repo " @@ -66,6 +66,47 @@ "A cluster label can override this." ), ), + cfg.StrOpt( + "csi_cinder_default_volume_type", + help=("Default StorageClass volume type for persistent volumes."), + ), + cfg.StrOpt( + "csi_cinder_reclaim_policy", + default="Retain", + help=( + "Policy for reclaiming dynamically created " + "persistent volumes. Can be 'Retain' or 'Delete'." + ), + ), + cfg.BoolOpt( + "csi_cinder_allow_volume_expansion", + default=True, + help=( + "Allows the users to resize the volume by " + "editing the corresponding PVC object." + ), + ), + cfg.ListOpt( + "csi_cinder_allowed_topologies", + default=[], + help=( + "Select the Nodes where the application " + "Pods may be scheduled based on Node labels." + ), + ), + cfg.StrOpt( + "csi_cinder_fstype", + default="ext4", + help=("Filesystem type for persistent volumes."), + ), + cfg.StrOpt( + "csi_cinder_volume_binding_mode", + default="Immediate", + help=( + "The volumeBindingMode field controls when " + "volume binding and dynamic provisioning should occur." + ), + ), ] CONF = cfg.CONF diff --git a/magnum_capi_helm/driver.py b/magnum_capi_helm/driver.py index 2f55f31..9e2e633 100644 --- a/magnum_capi_helm/driver.py +++ b/magnum_capi_helm/driver.py @@ -549,6 +549,83 @@ def _get_fixed_network_id(self, context, cluster): context, network, source="name", target="id", external=False ) + def _get_csi_cinder_reclaim_policy(self, cluster): + return self._label( + cluster, + "csi_cinder_reclaim_policy", + CONF.capi_helm.csi_cinder_reclaim_policy, + ) + + def _get_csi_cinder_fstype(self, cluster): + return self._label( + cluster, + "csi_cinder_fstype", + CONF.capi_helm.csi_cinder_fstype, + ) + + def _get_csi_cinder_allow_volume_expansion(self, cluster): + return self._get_label_bool( + cluster, + "csi_cinder_allow_volume_expansion", + CONF.capi_helm.csi_cinder_allow_volume_expansion, + ) + + def _storageclass_definitions(self, context, cluster): + """Query cinder API to retrieve list of available volume types. + + @return dict(dict,list(dict)) containing storage classes + """ + LOG.debug("Retrieve volume types from cinder for StorageClasses.") + client = clients.OpenStackClients(context) + region_name = client.cinder_region_name() + c_client = client.cinder() + volume_types = [i.name for i in c_client.volume_types.list()] + # Use the default volume type if defined. Otherwise use the first + # type returned by cinder. + default_volume_type = CONF.capi_helm.csi_cinder_default_volume_type + LOG.debug( + f"Default volume type: {default_volume_type}" + f" Volume types: {volume_types}" + ) + if not default_volume_type: + default_volume_type = volume_types[0] + LOG.warning( + f"Default volume type not defined." + f" Using {default_volume_type}." + ) + elif default_volume_type not in volume_types: + # If default does not exist throw an error. + raise exception.MagnumException( + message=f"{default_volume_type} is not a" + " valid Cinder volume type." + ) + default_storage_class = {} + additional_storage_classes = [] + allow_expansion = self._get_csi_cinder_allow_volume_expansion(cluster) + reclaim_policy = self._get_csi_cinder_reclaim_policy(cluster) + allowed_topologies = CONF.capi_helm.csi_cinder_allowed_topologies + fstype = self._get_csi_cinder_fstype(cluster) + + for volume_type in volume_types: + storage_class = { + "name": volume_type, + "reclaimPolicy": reclaim_policy, + "allowVolumeExpansion": allow_expansion, + "availabilityZone": region_name, + "volumeType": volume_type, + "allowedTopologies": allowed_topologies, + "fstype": fstype, + "enabled": True, + } + if volume_type == default_volume_type: + default_storage_class = storage_class + else: + additional_storage_classes.append(storage_class) + return dict( + defaultStorageClass=default_storage_class, + additionalStorageClasses=additional_storage_classes, + ) + def _update_helm_release(self, context, cluster, nodegroups=None): if nodegroups is None: nodegroups = cluster.nodegroups @@ -559,7 +636,6 @@ def _update_helm_release(self, context, cluster, nodegroups=None): network_id = self._get_fixed_network_id(context, cluster) subnet_id = neutron.get_fixed_subnet_id(context, cluster.fixed_subnet) - values = { "kubernetesVersion": kube_version, "machineImageId": image_id, @@ -610,6 +686,11 @@ def _update_helm_release(self, context, cluster, nodegroups=None): if ng.role != NODE_GROUP_ROLE_CONTROLLER ], "addons": { + "openstack": { + "csiCinder": self._storageclass_definitions( + context, cluster + ) + }, "monitoring": { "enabled": self._get_monitoring_enabled(cluster) }, diff --git a/magnum_capi_helm/tests/test_driver.py b/magnum_capi_helm/tests/test_driver.py index 21bd294..e8c37b1 100644 --- a/magnum_capi_helm/tests/test_driver.py +++ b/magnum_capi_helm/tests/test_driver.py @@ -26,7 +26,6 @@ from magnum_capi_helm import helm from magnum_capi_helm import kubernetes - CONF = conf.CONF @@ -1173,6 +1172,7 @@ def _get_cluster_helm_standard_values(self): "monitoring": {"enabled": False}, "kubernetesDashboard": {"enabled": True}, "ingress": {"enabled": False}, + "openstack": {"csiCinder": mock.ANY}, }, "nodeGroups": [ { @@ -1188,6 +1188,11 @@ def _get_cluster_helm_standard_values(self): "machineSSHKeyName": None, } + @mock.patch.object( + driver.Driver, + "_storageclass_definitions", + return_value=mock.ANY, + ) @mock.patch.object(neutron, "get_network", autospec=True) @mock.patch.object( driver.Driver, "_ensure_certificate_secrets", autospec=True @@ -1204,6 +1209,7 @@ def test_create_cluster( mock_appcred, mock_certs, mock_get_net, + mock_storageclasses, ): mock_image.return_value = ( "imageid1", @@ -1244,6 +1250,11 @@ def test_create_cluster( ) self.assertEqual([], mock_get_net.call_args_list) + @mock.patch.object( + driver.Driver, + "_storageclass_definitions", + return_value=mock.ANY, + ) @mock.patch.object( driver.Driver, "_ensure_certificate_secrets", autospec=True ) @@ -1258,6 +1269,7 @@ def test_create_cluster_no_dns( mock_load, mock_appcred, mock_certs, + mock_storageclasses, ): mock_image.return_value = ("imageid1", "1.27.4", "ubuntu") mock_client = mock.MagicMock(spec=kubernetes.Client) @@ -1293,6 +1305,11 @@ def test_create_cluster_no_dns( self.driver, self.context, self.cluster_obj ) + @mock.patch.object( + driver.Driver, + "_storageclass_definitions", + return_value=mock.ANY, + ) @mock.patch.object( driver.Driver, "_ensure_certificate_secrets", autospec=True ) @@ -1307,6 +1324,7 @@ def test_create_cluster_boot_volume( mock_load, mock_appcred, mock_certs, + mock_storageclasses, ): mock_image.return_value = ("imageid1", "1.27.4", "ubuntu") mock_client = mock.MagicMock(spec=kubernetes.Client) @@ -1351,6 +1369,11 @@ def test_create_cluster_boot_volume( self.driver, self.context, self.cluster_obj ) + @mock.patch.object( + driver.Driver, + "_storageclass_definitions", + return_value=mock.ANY, + ) @mock.patch.object( driver.Driver, "_ensure_certificate_secrets", autospec=True ) @@ -1365,6 +1388,7 @@ def test_create_cluster_boot_volume_extra_network( mock_load, mock_appcred, mock_certs, + mock_storageclasses, ): mock_image.return_value = ( "imageid1", @@ -1425,6 +1449,11 @@ def test_create_cluster_boot_volume_extra_network( self.driver, self.context, self.cluster_obj ) + @mock.patch.object( + driver.Driver, + "_storageclass_definitions", + return_value=mock.ANY, + ) @mock.patch.object(driver.Driver, "_ensure_certificate_secrets") @mock.patch.object(driver.Driver, "_create_appcred_secret") @mock.patch.object(kubernetes.Client, "load") @@ -1437,6 +1466,7 @@ def test_create_cluster_with_keypair( mock_load, mock_appcred, mock_certs, + mock_storageclasses, ): mock_image.return_value = ( "imageid1", @@ -1470,6 +1500,11 @@ def test_create_cluster_with_keypair( mock_appcred.assert_called_once_with(self.context, self.cluster_obj) mock_certs.assert_called_once_with(self.context, self.cluster_obj) + @mock.patch.object( + driver.Driver, + "_storageclass_definitions", + return_value=mock.ANY, + ) @mock.patch.object(driver.Driver, "_ensure_certificate_secrets") @mock.patch.object(driver.Driver, "_create_appcred_secret") @mock.patch.object(kubernetes.Client, "load") @@ -1482,6 +1517,7 @@ def test_create_cluster_flatcar( mock_load, mock_appcred, mock_certs, + mock_storageclasses, ): mock_image.return_value = ( "imageid1", @@ -1511,6 +1547,11 @@ def test_create_cluster_flatcar( mock_appcred.assert_called_once_with(self.context, self.cluster_obj) mock_certs.assert_called_once_with(self.context, self.cluster_obj) + @mock.patch.object( + driver.Driver, + "_storageclass_definitions", + return_value=mock.ANY, + ) @mock.patch.object( driver.Driver, "_ensure_certificate_secrets", autospec=True ) @@ -1525,6 +1566,7 @@ def test_create_cluster_no_autoheal( mock_load, mock_appcred, mock_certs, + mock_storageclasses, ): mock_image.return_value = ("imageid1", "1.27.4", "ubuntu") mock_client = mock.MagicMock(spec=kubernetes.Client) @@ -1633,6 +1675,101 @@ def test_ensure_certificate_secrets( ) mock_labels.assert_called_with(self.cluster_obj) + @mock.patch("magnum.common.clients.OpenStackClients.cinder_region_name") + @mock.patch("magnum.common.clients.OpenStackClients.cinder") + def test_get_storage_classes(self, mock_cinder, mock_osc_rn): + CONF.capi_helm.csi_cinder_default_volume_type = "type3" + mock_osc_rn.return_value = "middle_earth_east" + mock_vol_type_1 = mock.MagicMock() + mock_vol_type_1.name = "type1" + mock_vol_type_2 = mock.MagicMock() + mock_vol_type_2.name = "type2" + mock_vol_type_3 = mock.MagicMock() + mock_vol_type_3.name = "type3" + mock_volume_types = mock.Mock() + mock_volume_types.list.return_value = [ + mock_vol_type_1, + mock_vol_type_2, + mock_vol_type_3, + ] + mock_cinder_client = mock.Mock() + mock_cinder_client.volume_types = mock_volume_types + mock_cinder.return_value = mock_cinder_client + storage_classes = self.driver._storageclass_definitions( + self.context, self.cluster_obj + ) + self.assertIsInstance(storage_classes, dict) + self.assertIsInstance(storage_classes["defaultStorageClass"], dict) + self.assertIsInstance( + storage_classes["additionalStorageClasses"], list + ) + self.assertEqual( + "type3", storage_classes["defaultStorageClass"]["volumeType"] + ) + self.assertEqual( + "middle_earth_east", + storage_classes["additionalStorageClasses"][0]["availabilityZone"], + ) + + @mock.patch("magnum.common.clients.OpenStackClients.cinder_region_name") + @mock.patch("magnum.common.clients.OpenStackClients.cinder") + def test_get_storage_class_volume_type_not_available( + self, mock_cinder, mock_osc_rn + ): + CONF.capi_helm.csi_cinder_default_volume_type = "type4" + mock_osc_rn.return_value = "middle_earth_east" + mock_vol_type_1 = mock.MagicMock() + mock_vol_type_1.name = "type1" + mock_vol_type_2 = mock.MagicMock() + mock_vol_type_2.name = "type2" + mock_vol_type_3 = mock.MagicMock() + mock_vol_type_3.name = "type3" + mock_volume_types = mock.Mock() + mock_volume_types.list.return_value = [ + mock_vol_type_1, + mock_vol_type_2, + mock_vol_type_3, + ] + mock_cinder_client = mock.Mock() + mock_cinder_client.volume_types = mock_volume_types + mock_cinder.return_value = mock_cinder_client + self.assertRaisesRegex( + exception.MagnumException, + r"not\sa\svalid\sCinder", + self.driver._storageclass_definitions, + self.context, + self.cluster_obj, + ) + + @mock.patch("magnum.common.clients.OpenStackClients.cinder_region_name") + @mock.patch("magnum.common.clients.OpenStackClients.cinder") + def test_get_storage_class_volume_type_not_defined( + self, mock_cinder, mock_osc_rn + ): + CONF.capi_helm.csi_cinder_default_volume_type = None + mock_osc_rn.return_value = "middle_earth_east" + mock_vol_type_1 = mock.MagicMock() + mock_vol_type_1.name = "type1" + mock_vol_type_2 = mock.MagicMock() + mock_vol_type_2.name = "type2" + mock_vol_type_3 = mock.MagicMock() + mock_vol_type_3.name = "type3" + mock_volume_types = mock.Mock() + mock_volume_types.list.return_value = [ + mock_vol_type_1, + mock_vol_type_2, + mock_vol_type_3, + ] + mock_cinder_client = mock.Mock() + mock_cinder_client.volume_types = mock_volume_types + mock_cinder.return_value = mock_cinder_client + storage_classes = self.driver._storageclass_definitions( + self.context, self.cluster_obj + ) + self.assertEqual( + "type1", storage_classes["defaultStorageClass"]["volumeType"] + ) + @mock.patch.object(helm.Client, "uninstall_release") def test_delete_cluster(self, mock_uninstall): self.driver.delete_cluster(self.context, self.cluster_obj)