clustermodules: prevent creation of new modules if DoesExist returns …

…an error
kubernetes-sigs · Aug 8, 2023 · fe98aab · fe98aab
1 parent f1341ab
commit fe98aab
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 13 deletions.
diff --git a/controllers/clustermodule_reconciler.go b/controllers/clustermodule_reconciler.go
@@ -74,6 +74,8 @@ func (r Reconciler) Reconcile(ctx *context.ClusterContext) (reconcile.Result, er
 		return reconcile.Result{}, err
 	}
 
+	modErrs := []clusterModError{}
+
 	clusterModuleSpecs := []infrav1.ClusterModule{}
 	for _, mod := range ctx.VSphereCluster.Spec.ClusterModules {
 		curr := mod.TargetObjectName
@@ -91,29 +93,34 @@ func (r Reconciler) Reconcile(ctx *context.ClusterContext) (reconcile.Result, er
 		} else {
 			// verify the cluster module
 			exists, err := r.ClusterModuleService.DoesExist(ctx, obj, mod.ModuleUUID)
+			// Nothing to do if there was no error while trying to find the cluster module.
+			// This leads to entering the creation flow below.
+			if err == nil && !exists {
+				ctx.Logger.Info("module for object not found",
+					"moduleUUID", mod.ModuleUUID,
+					"object", mod.TargetObjectName)
+				continue
+			}
+
+			// add the error to modErrors so we bubble it up to the vSphereCluster below.
 			if err != nil {
 				ctx.Logger.Error(err, "failed to verify cluster module for object",
 					"name", mod.TargetObjectName, "moduleUUID", mod.ModuleUUID)
+				modErrs = append(modErrs, clusterModError{obj.GetName(), errors.Wrapf(err, "failed to verify cluster module %q", mod.ModuleUUID)})
 			}
+
 			// append the module and object info to the VSphereCluster object
 			// and remove it from the object map since no new cluster module
 			// needs to be created.
-			if exists {
-				clusterModuleSpecs = append(clusterModuleSpecs, infrav1.ClusterModule{
-					ControlPlane:     obj.IsControlPlane(),
-					TargetObjectName: obj.GetName(),
-					ModuleUUID:       mod.ModuleUUID,
-				})
-				delete(objectMap, curr)
-			} else {
-				ctx.Logger.Info("module for object not found",
-					"moduleUUID", mod.ModuleUUID,
-					"object", mod.TargetObjectName)
-			}
+			clusterModuleSpecs = append(clusterModuleSpecs, infrav1.ClusterModule{
+				ControlPlane:     obj.IsControlPlane(),
+				TargetObjectName: obj.GetName(),
+				ModuleUUID:       mod.ModuleUUID,
+			})
+			delete(objectMap, curr)
 		}
 	}
 
-	modErrs := []clusterModError{}
 	for _, obj := range objectMap {
 		moduleUUID, err := r.ClusterModuleService.Create(ctx, obj)
 		if err != nil {

diff --git a/controllers/clustermodule_reconciler_test.go b/controllers/clustermodule_reconciler_test.go
@@ -41,6 +41,7 @@ func TestReconciler_Reconcile(t *testing.T) {
 	kcpUUID, mdUUID := uuid.New().String(), uuid.New().String()
 	kcp := controlPlane("kcp", metav1.NamespaceDefault, fake.Clusterv1a2Name)
 	md := machineDeployment("md", metav1.NamespaceDefault, fake.Clusterv1a2Name)
+	vCenter500err := errors.New("500 Internal Server Error")
 
 	tests := []struct {
 		name           string
@@ -72,6 +73,66 @@ func TestReconciler_Reconcile(t *testing.T) {
 				g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
 			},
 		},
+		{
+			name: "when cluster modules already exist but vCenter returns an error",
+			clusterModules: []infrav1.ClusterModule{
+				{
+					ControlPlane:     true,
+					TargetObjectName: "kcp",
+					ModuleUUID:       kcpUUID,
+				},
+				{
+					ControlPlane:     false,
+					TargetObjectName: "md",
+					ModuleUUID:       mdUUID,
+				},
+			},
+			haveError: true,
+			setupMocks: func(svc *cmodfake.CMService) {
+				svc.On("DoesExist", mock.Anything, mock.Anything, kcpUUID).Return(false, vCenter500err)
+				svc.On("DoesExist", mock.Anything, mock.Anything, mdUUID).Return(false, vCenter500err)
+			},
+			customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
+				// Ensure the old modules still exist.
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ModuleUUID).To(gomega.BeElementOf(kcpUUID, mdUUID))
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules[1].ModuleUUID).To(gomega.BeElementOf(kcpUUID, mdUUID))
+				// Check that condition got set.
+				g.Expect(conditions.Has(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
+				g.Expect(conditions.IsFalse(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
+				g.Expect(conditions.Get(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition).Message).To(gomega.ContainSubstring(vCenter500err.Error()))
+			},
+		},
+		{
+			name: "when cluster modules do not exist anymore",
+			clusterModules: []infrav1.ClusterModule{
+				{
+					ControlPlane:     true,
+					TargetObjectName: "kcp",
+					ModuleUUID:       kcpUUID,
+				},
+				{
+					ControlPlane:     false,
+					TargetObjectName: "md",
+					ModuleUUID:       mdUUID,
+				},
+			},
+			setupMocks: func(svc *cmodfake.CMService) {
+				svc.On("DoesExist", mock.Anything, mock.Anything, kcpUUID).Return(false, nil)
+				svc.On("DoesExist", mock.Anything, mock.Anything, mdUUID).Return(false, nil)
+				svc.On("Create", mock.Anything, clustermodule.NewWrapper(kcp)).Return(kcpUUID+"a", nil)
+				svc.On("Create", mock.Anything, clustermodule.NewWrapper(md)).Return(mdUUID+"a", nil)
+			},
+			customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
+				// Ensure the old modules still exist.
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ModuleUUID).To(gomega.BeElementOf(kcpUUID+"a", mdUUID+"a"))
+				g.Expect(ctx.VSphereCluster.Spec.ClusterModules[1].ModuleUUID).To(gomega.BeElementOf(kcpUUID+"a", mdUUID+"a"))
+				// Check that condition got set.
+				g.Expect(conditions.Has(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
+				g.Expect(conditions.IsTrue(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
+			},
+		},
 		{
 			name:           "when no cluster modules exist",
 			clusterModules: []infrav1.ClusterModule{},