Skip to content

Commit 2df1a7a

Browse files
committed
fix(machinedeployment): fix completion flakiness
MachineDeploymentComplete() has a regression when introducing v1beta2 where it has flakiness due to lossy conversion and stale status. Replaced conversion-data annotation check with deployment.Status.V1Beta2. Additionally, guard against CAPI status race by adding a MachineSet template cross-verification to MachineDeploymentComplete, and move status reconciliation outside CreateOrPatch so it reads post-persist state. Signed-off-by: Borja Clemente <bclement@redhat.com>
1 parent bc3bda9 commit 2df1a7a

3 files changed

Lines changed: 284 additions & 140 deletions

File tree

hypershift-operator/controllers/nodepool/capi.go

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ func (c *CAPI) Reconcile(ctx context.Context) error {
149149
} else {
150150
log.Info("Reconciled MachineDeployment", "result", result)
151151
}
152+
c.reconcileMachineDeploymentStatus(ctx, log, md, template)
152153
}
153154

154155
mhc := c.machineHealthCheck()
@@ -473,12 +474,7 @@ func (c *CAPI) reconcileMachineDeployment(ctx context.Context, log logr.Logger,
473474
}
474475

475476
setMachineDeploymentReplicas(nodePool, machineDeployment)
476-
477-
if updated := c.propagateVersionAndTemplate(log, machineDeployment, machineTemplateCR); updated {
478-
return nil
479-
}
480-
481-
c.reconcileMachineDeploymentStatus(log, machineDeployment, machineTemplateCR)
477+
c.propagateVersionAndTemplate(log, machineDeployment, machineTemplateCR)
482478

483479
return nil
484480
}
@@ -563,12 +559,11 @@ func (c *CAPI) propagateLabelsAndTaintsToMachines(ctx context.Context, log logr.
563559
return nil
564560
}
565561

566-
func (c *CAPI) propagateVersionAndTemplate(log logr.Logger, machineDeployment *capiv1.MachineDeployment, machineTemplateCR client.Object) bool {
562+
func (c *CAPI) propagateVersionAndTemplate(log logr.Logger, machineDeployment *capiv1.MachineDeployment, machineTemplateCR client.Object) {
567563
nodePool := c.nodePool
568564
userDataSecret := c.UserDataSecret()
569565
targetVersion := c.Version()
570566
targetConfigHash := c.HashWithoutVersion()
571-
isUpdating := false
572567

573568
if userDataSecret.Name != ptr.Deref(machineDeployment.Spec.Template.Spec.Bootstrap.DataSecretName, "") {
574569
log.Info("New user data Secret has been generated",
@@ -586,29 +581,25 @@ func (c *CAPI) propagateVersionAndTemplate(log logr.Logger, machineDeployment *c
586581
}
587582
machineDeployment.Spec.Template.Spec.Version = &targetVersion
588583
machineDeployment.Spec.Template.Spec.Bootstrap.DataSecretName = ptr.To(userDataSecret.Name)
589-
isUpdating = true
590584
}
591585

592586
if machineTemplateCR.GetName() != machineDeployment.Spec.Template.Spec.InfrastructureRef.Name {
593587
log.Info("New machine template has been generated",
594588
"current", machineDeployment.Spec.Template.Spec.InfrastructureRef.Name,
595589
"target", machineTemplateCR.GetName())
596590
machineDeployment.Spec.Template.Spec.InfrastructureRef.Name = machineTemplateCR.GetName()
597-
isUpdating = true
598591
}
599-
600-
return isUpdating
601592
}
602593

603-
func (c *CAPI) reconcileMachineDeploymentStatus(log logr.Logger, machineDeployment *capiv1.MachineDeployment, machineTemplateCR client.Object) {
594+
func (c *CAPI) reconcileMachineDeploymentStatus(ctx context.Context, log logr.Logger, machineDeployment *capiv1.MachineDeployment, machineTemplateCR client.Object) {
604595
nodePool := c.nodePool
605596
targetVersion := c.Version()
606597
targetConfigHash := c.HashWithoutVersion()
607598
targetConfigVersionHash := c.Hash()
608599

609600
// If the MachineDeployment is now processing we know
610601
// is at the expected version (spec.version) and config (userData Secret) so we reconcile status and annotation.
611-
if MachineDeploymentComplete(machineDeployment) {
602+
if MachineDeploymentComplete(ctx, c.Client, machineDeployment, machineTemplateCR.GetName()) {
612603
if nodePool.Status.Version != targetVersion {
613604
log.Info("Version update complete",
614605
"previous", nodePool.Status.Version, "new", targetVersion)

0 commit comments

Comments
 (0)