Skip to content

Commit 5e0d5e8

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#2205 from k8s-infra-cherrypick-robot/cherry-pick-2185-to-release-1.6
[release-1.6] 🐛 clustermodules: prevent creation of new modules if DoesExist returns an error
2 parents b3e3f71 + ab1f6fe commit 5e0d5e8

File tree

2 files changed

+161
-1
lines changed

2 files changed

+161
-1
lines changed

‎controllers/clustermodule_reconciler.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ func (r Reconciler) Reconcile(ctx *context.ClusterContext) (reconcile.Result, er
7272
return reconcile.Result{}, err
7373
}
7474

75+
modErrs := []clusterModError{}
76+
7577
clusterModuleSpecs := []infrav1.ClusterModule{}
7678
for _, mod := range ctx.VSphereCluster.Spec.ClusterModules {
7779
curr := mod.TargetObjectName
@@ -90,9 +92,20 @@ func (r Reconciler) Reconcile(ctx *context.ClusterContext) (reconcile.Result, er
9092
// verify the cluster module
9193
exists, err := r.ClusterModuleService.DoesExist(ctx, obj, mod.ModuleUUID)
9294
if err != nil {
95+
// Add the error to modErrs so it gets handled below.
96+
modErrs = append(modErrs, clusterModError{obj.GetName(), errors.Wrapf(err, "failed to verify cluster module %q", mod.ModuleUUID)})
9397
ctx.Logger.Error(err, "failed to verify cluster module for object",
9498
"name", mod.TargetObjectName, "moduleUUID", mod.ModuleUUID)
99+
// Append the module and remove it from objectMap to not create new ones instead.
100+
clusterModuleSpecs = append(clusterModuleSpecs, infrav1.ClusterModule{
101+
ControlPlane: obj.IsControlPlane(),
102+
TargetObjectName: obj.GetName(),
103+
ModuleUUID: mod.ModuleUUID,
104+
})
105+
delete(objectMap, curr)
106+
continue
95107
}
108+
96109
// append the module and object info to the VSphereCluster object
97110
// and remove it from the object map since no new cluster module
98111
// needs to be created.
@@ -111,7 +124,6 @@ func (r Reconciler) Reconcile(ctx *context.ClusterContext) (reconcile.Result, er
111124
}
112125
}
113126

114-
modErrs := []clusterModError{}
115127
for _, obj := range objectMap {
116128
moduleUUID, err := r.ClusterModuleService.Create(ctx, obj)
117129
if err != nil {

‎controllers/clustermodule_reconciler_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ func TestReconciler_Reconcile(t *testing.T) {
4141
kcpUUID, mdUUID := uuid.New().String(), uuid.New().String()
4242
kcp := controlPlane("kcp", metav1.NamespaceDefault, fake.Clusterv1a2Name)
4343
md := machineDeployment("md", metav1.NamespaceDefault, fake.Clusterv1a2Name)
44+
vCenter500err := errors.New("500 Internal Server Error")
4445

4546
tests := []struct {
4647
name string
@@ -92,6 +93,153 @@ func TestReconciler_Reconcile(t *testing.T) {
9293
g.Expect(moduleUUIDs).To(gomega.ConsistOf(kcpUUID, mdUUID))
9394
},
9495
},
96+
{
97+
name: "when one cluster modules exist",
98+
clusterModules: []infrav1.ClusterModule{
99+
{
100+
ControlPlane: true,
101+
TargetObjectName: "kcp",
102+
ModuleUUID: kcpUUID,
103+
},
104+
},
105+
setupMocks: func(svc *cmodfake.CMService) {
106+
svc.On("DoesExist", mock.Anything, mock.Anything, kcpUUID).Return(true, nil)
107+
svc.On("Create", mock.Anything, clustermodule.NewWrapper(md)).Return(mdUUID, nil)
108+
},
109+
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
110+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
111+
var (
112+
names, moduleUUIDs []string
113+
)
114+
for _, mod := range ctx.VSphereCluster.Spec.ClusterModules {
115+
names = append(names, mod.TargetObjectName)
116+
moduleUUIDs = append(moduleUUIDs, mod.ModuleUUID)
117+
}
118+
g.Expect(names).To(gomega.ConsistOf("kcp", "md"))
119+
g.Expect(moduleUUIDs).To(gomega.ConsistOf(kcpUUID, mdUUID))
120+
},
121+
},
122+
{
123+
name: "when cluster modules do not exist anymore",
124+
clusterModules: []infrav1.ClusterModule{
125+
{
126+
ControlPlane: true,
127+
TargetObjectName: "kcp",
128+
ModuleUUID: kcpUUID,
129+
},
130+
{
131+
ControlPlane: false,
132+
TargetObjectName: "md",
133+
ModuleUUID: mdUUID,
134+
},
135+
},
136+
setupMocks: func(svc *cmodfake.CMService) {
137+
svc.On("DoesExist", mock.Anything, mock.Anything, kcpUUID).Return(false, nil)
138+
svc.On("DoesExist", mock.Anything, mock.Anything, mdUUID).Return(false, nil)
139+
svc.On("Create", mock.Anything, clustermodule.NewWrapper(kcp)).Return(kcpUUID+"a", nil)
140+
svc.On("Create", mock.Anything, clustermodule.NewWrapper(md)).Return(mdUUID+"a", nil)
141+
},
142+
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
143+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
144+
// Ensure the new modules exist.
145+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ModuleUUID).To(gomega.BeElementOf(kcpUUID+"a", mdUUID+"a"))
146+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[1].ModuleUUID).To(gomega.BeElementOf(kcpUUID+"a", mdUUID+"a"))
147+
// Check that condition got set.
148+
g.Expect(conditions.Has(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
149+
g.Expect(conditions.IsTrue(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
150+
},
151+
},
152+
{
153+
name: "when cluster modules already exist but vCenter returns an error",
154+
clusterModules: []infrav1.ClusterModule{
155+
{
156+
ControlPlane: true,
157+
TargetObjectName: "kcp",
158+
ModuleUUID: kcpUUID,
159+
},
160+
{
161+
ControlPlane: false,
162+
TargetObjectName: "md",
163+
ModuleUUID: mdUUID,
164+
},
165+
},
166+
haveError: true,
167+
setupMocks: func(svc *cmodfake.CMService) {
168+
svc.On("DoesExist", mock.Anything, mock.Anything, kcpUUID).Return(false, vCenter500err)
169+
svc.On("DoesExist", mock.Anything, mock.Anything, mdUUID).Return(false, vCenter500err)
170+
},
171+
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
172+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
173+
// Ensure the old modules still exist.
174+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ModuleUUID).To(gomega.BeElementOf(kcpUUID, mdUUID))
175+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[1].ModuleUUID).To(gomega.BeElementOf(kcpUUID, mdUUID))
176+
// Check that condition got set.
177+
g.Expect(conditions.Has(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
178+
g.Expect(conditions.IsFalse(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
179+
g.Expect(conditions.Get(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition).Message).To(gomega.ContainSubstring(vCenter500err.Error()))
180+
},
181+
},
182+
{
183+
name: "when one cluster modules exists and for the other we get an error",
184+
clusterModules: []infrav1.ClusterModule{
185+
{
186+
ControlPlane: true,
187+
TargetObjectName: "kcp",
188+
ModuleUUID: kcpUUID,
189+
},
190+
{
191+
ControlPlane: false,
192+
TargetObjectName: "md",
193+
ModuleUUID: mdUUID,
194+
},
195+
},
196+
haveError: true,
197+
setupMocks: func(svc *cmodfake.CMService) {
198+
svc.On("DoesExist", mock.Anything, mock.Anything, kcpUUID).Return(true, nil)
199+
svc.On("DoesExist", mock.Anything, mock.Anything, mdUUID).Return(false, vCenter500err)
200+
},
201+
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
202+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
203+
// Ensure the old modules still exist.
204+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ModuleUUID).To(gomega.BeElementOf(kcpUUID, mdUUID))
205+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[1].ModuleUUID).To(gomega.BeElementOf(kcpUUID, mdUUID))
206+
// Check that condition got set.
207+
g.Expect(conditions.Has(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
208+
g.Expect(conditions.IsFalse(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
209+
g.Expect(conditions.Get(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition).Message).To(gomega.ContainSubstring(vCenter500err.Error()))
210+
},
211+
},
212+
{
213+
name: "when one cluster modules does not exist and for the other we get an error",
214+
clusterModules: []infrav1.ClusterModule{
215+
{
216+
ControlPlane: true,
217+
TargetObjectName: "kcp",
218+
ModuleUUID: kcpUUID,
219+
},
220+
{
221+
ControlPlane: false,
222+
TargetObjectName: "md",
223+
ModuleUUID: mdUUID,
224+
},
225+
},
226+
haveError: true,
227+
setupMocks: func(svc *cmodfake.CMService) {
228+
svc.On("DoesExist", mock.Anything, mock.Anything, kcpUUID).Return(false, nil)
229+
svc.On("DoesExist", mock.Anything, mock.Anything, mdUUID).Return(false, vCenter500err)
230+
svc.On("Create", mock.Anything, clustermodule.NewWrapper(kcp)).Return(kcpUUID+"a", nil)
231+
},
232+
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
233+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(2))
234+
// Ensure the errored and the new module exist.
235+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[0].ModuleUUID).To(gomega.BeElementOf(kcpUUID+"a", mdUUID))
236+
g.Expect(ctx.VSphereCluster.Spec.ClusterModules[1].ModuleUUID).To(gomega.BeElementOf(kcpUUID+"a", mdUUID))
237+
// Check that condition got set.
238+
g.Expect(conditions.Has(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
239+
g.Expect(conditions.IsFalse(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
240+
g.Expect(conditions.Get(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition).Message).To(gomega.ContainSubstring(vCenter500err.Error()))
241+
},
242+
},
95243
{
96244
name: "when cluster module creation is called for a resource pool owned by non compute cluster resource",
97245
clusterModules: []infrav1.ClusterModule{},

0 commit comments

Comments
 (0)