Merge pull request #8048 from smartxworks/hw/make-sure-kubernetes-service-create-before-apply-crs

🐛: Make sure the Kubernetes API Server service already created on remote cluster before applying ClusterResourceSets

Author: k8s-ci-robot

exp/addons/internal/controllers/clusterresourceset_controller.go

@@ -249,6 +249,13 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 		return err
 	}
 
+	// Ensure that the Kubernetes API Server service has been created in the remote cluster before applying the ClusterResourceSet to avoid service IP conflict.
+	// This action is required when the remote cluster Kubernetes version is lower than v1.25.
+	// TODO: Remove this action once CAPI no longer supports Kubernetes versions below v1.25. See: https://github.com/kubernetes-sigs/cluster-api/issues/7804
+	if err = ensureKubernetesServiceCreated(ctx, remoteClient); err != nil {
+		return errors.Wrapf(err, "failed to retrieve the Service for Kubernetes API Server of the cluster %s/%s", cluster.Namespace, cluster.Name)
+	}
+
 	// Get ClusterResourceSetBinding object for the cluster.
 	clusterResourceSetBinding, err := r.getOrCreateClusterResourceSetBinding(ctx, cluster, clusterResourceSet)
 	if err != nil {

exp/addons/internal/controllers/clusterresourceset_controller_test.go

@@ -910,6 +910,100 @@ metadata:
 		g.Expect(env.Delete(ctx, resourceConfigMapWithMissingNamespace)).To(Succeed())
 		g.Expect(env.Delete(ctx, missingNs)).To(Succeed())
 	})
+
+	t.Run("Should only create ClusterResourceSetBinding after the remote cluster's Kubernetes API Server Service has been created", func(t *testing.T) {
+		g := NewWithT(t)
+		ns := setup(t, g)
+		defer teardown(t, g, ns)
+
+		kubernetesAPIServerService := &corev1.Service{
+			TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "kubernetes",
+				Namespace: metav1.NamespaceDefault,
+			},
+		}
+
+		fakeService := &corev1.Service{
+			TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "fake",
+				Namespace: metav1.NamespaceDefault,
+			},
+			Spec: corev1.ServiceSpec{
+				Ports: []corev1.ServicePort{
+					{
+						Name: "https",
+						Port: 443,
+					},
+				},
+				Type: "ClusterIP",
+			},
+		}
+
+		t.Log("Verifying Kubernetes API Server Service has been created")
+		g.Expect(env.Get(ctx, client.ObjectKeyFromObject(kubernetesAPIServerService), kubernetesAPIServerService)).To(Succeed())
+
+		fakeService.Spec.ClusterIP = kubernetesAPIServerService.Spec.ClusterIP
+
+		t.Log("Let Kubernetes API Server Service fail to create by occupying its IP")
+		g.Eventually(func() error {
+			err := env.Delete(ctx, kubernetesAPIServerService)
+			if err != nil {
+				return err
+			}
+			err = env.Create(ctx, fakeService)
+			if err != nil {
+				return err
+			}
+			return nil
+		}, timeout).Should(Succeed())
+		g.Expect(apierrors.IsNotFound(env.Get(ctx, client.ObjectKeyFromObject(kubernetesAPIServerService), kubernetesAPIServerService))).To(BeTrue())
+
+		clusterResourceSetInstance := &addonsv1.ClusterResourceSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      clusterResourceSetName,
+				Namespace: ns.Name,
+			},
+			Spec: addonsv1.ClusterResourceSetSpec{
+				ClusterSelector: metav1.LabelSelector{
+					MatchLabels: labels,
+				},
+			},
+		}
+		// Create the ClusterResourceSet.
+		g.Expect(env.Create(ctx, clusterResourceSetInstance)).To(Succeed())
+
+		testCluster.SetLabels(labels)
+		g.Expect(env.Update(ctx, testCluster)).To(Succeed())
+
+		// ClusterResourceSetBinding for the Cluster is not created because the Kubernetes API Server Service doesn't exist.
+		clusterResourceSetBindingKey := client.ObjectKey{
+			Namespace: testCluster.Namespace,
+			Name:      testCluster.Name,
+		}
+		g.Consistently(func() bool {
+			binding := &addonsv1.ClusterResourceSetBinding{}
+
+			err := env.Get(ctx, clusterResourceSetBindingKey, binding)
+			return apierrors.IsNotFound(err)
+		}, timeout).Should(BeTrue())
+
+		t.Log("Make sure Kubernetes API Server Service has been created")
+		g.Expect(env.Delete(ctx, fakeService)).Should(Succeed())
+		g.Eventually(func() bool {
+			err := env.Get(ctx, client.ObjectKeyFromObject(kubernetesAPIServerService), kubernetesAPIServerService)
+			return err == nil
+		}, timeout).Should(BeTrue())
+
+		// Wait until ClusterResourceSetBinding is created for the Cluster
+		g.Eventually(func() bool {
+			binding := &addonsv1.ClusterResourceSetBinding{}
+
+			err := env.Get(ctx, clusterResourceSetBindingKey, binding)
+			return err == nil
+		}, timeout).Should(BeTrue())
+	})
 }
 
 func clusterResourceSetBindingReady(env *envtest.Environment, cluster *clusterv1.Cluster) func() bool {

exp/addons/internal/controllers/clusterresourceset_helpers.go

@@ -250,3 +250,15 @@ func getClusterNameFromOwnerRef(obj metav1.ObjectMeta) (string, error) {
 	}
 	return "", errors.New("failed to find cluster name in ownerRefs: no cluster ownerRef")
 }
+
+// ensureKubernetesServiceCreated ensures that the Service for Kubernetes API Server has been created.
+func ensureKubernetesServiceCreated(ctx context.Context, client client.Client) error {
+	err := client.Get(ctx, types.NamespacedName{
+		Namespace: metav1.NamespaceDefault,
+		Name:      "kubernetes",
+	}, &corev1.Service{})
+	if err != nil {
+		return err
+	}
+	return nil
+}

exp/addons/internal/controllers/clusterresourceset_helpers_test.go

@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -219,3 +220,54 @@ func TestGetConfigMapFromNamespacedName(t *testing.T) {
 		})
 	}
 }
+
+func TestEnsureKubernetesServiceCreated(t *testing.T) {
+	g := NewWithT(t)
+
+	scheme := runtime.NewScheme()
+	g.Expect(corev1.AddToScheme(scheme)).To(Succeed())
+
+	kubernetesAPIServerService := &corev1.Service{
+		TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "kubernetes",
+			Namespace: metav1.NamespaceDefault,
+		},
+	}
+
+	tests := []struct {
+		name         string
+		existingObjs []client.Object
+		wantErr      bool
+	}{
+		{
+			name:         "should return nil when Kubernetes API Server Service exists",
+			existingObjs: []client.Object{kubernetesAPIServerService},
+			wantErr:      false,
+		},
+		{
+			name:         "should return error when Kubernetes API Server Service does not exist",
+			existingObjs: []client.Object{},
+			wantErr:      true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gs := NewWithT(t)
+
+			c := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(tt.existingObjs...).
+				Build()
+
+			err := ensureKubernetesServiceCreated(context.TODO(), c)
+
+			if tt.wantErr {
+				gs.Expect(err).To(HaveOccurred())
+				return
+			}
+			gs.Expect(err).NotTo(HaveOccurred())
+		})
+	}
+}