Merge pull request #1671 from srm09/automated-cherry-pick-of-#1670-release-1.3

🌱 Automated cherry pick of #1670: Skip creation of cluster modules

Author: k8s-ci-robot

controllers/clustermodule_reconciler.go

@@ -119,6 +119,10 @@ func (r Reconciler) Reconcile(ctx *context.ClusterContext) (reconcile.Result, er
 			modErrs = append(modErrs, clusterModError{obj.GetName(), err})
 			continue
 		}
+		// module creation was skipped
+		if moduleUUID == "" {
+			continue
+		}
 		clusterModuleSpecs = append(clusterModuleSpecs, infrav1.ClusterModule{
 			ControlPlane:     obj.IsControlPlane(),
 			TargetObjectName: obj.GetName(),

controllers/clustermodule_reconciler_test.go

@@ -146,6 +146,21 @@ func TestReconciler_Reconcile(t *testing.T) {
 				g.Expect(conditions.Get(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition).Message).To(gomega.ContainSubstring("kcp"))
 			},
 		},
+		{
+			name:           "when some cluster module creations are skipped",
+			clusterModules: []infrav1.ClusterModule{},
+			setupMocks: func(svc *cmodfake.CMService) {
+				svc.On("Create", mock.Anything, clustermodule.NewWrapper(kcp)).Return(kcpUUID, nil)
+				// mimics cluster module creation was skipped
+				svc.On("Create", mock.Anything, clustermodule.NewWrapper(md)).Return("", nil)
+			},
+			customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(1))
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].TargetObjectName).To(gomega.Equal("kcp"))
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ModuleUUID).To(gomega.Equal(kcpUUID))
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ControlPlane).To(gomega.BeTrue())
+			},
+		},
 		{
 			name: "when machine deployment is being deleted",
 			beforeFn: func(object client.Object) {

pkg/clustermodule/service.go

@@ -19,6 +19,7 @@ package clustermodule
 import (
 	goctx "context"
 
+	"github.com/pkg/errors"
 	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/vim25/types"
 
@@ -27,19 +28,37 @@ import (
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/session"
 )
 
+const validMachineTemplate = "VSphereMachineTemplate"
+
 type service struct{}
 
 func NewService() Service {
 	return service{}
 }
 
 func (s service) Create(ctx *context.ClusterContext, wrapper Wrapper) (string, error) {
-	logger := ctx.Logger.WithValues("object", wrapper.GetName())
-	template, err := fetchMachineTemplate(ctx, wrapper)
+	logger := ctx.Logger.WithValues("object", wrapper.GetName(), "namespace", wrapper.GetNamespace())
+
+	templateRef, err := fetchTemplateRef(ctx, ctx.Client, wrapper)
+	if err != nil {
+		logger.V(4).Error(err, "error fetching template for object")
+		return "", errors.Wrapf(err, "error fetching machine template for object %s/%s", wrapper.GetNamespace(), wrapper.GetName())
+	}
+	if templateRef.Kind != validMachineTemplate {
+		// since this is a heterogeneous cluster, we should skip cluster module creation for non VSphereMachine objects
+		logger.V(4).Info("skipping module creation for object")
+		return "", nil
+	}
+
+	template, err := fetchMachineTemplate(ctx, wrapper, templateRef.Name)
 	if err != nil {
 		logger.V(4).Error(err, "error fetching template")
 		return "", err
 	}
+	if server := template.Spec.Template.Spec.Server; server != ctx.VSphereCluster.Spec.Server {
+		logger.V(4).Info("skipping module creation for object since template uses a different server", "server", server)
+		return "", nil
+	}
 
 	vCenterSession, err := fetchSessionForObject(ctx, template)
 	if err != nil {
@@ -67,7 +86,14 @@ func (s service) Create(ctx *context.ClusterContext, wrapper Wrapper) (string, e
 
 func (s service) DoesExist(ctx *context.ClusterContext, wrapper Wrapper, moduleUUID string) (bool, error) {
 	logger := ctx.Logger.WithValues("object", wrapper.GetName())
-	template, err := fetchMachineTemplate(ctx, wrapper)
+
+	templateRef, err := fetchTemplateRef(ctx, ctx.Client, wrapper)
+	if err != nil {
+		logger.V(4).Error(err, "error fetching template for object")
+		return false, errors.Wrapf(err, "error fetching infrastructure machine template for object %s/%s", wrapper.GetNamespace(), wrapper.GetName())
+	}
+
+	template, err := fetchMachineTemplate(ctx, wrapper, templateRef.Name)
 	if err != nil {
 		logger.V(4).Error(err, "error fetching template")
 		return false, err

pkg/clustermodule/service_test.go

@@ -0,0 +1,97 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package clustermodule
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/fake"
+)
+
+func TestService_Create(t *testing.T) {
+	svc := NewService()
+
+	t.Run("creation is skipped", func(t *testing.T) {
+		t.Run("when wrapper points to template != VSphereMachineTemplate", func(t *testing.T) {
+			md := machineDeployment("md", fake.Namespace, fake.Clusterv1a2Name)
+			md.Spec.Template.Spec.InfrastructureRef = corev1.ObjectReference{
+				Kind:      "NonVSphereMachineTemplate",
+				Namespace: fake.Namespace,
+				Name:      "blah",
+			}
+
+			g := gomega.NewWithT(t)
+			controllerCtx := fake.NewControllerContext(fake.NewControllerManagerContext(md))
+			ctx := fake.NewClusterContext(controllerCtx)
+
+			moduleUUID, err := svc.Create(ctx, mdWrapper{md})
+			g.Expect(err).ToNot(gomega.HaveOccurred())
+			g.Expect(moduleUUID).To(gomega.BeEmpty())
+		})
+
+		t.Run("when template uses a different vCenter URL", func(t *testing.T) {
+			md := machineDeployment("md", fake.Namespace, fake.Clusterv1a2Name)
+			md.Spec.Template.Spec.InfrastructureRef = corev1.ObjectReference{
+				Kind:      "VSphereMachineTemplate",
+				Namespace: fake.Namespace,
+				Name:      "blah-template",
+			}
+
+			machineTemplate := &infrav1.VSphereMachineTemplate{
+				TypeMeta: metav1.TypeMeta{Kind: "VSphereMachineTemplate"},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "blah-template",
+					Namespace: fake.Namespace,
+				},
+				Spec: infrav1.VSphereMachineTemplateSpec{
+					Template: infrav1.VSphereMachineTemplateResource{Spec: infrav1.VSphereMachineSpec{
+						VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{Server: fmt.Sprintf("not.%s", fake.VCenterURL)},
+					}},
+				},
+			}
+
+			g := gomega.NewWithT(t)
+			controllerCtx := fake.NewControllerContext(fake.NewControllerManagerContext(md, machineTemplate))
+			ctx := fake.NewClusterContext(controllerCtx)
+
+			moduleUUID, err := svc.Create(ctx, mdWrapper{md})
+			g.Expect(err).ToNot(gomega.HaveOccurred())
+			g.Expect(moduleUUID).To(gomega.BeEmpty())
+		})
+	})
+}
+
+func machineDeployment(name, namespace, cluster string) *clusterv1.MachineDeployment {
+	return &clusterv1.MachineDeployment{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "MachineDeployment",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+			Labels:    map[string]string{clusterv1.ClusterLabelName: cluster},
+		},
+	}
+}

pkg/clustermodule/session.go

@@ -80,16 +80,10 @@ func fetchTemplateRef(ctx goctx.Context, c client.Client, input Wrapper) (*corev
 	return &objRef, nil
 }
 
-func fetchMachineTemplate(ctx *context.ClusterContext, input Wrapper) (*infrav1.VSphereMachineTemplate, error) {
-	templateRef, err := fetchTemplateRef(ctx, ctx.Client, input)
-	if err != nil {
-		return nil, errors.Wrapf(err, "error fetching machine template for object %s/%s", input.GetNamespace(), input.GetName())
-	}
-
-	ctx.Logger.Info("found template", "ref", templateRef.Name)
+func fetchMachineTemplate(ctx *context.ClusterContext, input Wrapper, templateName string) (*infrav1.VSphereMachineTemplate, error) {
 	template := &infrav1.VSphereMachineTemplate{}
 	if err := ctx.Client.Get(ctx, client.ObjectKey{
-		Name:      templateRef.Name,
+		Name:      templateName,
 		Namespace: input.GetNamespace(),
 	}, template); err != nil {
 		return nil, err

pkg/context/fake/constants.go

@@ -72,6 +72,8 @@ const (
 
 	// ServiceCIDR is the CIDR for the service network.
 	ServiceCIDR = "2.0.0.0/16"
+
+	VCenterURL = "foo.vcenter.com"
 )
 
 var boolTrue = true

pkg/context/fake/fake_cluster_context.go

@@ -85,6 +85,8 @@ func newVSphereCluster(owner clusterv1.Cluster) infrav1.VSphereCluster {
 				},
 			},
 		},
-		Spec: infrav1.VSphereClusterSpec{},
+		Spec: infrav1.VSphereClusterSpec{
+			Server: VCenterURL,
+		},
 	}
 }