Skip to content

Commit 2c00bff

Browse files
committed
Added check to prevent duplicate failure domains in CPMS.
1 parent 666a25c commit 2c00bff

File tree

2 files changed

+58
-4
lines changed

2 files changed

+58
-4
lines changed

pkg/asset/machines/vsphere/machines.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,11 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine
8383
"machine.openshift.io/cluster-api-machine-type": role,
8484
}
8585

86-
failureDomains = append(failureDomains, machinev1.VSphereFailureDomain{
87-
Name: failureDomain.Name,
88-
})
86+
if !hasFailureDomain(failureDomains, failureDomain.Name) {
87+
failureDomains = append(failureDomains, machinev1.VSphereFailureDomain{
88+
Name: failureDomain.Name,
89+
})
90+
}
8991

9092
osImageForZone := failureDomain.Topology.Template
9193
if failureDomain.Topology.Template == "" {
@@ -255,3 +257,12 @@ func provider(clusterID string, vcenter *vsphere.VCenter, failureDomain vsphere.
255257
// ConfigMasters sets the PublicIP flag and assigns a set of load balancers to the given machines
256258
func ConfigMasters(machines []machineapi.Machine, clusterID string) {
257259
}
260+
261+
func hasFailureDomain(failureDomains []machinev1.VSphereFailureDomain, failureDomain string) bool {
262+
for _, fd := range failureDomains {
263+
if fd.Name == failureDomain {
264+
return true
265+
}
266+
}
267+
return false
268+
}

pkg/asset/machines/vsphere/machines_test.go

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,26 @@ var machinePoolReducedZones = types.MachinePool{
215215
Architecture: types.ArchitectureAMD64,
216216
}
217217

218+
var machinePoolSingleZones = types.MachinePool{
219+
Name: "master",
220+
Replicas: &machinePoolReplicas,
221+
Platform: types.MachinePoolPlatform{
222+
VSphere: &vsphere.MachinePool{
223+
NumCPUs: 4,
224+
NumCoresPerSocket: 2,
225+
MemoryMiB: 16384,
226+
OSDisk: vsphere.OSDisk{
227+
DiskSizeGB: 60,
228+
},
229+
Zones: []string{
230+
"deployzone-us-east-1a",
231+
},
232+
},
233+
},
234+
Hyperthreading: "true",
235+
Architecture: types.ArchitectureAMD64,
236+
}
237+
218238
var machinePoolUndefinedZones = types.MachinePool{
219239
Name: "master",
220240
Replicas: &machinePoolReplicas,
@@ -344,6 +364,21 @@ func TestConfigMasters(t *testing.T) {
344364
},
345365
},
346366
},
367+
{
368+
testCase: "all masters in single zone / pool",
369+
machinePool: &machinePoolSingleZones,
370+
maxAllowedWorkspaceMatches: 3,
371+
installConfig: installConfig,
372+
workspaces: []machineapi.Workspace{
373+
{
374+
Server: "your.vcenter.example.com",
375+
Datacenter: "dc1",
376+
Folder: "/dc1/vm/folder1",
377+
Datastore: "/dc1/datastore/datastore1",
378+
ResourcePool: "/dc1/host/c1/Resources/rp1",
379+
},
380+
},
381+
},
347382
{
348383
testCase: "full path to cluster resource pool when no pool provided via placement constraint",
349384
machinePool: &machinePoolValidZones,
@@ -378,7 +413,7 @@ func TestConfigMasters(t *testing.T) {
378413

379414
for _, tc := range testCases {
380415
t.Run(tc.testCase, func(t *testing.T) {
381-
machines, _, err := Machines(clusterID, tc.installConfig, tc.machinePool, "", "", "")
416+
machines, cpms, err := Machines(clusterID, tc.installConfig, tc.machinePool, "", "", "")
382417
assertOnUnexpectedErrorState(t, tc.expectedError, err)
383418

384419
if len(tc.workspaces) > 0 {
@@ -404,6 +439,14 @@ func TestConfigMasters(t *testing.T) {
404439
t.Errorf("machine workspace was enountered too few times[min: %d]", tc.minAllowedWorkspaceMatches)
405440
}
406441
}
442+
443+
if cpms != nil {
444+
// Make sure FDs equal same quantity as config
445+
fds := cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains
446+
if len(fds.VSphere) != len(tc.workspaces) {
447+
t.Errorf("machine workspace count %d does not equal number of failure domains [count: %d] in CPMS", len(tc.workspaces), len(fds.VSphere))
448+
}
449+
}
407450
}
408451
})
409452
}

0 commit comments

Comments
 (0)