Skip to content

Commit e74e151

Browse files
author
Joshua Reed
committed
Fix worker node placement logic.
1 parent feba8b8 commit e74e151

7 files changed

+50
-42
lines changed

‎api/v1beta1/cloudstackmachine_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ type InstanceState string
7474

7575
// Type pulled mostly from the CloudStack API.
7676
type CloudStackMachineStatus struct {
77+
// Zone ID is used so that the zone can be computed once per reconcile and then propagate.
78+
// +optional
79+
ZoneID string `json:"zoneID,omitempty"`
80+
7781
// Addresses contains a CloudStack VM instance's IP addresses.
7882
Addresses []corev1.NodeAddress `json:"addresses,omitempty"`
7983

‎config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,8 @@ spec:
136136
publicIPID:
137137
description: The CS public IP ID to use for the k8s endpoint.
138138
type: string
139-
publicIPNetworkId:
140-
description: The Id of the network the PublicIP is in.
139+
publicIPNetworkID:
140+
description: The ID of the network the PublicIP is in.
141141
type: string
142142
ready:
143143
description: Reflects the readiness of the CS cluster.

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ spec:
6262
description: CloudStackMachineSpec defines the desired state of CloudStackMachine
6363
properties:
6464
affinity:
65-
description: Mutually exclusive parameter with AffinityGroupIds. Defaults
65+
description: Mutually exclusive parameter with AffinityGroupIDs. Defaults
6666
to `no`. Can be `pro` or `anti`. Will create an affinity group per
6767
machine set.
6868
type: string
@@ -101,7 +101,7 @@ spec:
101101
type: string
102102
providerID:
103103
description: 'The CS specific unique identifier. Of the form: fmt.Sprintf("cloudstack:///%s",
104-
CS Machine Id)'
104+
CS Machine ID)'
105105
type: string
106106
sshKey:
107107
description: CloudStack ssh key to use.
@@ -140,6 +140,10 @@ spec:
140140
ready:
141141
description: Ready indicates the readiness of the provider resource.
142142
type: boolean
143+
zoneID:
144+
description: Zone ID is used so that the zone can be computed once
145+
per reconcile and then propagate.
146+
type: string
143147
required:
144148
- ready
145149
type: object

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ spec:
4848
CloudStackMachine
4949
properties:
5050
affinity:
51-
description: Mutually exclusive parameter with AffinityGroupIds.
51+
description: Mutually exclusive parameter with AffinityGroupIDs.
5252
Defaults to `no`. Can be `pro` or `anti`. Will create an
5353
affinity group per machine set.
5454
type: string
@@ -89,7 +89,7 @@ spec:
8989
type: string
9090
providerID:
9191
description: 'The CS specific unique identifier. Of the form:
92-
fmt.Sprintf("cloudstack:///%s", CS Machine Id)'
92+
fmt.Sprintf("cloudstack:///%s", CS Machine ID)'
9393
type: string
9494
sshKey:
9595
description: CloudStack ssh key to use.

‎controllers/cloudstackmachine_controller.go

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ func (r *CloudStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Re
136136
}
137137
}
138138

139-
if capiMachine.Spec.FailureDomain != nil && *capiMachine.Spec.FailureDomain == "" {
139+
if util.IsControlPlaneMachine(capiMachine) &&
140+
(capiMachine.Spec.FailureDomain == nil || *capiMachine.Spec.FailureDomain == "") {
140141
log.Info("CloudStackCluster ZoneID not initialized. Likely not ready.")
141142
return ctrl.Result{RequeueAfter: requeueTimeout}, nil
142143
}
@@ -154,49 +155,51 @@ func (r *CloudStackMachineReconciler) reconcile(
154155
csCluster *infrav1.CloudStackCluster) (ctrl.Result, error) {
155156

156157
log.V(1).Info("reconcile CloudStackMachine")
157-
158-
zoneID := capiMachine.Spec.FailureDomain
159-
zone := infrav1.Zone{}
160-
for _, zoneStatus := range csCluster.Status.Zones {
161-
if zoneID == &zoneStatus.ID {
162-
zone = zoneStatus
163-
break
164-
}
165-
}
166-
167158
// Make sure bootstrap data is available in CAPI machine.
168159
if capiMachine.Spec.Bootstrap.DataSecretName == nil {
169160
log.Info("Bootstrap DataSecretName not yet available.")
170161
return ctrl.Result{}, nil
171162
}
172163
log.Info("Got Bootstrap DataSecretName.")
173164

165+
// Set ZoneID on csMachine.
166+
if util.IsControlPlaneMachine(capiMachine) { // Use failure domain zone.
167+
csMachine.Status.ZoneID = *capiMachine.Spec.FailureDomain
168+
} else { // Random zone.
169+
zones := make([]string, len(csCluster.Status.Zones))
170+
zidx := 0
171+
for zoneID := range csCluster.Status.Zones {
172+
zones[zidx] = zoneID
173+
zidx++
174+
}
175+
}
176+
174177
secret := &corev1.Secret{}
175178
key := types.NamespacedName{Namespace: capiMachine.Namespace, Name: *capiMachine.Spec.Bootstrap.DataSecretName}
176179
if err := r.Client.Get(context.TODO(), key, secret); err != nil {
177180
return ctrl.Result{}, err
178181
}
179182

180-
value, ok := secret.Data["value"]
181-
if !ok {
182-
return ctrl.Result{}, errors.New("bootstrap secret data not ok")
183-
}
184-
185-
// Create VM (or Fetch if present). Will set ready to true.
186-
if err := r.CS.GetOrCreateVMInstance(csMachine, capiMachine, csCluster, string(value)); err == nil {
187-
if !controllerutil.ContainsFinalizer(csMachine, infrav1.MachineFinalizer) { // Fetched or Created?
188-
log.Info("CloudStack instance Created", "instanceStatus", csMachine.Status, "instanceSpec", csMachine.Spec)
189-
controllerutil.AddFinalizer(csMachine, infrav1.MachineFinalizer)
183+
// Create VM (or Fetch if present).
184+
if value, found := secret.Data["value"]; found {
185+
if err := r.CS.GetOrCreateVMInstance(csMachine, capiMachine, csCluster, string(value)); err == nil {
186+
if !controllerutil.ContainsFinalizer(csMachine, infrav1.MachineFinalizer) { // Fetched or Created?
187+
log.Info("CloudStack instance Created", "instanceStatus", csMachine.Status)
188+
controllerutil.AddFinalizer(csMachine, infrav1.MachineFinalizer)
189+
}
190+
} else if err != nil {
191+
return ctrl.Result{}, err
190192
}
191-
} else if err != nil {
192-
return ctrl.Result{}, err
193+
} else {
194+
return ctrl.Result{}, errors.New("bootstrap secret data not ok")
193195
}
194196

197+
// Check status of machine.
195198
if csMachine.Status.InstanceState == "Running" {
196199
log.Info("Machine instance is Running...")
197200
csMachine.Status.Ready = true
198201
} else if csMachine.Status.InstanceState == "Error" {
199-
log.Info("CloudStackMachine VM in error state. Deleting associated Machine.", "csMachine", csMachine)
202+
log.Info("CloudStackMachine VM in error state. Deleting associated Machine.", "csMachine", csMachine)
200203
if err := r.Client.Delete(ctx, csMachine); err != nil {
201204
return ctrl.Result{}, err
202205
}
@@ -206,6 +209,8 @@ func (r *CloudStackMachineReconciler) reconcile(
206209
return ctrl.Result{RequeueAfter: requeueTimeout}, nil
207210
}
208211

212+
// Add machine to load balancer if it is a control plane in an isolated network.
213+
zone := csCluster.Status.Zones[csMachine.Status.ZoneID]
209214
if util.IsControlPlaneMachine(capiMachine) && zone.Network.Type == cloud.NetworkTypeIsolated {
210215
log.Info("Assigning VM to load balancer rule.")
211216
err := r.CS.AssignVMToLoadBalancerRule(csCluster, *csMachine.Spec.InstanceID)

‎pkg/cloud/cluster.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (c *client) resolveZones(csCluster *infrav1.CloudStackCluster) (retErr erro
4646
"expected 1 Zone with UUID %s, but got %d", specZone.ID, count))
4747
} else {
4848
csCluster.Status.Zones[resp.Id] = infrav1.Zone{
49-
Name: resp.Name, ID: specZone.ID, Network: specZone.Network}
49+
Name: resp.Name, ID: resp.Id, Network: specZone.Network}
5050
}
5151
}
5252

@@ -64,6 +64,7 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
6464
csCluster.Status.FailureDomains = capiv1.FailureDomains{}
6565
for _, zone := range csCluster.Status.Zones {
6666
csCluster.Status.FailureDomains[zone.ID] = capiv1.FailureDomainSpec{ControlPlane: true}
67+
csCluster.Status.FailureDomains[zone.ID+"_workers"] = capiv1.FailureDomainSpec{ControlPlane: false}
6768
}
6869

6970
// If provided, translate Domain name to Domain ID.

‎pkg/cloud/instance.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,9 @@ func (c *client) ResolveServiceOffering(csMachine *infrav1.CloudStackMachine) (o
108108
func (c *client) ResolveTemplate(
109109
csCluster *infrav1.CloudStackCluster,
110110
csMachine *infrav1.CloudStackMachine,
111-
zone infrav1.Zone,
111+
zoneID string,
112112
) (templateID string, retErr error) {
113-
114-
templateID, count, err := c.cs.Template.GetTemplateID(csMachine.Spec.Template, "all", zone.ID)
113+
templateID, count, err := c.cs.Template.GetTemplateID(csMachine.Spec.Template, "all", zoneID)
115114
if err != nil {
116115
retErr = multierror.Append(retErr, errors.Wrapf(
117116
err, "could not get Template ID from %s", csMachine.Spec.Template))
@@ -143,11 +142,6 @@ func (c *client) GetOrCreateVMInstance(
143142
csCluster *infrav1.CloudStackCluster,
144143
userData string) error {
145144

146-
zone, ok := csCluster.Status.Zones[*capiMachine.Spec.FailureDomain]
147-
if !ok {
148-
return errors.New("failureDomain zone not found")
149-
}
150-
151145
// Check if VM instance already exists.
152146
if err := c.ResolveVMInstanceDetails(csMachine); err == nil ||
153147
!strings.Contains(strings.ToLower(err.Error()), "no match") {
@@ -157,14 +151,14 @@ func (c *client) GetOrCreateVMInstance(
157151
if err != nil {
158152
return err
159153
}
160-
161-
templateID, err := c.ResolveTemplate(csCluster, csMachine, zone)
154+
templateID, err := c.ResolveTemplate(csCluster, csMachine, csMachine.Status.ZoneID)
162155
if err != nil {
163156
return err
164157
}
165158

166159
// Create VM instance.
167-
p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offeringID, templateID, zone.ID)
160+
p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offeringID, templateID, csMachine.Status.ZoneID)
161+
zone := csCluster.Status.Zones[csMachine.Status.ZoneID]
168162
p.SetNetworkids([]string{zone.Network.ID})
169163
setIfNotEmpty(csMachine.Name, p.SetName)
170164
setIfNotEmpty(csMachine.Name, p.SetDisplayname)

0 commit comments

Comments
 (0)