Use API LB floating IPs according to template and label

Label master_lb_floating_ip_enabled is used if present.
Cluster template field floating_ip_enabled is used
as a fallback field because it has a wider scope (it also
means to add a floating ip to all worker nodes - not implemented
in the Magnum CAPI Helm driver).

When this option is disabled the CAPI Management cluster
must have an alternative route to reach the KubeAPI,
otherwise cluster creation will not succeed.

Closes-bug: #2085339
Co-Authored-By: Piotr Parczewski <piotr@stackhpc.com>
Change-Id: I48fda8cdb6e9de915528e47de40a771614c092d1
Signed-off-by: Dale Smith <dale@catalystcloud.nz>

magnum_capi_helm/driver.py

@@ -850,6 +850,11 @@ class Driver(driver.Driver):
             "cloudCredentialsSecretName": self._get_app_cred_name(cluster),
             "etcd": self._get_etcd_config(cluster),
             "apiServer": {
+                "associateFloatingIP": self._get_label_bool(
+                    cluster,
+                    "master_lb_floating_ip_enabled",
+                    True,
+                ),
                 "enableLoadBalancer": True,
                 "loadBalancerProvider": self._get_octavia_provider(cluster),
             },

magnum_capi_helm/tests/test_driver.py

@@ -1175,6 +1175,7 @@ class ClusterAPIDriverTest(base.DbTestCase):
             },
             "etcd": {},
             "apiServer": {
+                "associateFloatingIP": True,
                 "enableLoadBalancer": True,
                 "loadBalancerProvider": "amphora",
             },
@@ -3202,3 +3203,98 @@ class ClusterAPIDriverTest(base.DbTestCase):
         self.assertEqual(helm_values_default_ng.get("machineCountMin"), None)
         self.assertEqual(helm_values_default_ng.get("machineCountMax"), None)
         self.assertEqual(helm_values_default_ng.get("machineCount"), 3)
+
+    @mock.patch.object(
+        driver.Driver,
+        "_storageclass_definitions",
+        return_value=mock.ANY,
+    )
+    @mock.patch.object(driver.Driver, "_validate_allowed_flavor")
+    @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_nodegroup_master_lb_fip(
+        self,
+        mock_install,
+        mock_image,
+        mock_load,
+        mock_appcred,
+        mock_certs,
+        mock_get_net,
+        mock_validate_allowed_flavor,
+        mock_storageclasses,
+    ):
+        # Disabled master LB in labels.
+        self.cluster_obj.labels = {"master_lb_floating_ip_enabled": "false"}
+        self.cluster_obj.cluster_template.floating_ip_enabled = True
+        mock_image.return_value = (
+            "imageid1",
+            "1.27.4",
+            "ubuntu",
+        )
+        self.driver.create_cluster(self.context, self.cluster_obj, 10)
+        helm_install_values = mock_install.call_args[0][3]
+        apiserver_expected = {
+            "associateFloatingIP": False,
+            "enableLoadBalancer": True,
+            "loadBalancerProvider": "amphora",
+        }
+        self.assertEqual(apiserver_expected, helm_install_values["apiServer"])
+
+        # Enabled master LB in labels.
+        self.cluster_obj.labels = {"master_lb_floating_ip_enabled": "true"}
+        self.cluster_obj.cluster_template.floating_ip_enabled = False
+        mock_image.return_value = (
+            "imageid1",
+            "1.27.4",
+            "ubuntu",
+        )
+        self.driver.create_cluster(self.context, self.cluster_obj, 10)
+        helm_install_values = mock_install.call_args[0][3]
+        apiserver_expected = {
+            "associateFloatingIP": True,
+            "enableLoadBalancer": True,
+            "loadBalancerProvider": "amphora",
+        }
+        self.assertEqual(apiserver_expected, helm_install_values["apiServer"])
+
+        # Absent master lb label means default to True.
+        # cluster_template floating_ip_enabled is used only for FIP on nodes.
+        self.cluster_obj.labels = {}
+        self.cluster_obj.cluster_template.floating_ip_enabled = False
+        mock_image.return_value = (
+            "imageid1",
+            "1.27.4",
+            "ubuntu",
+        )
+        self.driver.create_cluster(self.context, self.cluster_obj, 10)
+        helm_install_values = mock_install.call_args[0][3]
+        apiserver_expected = {
+            "associateFloatingIP": True,
+            "enableLoadBalancer": True,
+            "loadBalancerProvider": "amphora",
+        }
+        self.assertEqual(apiserver_expected, helm_install_values["apiServer"])
+
+        # Absent master lb label.
+        # cluster_template floating_ip_enabled is used only for FIP on nodes.
+        self.cluster_obj.labels = {}
+        self.cluster_obj.cluster_template.floating_ip_enabled = True
+        mock_image.return_value = (
+            "imageid1",
+            "1.27.4",
+            "ubuntu",
+        )
+        self.driver.create_cluster(self.context, self.cluster_obj, 10)
+        helm_install_values = mock_install.call_args[0][3]
+        apiserver_expected = {
+            "associateFloatingIP": True,
+            "enableLoadBalancer": True,
+            "loadBalancerProvider": "amphora",
+        }
+        self.assertEqual(apiserver_expected, helm_install_values["apiServer"])

releasenotes/notes/fix-floating-ips-bd6ac8f8785503d1.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issue where floating IP disable setting from cluster template was
+    ignored by the driver. `LP#2085339
+    <https://bugs.launchpad.net/kolla-ansible/+bug/2085339>`__