diff --git a/magnum_capi_helm/driver.py b/magnum_capi_helm/driver.py index c653e39..d38b227 100644 --- a/magnum_capi_helm/driver.py +++ b/magnum_capi_helm/driver.py @@ -480,6 +480,16 @@ def _label(self, cluster, key, default): # NOTE(johngarbutt): filtering untrusted user input return re.sub(r"[^a-zA-Z0-9\.\-\/ ]+", "", raw) + def _get_label_bool(self, cluster, label, default): + cluster_label = self._label(cluster, label, "") + if not cluster_label: + return default + if default: + # Default is on, so return for any value except "false" + return cluster_label != "false" + # Default is False, so only "true" responds with True + return cluster_label == "true" + def _get_chart_version(self, cluster): version = cluster.cluster_template.labels.get( "capi_helm_chart_version", @@ -540,15 +550,16 @@ def _get_dns_nameservers(self, cluster): return None def _get_monitoring_enabled(self, cluster): - mon_label = self._label(cluster, "monitoring_enabled", "") - # NOTE(mkjpryor) default of, like heat driver, - # as requires cinder and takes a while - return mon_label == "true" + # NOTE(mkjpryor) default off, like heat driver, + # as requires cinder and takes a while + return self._get_label_bool(cluster, "monitoring_enabled", False) def _get_kube_dash_enabled(self, cluster): - kube_dash_label = self._label(cluster, "kube_dashboard_enabled", "") - # NOTE(mkjpryor) default on, like the heat driver - return kube_dash_label != "false" + # NOTE(mkjpryor) default on, like the heat driver + return self._get_label_bool(cluster, "kube_dashboard_enabled", True) + + def _get_autoheal_enabled(self, cluster): + return self._get_label_bool(cluster, "auto_healing_enabled", True) def _get_fixed_network_id(self, context, cluster): network = cluster.fixed_network @@ -603,6 +614,14 @@ def _update_helm_release(self, context, cluster, nodegroups=None): "controlPlane": { "machineFlavor": cluster.master_flavor_id, "machineCount": cluster.master_count, + "healthCheck": { + "enabled": self._get_autoheal_enabled(cluster), + }, + }, + "nodeGroupDefaults": { + "healthCheck": { + "enabled": self._get_autoheal_enabled(cluster), + }, }, "nodeGroups": [ { diff --git a/magnum_capi_helm/tests/test_driver.py b/magnum_capi_helm/tests/test_driver.py index a863125..246cf16 100644 --- a/magnum_capi_helm/tests/test_driver.py +++ b/magnum_capi_helm/tests/test_driver.py @@ -1136,7 +1136,7 @@ def test_get_chart_version_from_template(self): self.assertEqual("1.42.0", version) - def get_cluster_helm_standard_values(self): + def _get_cluster_helm_standard_values(self): """Return standard helm values which can be modified for tests. There is little point in multiple tests writing the same dictionary @@ -1166,6 +1166,7 @@ def get_cluster_helm_standard_values(self): "controlPlane": { "machineFlavor": "flavor_small", "machineCount": 3, + "healthCheck": {"enabled": True}, }, "addons": { "monitoring": {"enabled": False}, @@ -1180,15 +1181,20 @@ def get_cluster_helm_standard_values(self): }, ], "osDistro": "ubuntu", + "nodeGroupDefaults": { + "healthCheck": {"enabled": True}, + }, "machineSSHKeyName": None, } - @mock.patch.object(neutron, "get_network") - @mock.patch.object(driver.Driver, "_ensure_certificate_secrets") - @mock.patch.object(driver.Driver, "_create_appcred_secret") - @mock.patch.object(kubernetes.Client, "load") - @mock.patch.object(driver.Driver, "_get_image_details") - @mock.patch.object(helm.Client, "install_or_upgrade") + @mock.patch.object(neutron, "get_network", autospec=True) + @mock.patch.object( + driver.Driver, "_ensure_certificate_secrets", autospec=True + ) + @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True) + @mock.patch.object(kubernetes.Client, "load", autospec=True) + @mock.patch.object(driver.Driver, "_get_image_details", autospec=True) + @mock.patch.object(helm.Client, "install_or_upgrade", autospec=True) def test_create_cluster( self, mock_install, @@ -1211,28 +1217,39 @@ def test_create_cluster( self.driver.create_cluster(self.context, self.cluster_obj, 10) - expected_values = self.get_cluster_helm_standard_values() + expected_values = self._get_cluster_helm_standard_values() mock_install.assert_called_once_with( + self.driver._helm_client, "cluster-example-a-111111111111", "openstack-cluster", - expected_values, + mock.ANY, # NOTE(dalees): Compared separately for improved diff repo=CONF.capi_helm.helm_chart_repo, version=CONF.capi_helm.default_helm_chart_version, namespace="magnum-fakeproject", ) + + helm_install_values = mock_install.call_args[0][3] + self.assertDictEqual(helm_install_values, expected_values) + mock_client.ensure_namespace.assert_called_once_with( "magnum-fakeproject" ) - mock_appcred.assert_called_once_with(self.context, self.cluster_obj) - mock_certs.assert_called_once_with(self.context, self.cluster_obj) + mock_appcred.assert_called_once_with( + self.driver, self.context, self.cluster_obj + ) + mock_certs.assert_called_once_with( + self.driver, self.context, self.cluster_obj + ) self.assertEqual([], mock_get_net.call_args_list) - @mock.patch.object(driver.Driver, "_ensure_certificate_secrets") - @mock.patch.object(driver.Driver, "_create_appcred_secret") - @mock.patch.object(kubernetes.Client, "load") - @mock.patch.object(driver.Driver, "_get_image_details") - @mock.patch.object(helm.Client, "install_or_upgrade") + @mock.patch.object( + driver.Driver, "_ensure_certificate_secrets", autospec=True + ) + @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True) + @mock.patch.object(kubernetes.Client, "load", autospec=True) + @mock.patch.object(driver.Driver, "_get_image_details", autospec=True) + @mock.patch.object(helm.Client, "install_or_upgrade", autospec=True) def test_create_cluster_no_dns( self, mock_install, @@ -1249,25 +1266,31 @@ def test_create_cluster_no_dns( self.driver.create_cluster(self.context, self.cluster_obj, 10) - expected_values = self.get_cluster_helm_standard_values() + expected_values = self._get_cluster_helm_standard_values() expected_values["clusterNetworking"]["dnsNameservers"] = None - helm_install_values = mock_install.call_args[0][2] - self.assertDictEqual(helm_install_values, expected_values) - mock_install.assert_called_once_with( + self.driver._helm_client, "cluster-example-a-111111111111", "openstack-cluster", - expected_values, + mock.ANY, repo=CONF.capi_helm.helm_chart_repo, version=CONF.capi_helm.default_helm_chart_version, namespace="magnum-fakeproject", ) + + helm_install_values = mock_install.call_args[0][3] + self.assertDictEqual(helm_install_values, expected_values) + mock_client.ensure_namespace.assert_called_once_with( "magnum-fakeproject" ) - mock_appcred.assert_called_once_with(self.context, self.cluster_obj) - mock_certs.assert_called_once_with(self.context, self.cluster_obj) + mock_appcred.assert_called_once_with( + self.driver, self.context, self.cluster_obj + ) + mock_certs.assert_called_once_with( + self.driver, self.context, self.cluster_obj + ) @mock.patch.object( driver.Driver, "_ensure_certificate_secrets", autospec=True @@ -1293,62 +1316,30 @@ def test_create_cluster_boot_volume( self.driver.create_cluster(self.context, self.cluster_obj, 10) - app_cred_name = "cluster-example-a-111111111111-cloud-credentials" - ext_net_id = self.cluster_obj.cluster_template.external_network_id + # Get standard values and modify them to match this test + expected_values = self._get_cluster_helm_standard_values() + expected_values["controlPlane"]["machineRootVolume"] = { + "volumeType": "nvme", + "diskSize": 12, + } + expected_values["nodeGroupDefaults"]["machineRootVolume"] = { + "volumeType": "nvme", + "diskSize": 12, + } + mock_install.assert_called_once_with( self.driver._helm_client, "cluster-example-a-111111111111", "openstack-cluster", - { - "kubernetesVersion": "1.27.4", - "machineImageId": "imageid1", - "cloudCredentialsSecretName": app_cred_name, - "clusterNetworking": { - "externalNetworkId": ext_net_id, - "internalNetwork": { - "networkFilter": None, - "subnetFilter": None, - "nodeCidr": "10.0.0.0/24", - }, - "dnsNameservers": ["8.8.1.1"], - }, - "apiServer": { - "enableLoadBalancer": True, - "loadBalancerProvider": "amphora", - }, - "osDistro": "ubuntu", - "controlPlane": { - "machineFlavor": "flavor_small", - "machineCount": 3, - "machineRootVolume": { - "volumeType": "nvme", - "diskSize": 12, - }, - }, - "addons": { - "monitoring": {"enabled": False}, - "kubernetesDashboard": {"enabled": True}, - "ingress": {"enabled": False}, - }, - "nodeGroups": [ - { - "name": "test-worker", - "machineFlavor": "flavor_medium", - "machineCount": 3, - }, - ], - "nodeGroupDefaults": { - "machineRootVolume": { - "volumeType": "nvme", - "diskSize": 12, - }, - }, - "machineSSHKeyName": None, - }, + mock.ANY, repo=CONF.capi_helm.helm_chart_repo, version=CONF.capi_helm.default_helm_chart_version, namespace="magnum-fakeproject", ) + + helm_install_values = mock_install.call_args[0][3] + self.assertDictEqual(helm_install_values, expected_values) + mock_client.ensure_namespace.assert_called_once_with( "magnum-fakeproject" ) @@ -1389,73 +1380,40 @@ def test_create_cluster_boot_volume_extra_network( self.driver.create_cluster(self.context, self.cluster_obj, 10) - app_cred_name = "cluster-example-a-111111111111-cloud-credentials" - ext_net_id = self.cluster_obj.cluster_template.external_network_id + expected_values = self._get_cluster_helm_standard_values() + expected_values["controlPlane"]["machineRootVolume"] = { + "volumeType": "nvme", + "diskSize": 12, + } + expected_values["nodeGroupDefaults"]["machineRootVolume"] = { + "volumeType": "nvme", + "diskSize": 12, + } + expected_values["nodeGroupDefaults"]["machineNetworking"] = { + "ports": [ + {}, + { + "network": { + "name": "foo", + }, + "securityGroups": [], + }, + ], + } + mock_install.assert_called_once_with( self.driver._helm_client, "cluster-example-a-111111111111", "openstack-cluster", - { - "kubernetesVersion": "1.27.4", - "machineImageId": "imageid1", - "cloudCredentialsSecretName": app_cred_name, - "clusterNetworking": { - "externalNetworkId": ext_net_id, - "internalNetwork": { - "networkFilter": None, - "subnetFilter": None, - "nodeCidr": "10.0.0.0/24", - }, - "dnsNameservers": ["8.8.1.1"], - }, - "apiServer": { - "enableLoadBalancer": True, - "loadBalancerProvider": "amphora", - }, - "osDistro": "ubuntu", - "controlPlane": { - "machineFlavor": "flavor_small", - "machineCount": 3, - "machineRootVolume": { - "volumeType": "nvme", - "diskSize": 12, - }, - }, - "addons": { - "monitoring": {"enabled": False}, - "kubernetesDashboard": {"enabled": True}, - "ingress": {"enabled": False}, - }, - "nodeGroups": [ - { - "name": "test-worker", - "machineFlavor": "flavor_medium", - "machineCount": 3, - }, - ], - "nodeGroupDefaults": { - "machineNetworking": { - "ports": [ - {}, - { - "network": { - "name": "foo", - }, - "securityGroups": [], - }, - ], - }, - "machineRootVolume": { - "volumeType": "nvme", - "diskSize": 12, - }, - }, - "machineSSHKeyName": None, - }, + mock.ANY, repo=CONF.capi_helm.helm_chart_repo, version=CONF.capi_helm.default_helm_chart_version, namespace="magnum-fakeproject", ) + + helm_install_values = mock_install.call_args[0][3] + self.assertDictEqual(helm_install_values, expected_values) + mock_client.ensure_namespace.assert_called_once_with( "magnum-fakeproject" ) @@ -1491,17 +1449,20 @@ def test_create_cluster_with_keypair( self.driver.create_cluster(self.context, self.cluster_obj, 10) - expected_values = self.get_cluster_helm_standard_values() + expected_values = self._get_cluster_helm_standard_values() expected_values["machineSSHKeyName"] = "kp1" mock_install.assert_called_once_with( "cluster-example-a-111111111111", "openstack-cluster", - expected_values, + mock.ANY, repo=CONF.capi_helm.helm_chart_repo, version=CONF.capi_helm.default_helm_chart_version, namespace="magnum-fakeproject", ) + helm_install_values = mock_install.call_args[0][2] + self.assertDictEqual(helm_install_values, expected_values) + mock_client.ensure_namespace.assert_called_once_with( "magnum-fakeproject" ) @@ -1531,7 +1492,7 @@ def test_create_cluster_flatcar( self.driver.create_cluster(self.context, self.cluster_obj, 10) - expected_values = self.get_cluster_helm_standard_values() + expected_values = self._get_cluster_helm_standard_values() expected_values["osDistro"] = "flatcar" mock_install.assert_called_once_with( @@ -1542,12 +1503,65 @@ def test_create_cluster_flatcar( version=CONF.capi_helm.default_helm_chart_version, namespace="magnum-fakeproject", ) + mock_client.ensure_namespace.assert_called_once_with( "magnum-fakeproject" ) 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, "_ensure_certificate_secrets", autospec=True + ) + @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True) + @mock.patch.object(kubernetes.Client, "load", autospec=True) + @mock.patch.object(driver.Driver, "_get_image_details", autospec=True) + @mock.patch.object(helm.Client, "install_or_upgrade", autospec=True) + def test_create_cluster_no_autoheal( + self, + mock_install, + mock_image, + mock_load, + mock_appcred, + mock_certs, + ): + mock_image.return_value = ("imageid1", "1.27.4", "ubuntu") + mock_client = mock.MagicMock(spec=kubernetes.Client) + mock_load.return_value = mock_client + + self.cluster_obj.cluster_template.labels[ + "auto_healing_enabled" + ] = "false" + + self.driver.create_cluster(self.context, self.cluster_obj, 10) + + # Get standard values and modify them to match this test + expected_values = self._get_cluster_helm_standard_values() + expected_values["controlPlane"]["healthCheck"]["enabled"] = False + expected_values["nodeGroupDefaults"]["healthCheck"]["enabled"] = False + + mock_install.assert_called_once_with( + self.driver._helm_client, + "cluster-example-a-111111111111", + "openstack-cluster", + mock.ANY, + repo=CONF.capi_helm.helm_chart_repo, + version=CONF.capi_helm.default_helm_chart_version, + namespace="magnum-fakeproject", + ) + helm_install_values = mock_install.call_args[0][3] + self.assertDictEqual(helm_install_values, expected_values) + + mock_client.ensure_namespace.assert_called_once_with( + "magnum-fakeproject" + ) + mock_appcred.assert_called_once_with( + self.driver, self.context, self.cluster_obj + ) + mock_certs.assert_called_once_with( + self.driver, self.context, self.cluster_obj + ) + @mock.patch.object(app_creds, "get_app_cred_string_data") @mock.patch.object(kubernetes.Client, "load") def test_create_appcred_secret(self, mock_load, mock_sd):