Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ Add machine UpToDate condition to MachineSet #11393

Merged
merged 1 commit into from
Nov 11, 2024
Merged

✨ Add machine UpToDate condition to MachineSet #11393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 52 additions & 15 deletions internal/controllers/machinedeployment/mdutil/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package mdutil
import (
"context"
"fmt"
"reflect"
"sort"
"strconv"
"strings"
Expand All @@ -38,7 +39,6 @@ import (
ctrl "sigs.k8s.io/controller-runtime"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/internal/util/compare"
"sigs.k8s.io/cluster-api/util/conversion"
)

Expand Down Expand Up @@ -371,13 +371,50 @@ func getMachineSetFraction(ms clusterv1.MachineSet, md clusterv1.MachineDeployme
return integer.RoundToInt32(newMSsize) - *(ms.Spec.Replicas)
}

// EqualMachineTemplate returns true if two given machineTemplateSpec are equal,
// ignoring all the in-place propagated fields, and the version from external references.
func EqualMachineTemplate(template1, template2 *clusterv1.MachineTemplateSpec) (equal bool, diff string, err error) {
t1Copy := MachineTemplateDeepCopyRolloutFields(template1)
t2Copy := MachineTemplateDeepCopyRolloutFields(template2)
// MachineTemplateUpToDate returns true if the current MachineTemplateSpec is up-to-date with a corresponding desired MachineTemplateSpec.
// Note: The comparison does not consider any in-place propagated fields, as well as the version from external references.
func MachineTemplateUpToDate(current, desired *clusterv1.MachineTemplateSpec) (upToDate bool, logMessages, conditionMessages []string) {
currentCopy := MachineTemplateDeepCopyRolloutFields(current)
fabriziopandini marked this conversation as resolved.
Show resolved Hide resolved
desiredCopy := MachineTemplateDeepCopyRolloutFields(desired)

return compare.Diff(t1Copy, t2Copy)
if !reflect.DeepEqual(currentCopy.Spec.Version, desiredCopy.Spec.Version) {
logMessages = append(logMessages, fmt.Sprintf("spec.version %s, %s required", ptr.Deref(currentCopy.Spec.Version, "nil"), ptr.Deref(desiredCopy.Spec.Version, "nil")))
conditionMessages = append(conditionMessages, fmt.Sprintf("Version %s, %s required", ptr.Deref(currentCopy.Spec.Version, "nil"), ptr.Deref(desiredCopy.Spec.Version, "nil")))
}

// Note: we return a message based on desired.bootstrap.ConfigRef != nil, but we always compare the entire bootstrap
// struct to catch cases when either configRef or dataSecretName is set in current vs desired (usually MachineTemplates
// have ConfigRef != nil, might be in some edge case dataSecret are used, but switching from one to another is not a
// common operation so it is acceptable to handle it in this way).
if currentCopy.Spec.Bootstrap.ConfigRef != nil {
if !reflect.DeepEqual(currentCopy.Spec.Bootstrap, desiredCopy.Spec.Bootstrap) {
logMessages = append(logMessages, fmt.Sprintf("spec.bootstrap.configRef %s %s, %s %s required", currentCopy.Spec.Bootstrap.ConfigRef.Kind, currentCopy.Spec.Bootstrap.ConfigRef.Name, ptr.Deref(desiredCopy.Spec.Bootstrap.ConfigRef, corev1.ObjectReference{}).Kind, ptr.Deref(desiredCopy.Spec.Bootstrap.ConfigRef, corev1.ObjectReference{}).Name))
// Note: dropping "Template" suffix because conditions message will surface on machine.
conditionMessages = append(conditionMessages, fmt.Sprintf("%s is not up-to-date", strings.TrimSuffix(currentCopy.Spec.Bootstrap.ConfigRef.Kind, clusterv1.TemplateSuffix)))
}
} else {
if !reflect.DeepEqual(currentCopy.Spec.Bootstrap, desiredCopy.Spec.Bootstrap) {
logMessages = append(logMessages, fmt.Sprintf("spec.bootstrap.dataSecretName %s, %s required", ptr.Deref(currentCopy.Spec.Bootstrap.DataSecretName, "nil"), ptr.Deref(desiredCopy.Spec.Bootstrap.DataSecretName, "nil")))
conditionMessages = append(conditionMessages, fmt.Sprintf("spec.bootstrap.dataSecretName %s, %s required", ptr.Deref(currentCopy.Spec.Bootstrap.DataSecretName, "nil"), ptr.Deref(desiredCopy.Spec.Bootstrap.DataSecretName, "nil")))
}
}

if !reflect.DeepEqual(currentCopy.Spec.InfrastructureRef, desiredCopy.Spec.InfrastructureRef) {
logMessages = append(logMessages, fmt.Sprintf("spec.infrastructureRef %s %s, %s %s required", currentCopy.Spec.InfrastructureRef.Kind, currentCopy.Spec.InfrastructureRef.Name, desiredCopy.Spec.InfrastructureRef.Kind, desiredCopy.Spec.InfrastructureRef.Name))
// Note: dropping "Template" suffix because conditions message will surface on machine.
conditionMessages = append(conditionMessages, fmt.Sprintf("%s is not up-to-date", strings.TrimSuffix(currentCopy.Spec.InfrastructureRef.Kind, clusterv1.TemplateSuffix)))
}

if !reflect.DeepEqual(currentCopy.Spec.FailureDomain, desiredCopy.Spec.FailureDomain) {
logMessages = append(logMessages, fmt.Sprintf("spec.failureDomain %s, %s required", ptr.Deref(currentCopy.Spec.FailureDomain, "nil"), ptr.Deref(desiredCopy.Spec.FailureDomain, "nil")))
conditionMessages = append(conditionMessages, fmt.Sprintf("Failure domain %s, %s required", ptr.Deref(currentCopy.Spec.FailureDomain, "nil"), ptr.Deref(desiredCopy.Spec.FailureDomain, "nil")))
}

if len(logMessages) > 0 || len(conditionMessages) > 0 {
return false, logMessages, conditionMessages
}

return true, nil, nil
}

// MachineTemplateDeepCopyRolloutFields copies a MachineTemplateSpec
Expand All @@ -386,6 +423,9 @@ func EqualMachineTemplate(template1, template2 *clusterv1.MachineTemplateSpec) (
func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpec) *clusterv1.MachineTemplateSpec {
templateCopy := template.DeepCopy()

// Moving MD from one cluster to another is not supported.
templateCopy.Spec.ClusterName = ""

// Drop labels and annotations
templateCopy.Labels = nil
templateCopy.Annotations = nil
Expand Down Expand Up @@ -413,7 +453,7 @@ func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpe
// NOTE: If we find a matching MachineSet which only differs in in-place mutable fields we can use it to
// fulfill the intent of the MachineDeployment by just updating the MachineSet to propagate in-place mutable fields.
// Thus we don't have to create a new MachineSet and we can avoid an unnecessary rollout.
// NOTE: Even after we changed EqualMachineTemplate to ignore fields that are propagated in-place we can guarantee that if there exists a "new machineset"
// NOTE: Even after we changed MachineTemplateUpToDate to ignore fields that are propagated in-place we can guarantee that if there exists a "new machineset"
// using the old logic then a new machineset will definitely exist using the new logic. The new logic is looser. Therefore, we will
// not face a case where there exists a machine set matching the old logic but there does not exist a machineset matching the new logic.
// In fact previously not matching MS can now start matching the target. Since there could be multiple matches, lets choose the
Expand All @@ -432,19 +472,16 @@ func FindNewMachineSet(deployment *clusterv1.MachineDeployment, msList []*cluste
var matchingMachineSets []*clusterv1.MachineSet
var diffs []string
for _, ms := range msList {
equal, diff, err := EqualMachineTemplate(&ms.Spec.Template, &deployment.Spec.Template)
if err != nil {
return nil, "", errors.Wrapf(err, "failed to compare MachineDeployment spec template with MachineSet %s", ms.Name)
}
if equal {
upToDate, logMessages, _ := MachineTemplateUpToDate(&ms.Spec.Template, &deployment.Spec.Template)
if upToDate {
matchingMachineSets = append(matchingMachineSets, ms)
} else {
diffs = append(diffs, fmt.Sprintf("MachineSet %s: diff: %s", ms.Name, diff))
diffs = append(diffs, fmt.Sprintf("MachineSet %s: diff: %s", ms.Name, strings.Join(logMessages, ", ")))
}
}

if len(matchingMachineSets) == 0 {
return nil, fmt.Sprintf("couldn't find MachineSet matching MachineDeployment spec template: %s", strings.Join(diffs, ",")), nil
return nil, fmt.Sprintf("couldn't find MachineSet matching MachineDeployment spec template: %s", strings.Join(diffs, "; ")), nil
}

// If RolloutAfter is not set, pick the first matching MachineSet.
Expand Down
Loading