Merge pull request kubernetes-sigs#1728 from alexandrevilain/fix/do-not-delete-byo-floating-ip

🐛 Don't remove BYO api server loadbalancer floating IP

Author: k8s-ci-robot

api/v1alpha7/openstackcluster_webhook.go

@@ -134,6 +134,12 @@ func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warn
 	old.Spec.AllowAllInClusterTraffic = false
 	r.Spec.AllowAllInClusterTraffic = false
 
+	// Allow change on the spec.APIServerFloatingIP only if it matches the current api server loadbalancer IP.
+	if old.Status.APIServerLoadBalancer != nil && r.Spec.APIServerFloatingIP == old.Status.APIServerLoadBalancer.IP {
+		r.Spec.APIServerFloatingIP = ""
+		old.Spec.APIServerFloatingIP = ""
+	}
+
 	if !reflect.DeepEqual(old.Spec, r.Spec) {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified"))
 	}

api/v1alpha7/openstackcluster_webhook_test.go

@@ -295,6 +295,54 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "Changing OpenStackCluster.Spec.APIServerFloatingIP is allowed when it matches the current api server loadbalancer IP",
+			oldTemplate: &OpenStackCluster{
+				Spec: OpenStackClusterSpec{
+					APIServerFloatingIP: "",
+				},
+				Status: OpenStackClusterStatus{
+					APIServerLoadBalancer: &LoadBalancer{
+						IP: "1.2.3.4",
+					},
+				},
+			},
+			newTemplate: &OpenStackCluster{
+				Spec: OpenStackClusterSpec{
+					APIServerFloatingIP: "1.2.3.4",
+				},
+				Status: OpenStackClusterStatus{
+					APIServerLoadBalancer: &LoadBalancer{
+						IP: "1.2.3.4",
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Changing OpenStackCluster.Spec.APIServerFloatingIP is not allowed when it doesn't matches the current api server loadbalancer IP",
+			oldTemplate: &OpenStackCluster{
+				Spec: OpenStackClusterSpec{
+					APIServerFloatingIP: "",
+				},
+				Status: OpenStackClusterStatus{
+					APIServerLoadBalancer: &LoadBalancer{
+						IP: "1.2.3.4",
+					},
+				},
+			},
+			newTemplate: &OpenStackCluster{
+				Spec: OpenStackClusterSpec{
+					APIServerFloatingIP: "5.6.7.8",
+				},
+				Status: OpenStackClusterStatus{
+					APIServerLoadBalancer: &LoadBalancer{
+						IP: "1.2.3.4",
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/cloud/services/loadbalancer/loadbalancer.go

@@ -491,8 +491,14 @@ func (s *Service) DeleteLoadBalancer(openStackCluster *infrav1.OpenStackCluster,
 			if err = s.networkingService.DisassociateFloatingIP(openStackCluster, fip.FloatingIP); err != nil {
 				return err
 			}
-			if err = s.networkingService.DeleteFloatingIP(openStackCluster, fip.FloatingIP); err != nil {
-				return err
+
+			// If the floating is user-provider (BYO floating IP), don't delete it.
+			if openStackCluster.Spec.APIServerFloatingIP != fip.FloatingIP {
+				if err = s.networkingService.DeleteFloatingIP(openStackCluster, fip.FloatingIP); err != nil {
+					return err
+				}
+			} else {
+				s.scope.Logger().Info("Skipping load balancer floating IP deletion as it's a user-provided resource", "name", loadBalancerName, "fip", fip.FloatingIP)
 			}
 		}
 	}