Skip to content

Commit 596ad4f

Browse files
author
Joshua Reed
committed
Address PR comments.
1 parent 2f5d504 commit 596ad4f

6 files changed

+31
-17
lines changed

api/v1beta2/cloudstackmachine_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ type CloudStackMachineSpec struct {
7171
Affinity string `json:"affinity,omitempty"`
7272

7373
// Mutually exclusive parameter with AffinityGroupIDs.
74-
// Is a reference to a CloudStack affiniity group CRD.
74+
// Is a reference to a CloudStack affinity group CRD.
7575
// +optional
7676
AffinityGroupRef *corev1.ObjectReference `json:"cloudstackaffinityref,omitempty"`
7777

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachines.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ spec:
308308
type: array
309309
cloudstackaffinityref:
310310
description: Mutually exclusive parameter with AffinityGroupIDs. Is
311-
a reference to a CloudStack affiniity group CRD.
311+
a reference to a CloudStack affinity group CRD.
312312
properties:
313313
apiVersion:
314314
description: API version of the referent.

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackmachinetemplates.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ spec:
258258
type: array
259259
cloudstackaffinityref:
260260
description: Mutually exclusive parameter with AffinityGroupIDs.
261-
Is a reference to a CloudStack affiniity group CRD.
261+
Is a reference to a CloudStack affinity group CRD.
262262
properties:
263263
apiVersion:
264264
description: API version of the referent.

config/manager/manager.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ spec:
1515
labels:
1616
control-plane: capc-controller-manager
1717
spec:
18-
# securityContext:
19-
# runAsNonRoot: true
18+
securityContext:
19+
runAsNonRoot: true
2020
containers:
2121
- command:
2222
- /manager

controllers/cloudstackcluster_controller.go

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -103,26 +103,40 @@ func (r *CloudStackClusterReconciliationRunner) SetReady() (ctrl.Result, error)
103103

104104
// VerifyFailureDomainCRDs verifies the FailureDomains found match against those requested.
105105
func (r *CloudStackClusterReconciliationRunner) VerifyFailureDomainCRDs() (ctrl.Result, error) {
106-
expected := len(r.ReconciliationSubject.Spec.FailureDomains)
107-
actual := len(r.FailureDomains.Items)
108-
if expected != actual {
109-
return r.RequeueWithMessage(fmt.Sprintf("Expected %d FailureDomains, but found %d", expected, actual))
110-
}
111-
for _, fd := range r.FailureDomains.Items {
112-
if !fd.Status.Ready {
113-
return r.RequeueWithMessage(fmt.Sprintf("FailureDomains %s/%s not ready, requeueing.", fd.Namespace, fd.Name))
106+
// Check that all required failure domains are present and ready.
107+
for _, requiredFd := range r.ReconciliationSubject.Spec.FailureDomains {
108+
found := false
109+
for _, fd := range r.FailureDomains.Items {
110+
requiredFDName := withClusterSuffix(requiredFd.Name, r.CAPICluster.Name)
111+
if requiredFDName == fd.Name {
112+
found = true
113+
if !fd.Status.Ready {
114+
return r.RequeueWithMessage(fmt.Sprintf("Required FailureDomain %s not ready, requeueing.", fd.Name))
115+
}
116+
break
117+
}
118+
}
119+
if !found {
120+
return r.RequeueWithMessage(fmt.Sprintf("Required FailureDomain %s not found, requeueing.", requiredFd.Name))
114121
}
115122
}
116123
return ctrl.Result{}, nil
117124
}
118125

126+
// withClusterSuffix appends a hyphen and the cluster name to a name if not already present.
127+
func withClusterSuffix(name string, clusterName string) string {
128+
newName := name
129+
if !strings.HasSuffix(name, "-"+clusterName) { // Add cluster name suffix if missing.
130+
newName = name + "-" + clusterName
131+
}
132+
return newName
133+
}
134+
119135
// SetFailureDomainsStatusMap sets failure domains in CloudStackCluster status to be used for CAPI machine placement.
120136
func (r *CloudStackClusterReconciliationRunner) SetFailureDomainsStatusMap() (ctrl.Result, error) {
121137
r.ReconciliationSubject.Status.FailureDomains = clusterv1.FailureDomains{}
122138
for _, fdSpec := range r.ReconciliationSubject.Spec.FailureDomains {
123-
if !strings.HasSuffix(fdSpec.Name, "-"+r.CAPICluster.Name) { // Add cluster name suffix if missing.
124-
fdSpec.Name = fdSpec.Name + "-" + r.CAPICluster.Name
125-
}
139+
fdSpec.Name = withClusterSuffix(fdSpec.Name, r.CAPICluster.Name)
126140
r.ReconciliationSubject.Status.FailureDomains[fdSpec.Name] = clusterv1.FailureDomainSpec{ControlPlane: true}
127141
}
128142
return ctrl.Result{}, nil

controllers/cloudstackmachine_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ var _ = Describe("CloudStackMachineReconciler", func() {
5050
setClusterReady()
5151
})
5252

53-
It("Shoueld call GetOrCreateVMInstance and set Status.Ready to true", func() {
53+
It("Should call GetOrCreateVMInstance and set Status.Ready to true", func() {
5454
// Mock a call to GetOrCreateVMInstance and set the machine to running.
5555
mockCloudClient.EXPECT().GetOrCreateVMInstance(
5656
gomock.Any(), gomock.Any(), gomock.Any(),

0 commit comments

Comments
 (0)