From 2e4f6d9bc3ad23be0663ac949ccc9dae4a3ebf65 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Thu, 16 Feb 2023 06:55:55 +0100 Subject: [PATCH] KCP: Log the reason of a Machine rollout --- .../kubeadm/internal/control_plane.go | 26 ++- .../internal/controllers/controller.go | 15 +- controlplane/kubeadm/internal/filters.go | 178 +++++++++++------- controlplane/kubeadm/internal/filters_test.go | 106 ++++++----- 4 files changed, 199 insertions(+), 126 deletions(-) diff --git a/controlplane/kubeadm/internal/control_plane.go b/controlplane/kubeadm/internal/control_plane.go index 7e7ba17d65e7..958aea89cbd6 100644 --- a/controlplane/kubeadm/internal/control_plane.go +++ b/controlplane/kubeadm/internal/control_plane.go @@ -167,22 +167,34 @@ func (c *ControlPlane) GetKubeadmConfig(machineName string) (*bootstrapv1.Kubead } // MachinesNeedingRollout return a list of machines that need to be rolled out. -func (c *ControlPlane) MachinesNeedingRollout() collections.Machines { +func (c *ControlPlane) MachinesNeedingRollout() (collections.Machines, map[string]string) { // Ignore machines to be deleted. machines := c.Machines.Filter(collections.Not(collections.HasDeletionTimestamp)) // Return machines if they are scheduled for rollout or if with an outdated configuration. - return machines.Filter( - NeedsRollout(&c.reconciliationTime, c.KCP.Spec.RolloutAfter, c.KCP.Spec.RolloutBefore, c.InfraResources, c.KubeadmConfigs, c.KCP), - ) + machinesNeedingRollout := make(collections.Machines, len(machines)) + rolloutReasons := map[string]string{} + for _, m := range machines { + reason, needsRollout := NeedsRollout(&c.reconciliationTime, c.KCP.Spec.RolloutAfter, c.KCP.Spec.RolloutBefore, c.InfraResources, c.KubeadmConfigs, c.KCP, m) + if needsRollout { + machinesNeedingRollout.Insert(m) + rolloutReasons[m.Name] = reason + } + } + return machinesNeedingRollout, rolloutReasons } // UpToDateMachines returns the machines that are up to date with the control // plane's configuration and therefore do not require rollout. func (c *ControlPlane) UpToDateMachines() collections.Machines { - return c.Machines.Filter( - collections.Not(NeedsRollout(&c.reconciliationTime, c.KCP.Spec.RolloutAfter, c.KCP.Spec.RolloutBefore, c.InfraResources, c.KubeadmConfigs, c.KCP)), - ) + upToDateMachines := make(collections.Machines, len(c.Machines)) + for _, m := range c.Machines { + _, needsRollout := NeedsRollout(&c.reconciliationTime, c.KCP.Spec.RolloutAfter, c.KCP.Spec.RolloutBefore, c.InfraResources, c.KubeadmConfigs, c.KCP, m) + if !needsRollout { + upToDateMachines.Insert(m) + } + } + return upToDateMachines } // getInfraResources fetches the external infrastructure resource for each machine in the collection and returns a map of machine.Name -> infraResource. diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go index 2614811f3727..8b818204d4d1 100644 --- a/controlplane/kubeadm/internal/controllers/controller.go +++ b/controlplane/kubeadm/internal/controllers/controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "strings" "time" "github.com/blang/semver" @@ -393,12 +394,16 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, controlPl } // Control plane machines rollout due to configuration changes (e.g. upgrades) takes precedence over other operations. - needRollout := controlPlane.MachinesNeedingRollout() + machinesNeedingRollout, rolloutReasons := controlPlane.MachinesNeedingRollout() switch { - case len(needRollout) > 0: - log.Info("Rolling out Control Plane machines", "needRollout", needRollout.Names()) - conditions.MarkFalse(controlPlane.KCP, controlplanev1.MachinesSpecUpToDateCondition, controlplanev1.RollingUpdateInProgressReason, clusterv1.ConditionSeverityWarning, "Rolling %d replicas with outdated spec (%d replicas up to date)", len(needRollout), len(controlPlane.Machines)-len(needRollout)) - return r.upgradeControlPlane(ctx, controlPlane, needRollout) + case len(machinesNeedingRollout) > 0: + var reasons []string + for _, rolloutReason := range rolloutReasons { + reasons = append(reasons, rolloutReason) + } + log.Info(fmt.Sprintf("Rolling out Control Plane machines: %s", strings.Join(reasons, ",")), "machinesNeedingRollout", machinesNeedingRollout.Names()) + conditions.MarkFalse(controlPlane.KCP, controlplanev1.MachinesSpecUpToDateCondition, controlplanev1.RollingUpdateInProgressReason, clusterv1.ConditionSeverityWarning, "Rolling %d replicas with outdated spec (%d replicas up to date)", len(machinesNeedingRollout), len(controlPlane.Machines)-len(machinesNeedingRollout)) + return r.upgradeControlPlane(ctx, controlPlane, machinesNeedingRollout) default: // make sure last upgrade operation is marked as completed. // NOTE: we are checking the condition already exists in order to avoid to set this condition at the first diff --git a/controlplane/kubeadm/internal/filters.go b/controlplane/kubeadm/internal/filters.go index 5dd8a9e016c1..5c3e9c9ada2c 100644 --- a/controlplane/kubeadm/internal/filters.go +++ b/controlplane/kubeadm/internal/filters.go @@ -18,7 +18,9 @@ package internal import ( "encoding/json" + "fmt" "reflect" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -29,101 +31,137 @@ import ( "sigs.k8s.io/cluster-api/util/collections" ) -// MatchesMachineSpec returns a filter to find all machines that matches with KCP config and do not require any rollout. +// matchesMachineSpec checks if a Machine matches any of a set of KubeadmConfigs and a set of infra machine configs. +// If it doesn't, it returns the reasons why. // Kubernetes version, infrastructure template, and KubeadmConfig field need to be equivalent. // Note: We don't need to compare the entire MachineSpec to determine if a Machine needs to be rolled out, // because all the fields in the MachineSpec, except for version, the infrastructureRef and bootstrap.ConfigRef, are either: // - mutated in-place (ex: NodeDrainTimeout) // - are not dictated by KCP (ex: ProviderID) // - are not relevant for the rollout decision (ex: failureDomain). -func MatchesMachineSpec(infraConfigs map[string]*unstructured.Unstructured, machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) func(machine *clusterv1.Machine) bool { - return collections.And( - collections.MatchesKubernetesVersion(kcp.Spec.Version), - MatchesKubeadmBootstrapConfig(machineConfigs, kcp), - MatchesTemplateClonedFrom(infraConfigs, kcp), - ) +func matchesMachineSpec(infraConfigs map[string]*unstructured.Unstructured, machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane, machine *clusterv1.Machine) (string, bool) { + mismatchReasons := []string{} + + if !collections.MatchesKubernetesVersion(kcp.Spec.Version)(machine) { + machineVersion := "" + if machine != nil && machine.Spec.Version != nil { + machineVersion = *machine.Spec.Version + } + mismatchReasons = append(mismatchReasons, fmt.Sprintf("Machine version %q is not equal to KCP version %q", machineVersion, kcp.Spec.Version)) + } + + if reason, matches := matchesKubeadmBootstrapConfig(machineConfigs, kcp, machine); !matches { + mismatchReasons = append(mismatchReasons, reason) + } + + if reason, matches := matchesTemplateClonedFrom(infraConfigs, kcp, machine); !matches { + mismatchReasons = append(mismatchReasons, reason) + } + + if len(mismatchReasons) > 0 { + return strings.Join(mismatchReasons, ","), false + } + + return "", true } -// NeedsRollout returns a filter to determine if a machine needs rollout. -func NeedsRollout(reconciliationTime, rolloutAfter *metav1.Time, rolloutBefore *controlplanev1.RolloutBefore, infraConfigs map[string]*unstructured.Unstructured, machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) func(machine *clusterv1.Machine) bool { - return collections.Or( - // Machines whose certificates are about to expire. - collections.ShouldRolloutBefore(reconciliationTime, rolloutBefore), - // Machines that are scheduled for rollout (KCP.Spec.RolloutAfter set, the RolloutAfter deadline is expired, and the machine was created before the deadline). - collections.ShouldRolloutAfter(reconciliationTime, rolloutAfter), - // Machines that do not match with KCP config. - collections.Not(MatchesMachineSpec(infraConfigs, machineConfigs, kcp)), - ) +// NeedsRollout checks if a Machine needs to be rolled out and returns the reason why. +func NeedsRollout(reconciliationTime, rolloutAfter *metav1.Time, rolloutBefore *controlplanev1.RolloutBefore, infraConfigs map[string]*unstructured.Unstructured, machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane, machine *clusterv1.Machine) (string, bool) { + rolloutReasons := []string{} + + // Machines whose certificates are about to expire. + if collections.ShouldRolloutBefore(reconciliationTime, rolloutBefore)(machine) { + rolloutReasons = append(rolloutReasons, "certificates will expire soon, rolloutBefore expired") + } + + // Machines that are scheduled for rollout (KCP.Spec.RolloutAfter set, + // the RolloutAfter deadline is expired, and the machine was created before the deadline). + if collections.ShouldRolloutAfter(reconciliationTime, rolloutAfter)(machine) { + rolloutReasons = append(rolloutReasons, "rolloutAfter expired") + } + + // Machines that do not match with KCP config. + if mismatchReason, matches := matchesMachineSpec(infraConfigs, machineConfigs, kcp, machine); !matches { + rolloutReasons = append(rolloutReasons, mismatchReason) + } + + if len(rolloutReasons) > 0 { + return fmt.Sprintf("Machine %s needs rollout: %s", machine.Name, strings.Join(rolloutReasons, ",")), true + } + + return "", false } -// MatchesTemplateClonedFrom returns a filter to find all machines that have a corresponding infrastructure machine that -// matches a given KCP infra template. +// matchesTemplateClonedFrom checks if a Machine has a corresponding infrastructure machine that +// matches a given KCP infra template and if it doesn't match returns the reason why. // Note: Differences to the labels and annotations on the infrastructure machine are not considered for matching // criteria, because changes to labels and annotations are propagated in-place to the infrastructure machines. // TODO: This function will be renamed in a follow-up PR to something better. (ex: MatchesInfraMachine). -func MatchesTemplateClonedFrom(infraConfigs map[string]*unstructured.Unstructured, kcp *controlplanev1.KubeadmControlPlane) collections.Func { - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - infraObj, found := infraConfigs[machine.Name] - if !found { - // Return true here because failing to get infrastructure machine should not be considered as unmatching. - return true - } - - clonedFromName, ok1 := infraObj.GetAnnotations()[clusterv1.TemplateClonedFromNameAnnotation] - clonedFromGroupKind, ok2 := infraObj.GetAnnotations()[clusterv1.TemplateClonedFromGroupKindAnnotation] - if !ok1 || !ok2 { - // All kcp cloned infra machines should have this annotation. - // Missing the annotation may be due to older version machines or adopted machines. - // Should not be considered as mismatch. - return true - } +func matchesTemplateClonedFrom(infraConfigs map[string]*unstructured.Unstructured, kcp *controlplanev1.KubeadmControlPlane, machine *clusterv1.Machine) (string, bool) { + if machine == nil { + return "Machine cannot be compared with KCP.spec.machineTemplate.infrastructureRef: Machine is nil", false + } + infraObj, found := infraConfigs[machine.Name] + if !found { + // Return true here because failing to get infrastructure machine should not be considered as unmatching. + return "", true + } - // Check if the machine's infrastructure reference has been created from the current KCP infrastructure template. - if clonedFromName != kcp.Spec.MachineTemplate.InfrastructureRef.Name || - clonedFromGroupKind != kcp.Spec.MachineTemplate.InfrastructureRef.GroupVersionKind().GroupKind().String() { - return false - } + clonedFromName, ok1 := infraObj.GetAnnotations()[clusterv1.TemplateClonedFromNameAnnotation] + clonedFromGroupKind, ok2 := infraObj.GetAnnotations()[clusterv1.TemplateClonedFromGroupKindAnnotation] + if !ok1 || !ok2 { + // All kcp cloned infra machines should have this annotation. + // Missing the annotation may be due to older version machines or adopted machines. + // Should not be considered as mismatch. + return "", true + } - return true + // Check if the machine's infrastructure reference has been created from the current KCP infrastructure template. + if clonedFromName != kcp.Spec.MachineTemplate.InfrastructureRef.Name || + clonedFromGroupKind != kcp.Spec.MachineTemplate.InfrastructureRef.GroupVersionKind().GroupKind().String() { + return fmt.Sprintf("Infrastructure template on KCP rotated from %s %s to %s %s", + clonedFromGroupKind, clonedFromName, + kcp.Spec.MachineTemplate.InfrastructureRef.GroupVersionKind().GroupKind().String(), kcp.Spec.MachineTemplate.InfrastructureRef.Name), false } + + return "", true } -// MatchesKubeadmBootstrapConfig checks if machine's KubeadmConfigSpec is equivalent with KCP's KubeadmConfigSpec. +// matchesKubeadmBootstrapConfig checks if machine's KubeadmConfigSpec is equivalent with KCP's KubeadmConfigSpec. // Note: Differences to the labels and annotations on the KubeadmConfig are not considered for matching // criteria, because changes to labels and annotations are propagated in-place to KubeadmConfig. -func MatchesKubeadmBootstrapConfig(machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) collections.Func { - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } +func matchesKubeadmBootstrapConfig(machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane, machine *clusterv1.Machine) (string, bool) { + if machine == nil { + return "Machine KubeadmConfig cannot be compared: Machine is nil", false + } - // Check if KCP and machine ClusterConfiguration matches, if not return - if match := matchClusterConfiguration(kcp, machine); !match { - return false - } + // Check if KCP and machine ClusterConfiguration matches, if not return + if !matchClusterConfiguration(kcp, machine) { + return "Machine ClusterConfiguration is outdated", false + } - bootstrapRef := machine.Spec.Bootstrap.ConfigRef - if bootstrapRef == nil { - // Missing bootstrap reference should not be considered as unmatching. - // This is a safety precaution to avoid selecting machines that are broken, which in the future should be remediated separately. - return true - } + bootstrapRef := machine.Spec.Bootstrap.ConfigRef + if bootstrapRef == nil { + // Missing bootstrap reference should not be considered as unmatching. + // This is a safety precaution to avoid selecting machines that are broken, which in the future should be remediated separately. + return "", true + } - machineConfig, found := machineConfigs[machine.Name] - if !found { - // Return true here because failing to get KubeadmConfig should not be considered as unmatching. - // This is a safety precaution to avoid rolling out machines if the client or the api-server is misbehaving. - return true - } + machineConfig, found := machineConfigs[machine.Name] + if !found { + // Return true here because failing to get KubeadmConfig should not be considered as unmatching. + // This is a safety precaution to avoid rolling out machines if the client or the api-server is misbehaving. + return "", true + } - // Check if KCP and machine InitConfiguration or JoinConfiguration matches - // NOTE: only one between init configuration and join configuration is set on a machine, depending - // on the fact that the machine was the initial control plane node or a joining control plane node. - return matchInitOrJoinConfiguration(machineConfig, kcp) + // Check if KCP and machine InitConfiguration or JoinConfiguration matches + // NOTE: only one between init configuration and join configuration is set on a machine, depending + // on the fact that the machine was the initial control plane node or a joining control plane node. + if !matchInitOrJoinConfiguration(machineConfig, kcp) { + return "Machine InitConfiguration or JoinConfiguration are outdated", false } + + return "", true } // matchClusterConfiguration verifies if KCP and machine ClusterConfiguration matches. diff --git a/controlplane/kubeadm/internal/filters_test.go b/controlplane/kubeadm/internal/filters_test.go index 79211ccd3ee9..b110b058828f 100644 --- a/controlplane/kubeadm/internal/filters_test.go +++ b/controlplane/kubeadm/internal/filters_test.go @@ -600,8 +600,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { machineConfigs := map[string]*bootstrapv1.KubeadmConfig{ m.Name: {}, } - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("returns false if ClusterConfiguration is NOT equal", func(t *testing.T) { g := NewWithT(t) @@ -624,8 +625,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { machineConfigs := map[string]*bootstrapv1.KubeadmConfig{ m.Name: {}, } - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeFalse()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeFalse()) + g.Expect(reason).To(Equal("Machine ClusterConfiguration is outdated")) }) t.Run("returns true if InitConfiguration is equal", func(t *testing.T) { g := NewWithT(t) @@ -673,8 +675,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, }, } - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("returns false if InitConfiguration is NOT equal", func(t *testing.T) { g := NewWithT(t) @@ -726,8 +729,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, }, } - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeFalse()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeFalse()) + g.Expect(reason).To(Equal("Machine InitConfiguration or JoinConfiguration are outdated")) }) t.Run("returns true if JoinConfiguration is equal", func(t *testing.T) { g := NewWithT(t) @@ -775,8 +779,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, }, } - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("returns false if JoinConfiguration is NOT equal", func(t *testing.T) { g := NewWithT(t) @@ -828,8 +833,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, }, } - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeFalse()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeFalse()) + g.Expect(reason).To(Equal("Machine InitConfiguration or JoinConfiguration are outdated")) }) t.Run("returns false if some other configurations are not equal", func(t *testing.T) { g := NewWithT(t) @@ -878,8 +884,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, }, } - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeFalse()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeFalse()) + g.Expect(reason).To(Equal("Machine InitConfiguration or JoinConfiguration are outdated")) }) t.Run("should match on labels and annotations", func(t *testing.T) { kcp := &controlplanev1.KubeadmControlPlane{ @@ -941,32 +948,36 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { g := NewWithT(t) machineConfigs[m.Name].Annotations = nil machineConfigs[m.Name].Labels = nil - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("by returning true if only labels don't match", func(t *testing.T) { g := NewWithT(t) machineConfigs[m.Name].Annotations = kcp.Spec.MachineTemplate.ObjectMeta.Annotations machineConfigs[m.Name].Labels = nil - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("by returning true if only annotations don't match", func(t *testing.T) { g := NewWithT(t) machineConfigs[m.Name].Annotations = nil machineConfigs[m.Name].Labels = kcp.Spec.MachineTemplate.ObjectMeta.Labels - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("by returning true if both labels and annotations match", func(t *testing.T) { g := NewWithT(t) machineConfigs[m.Name].Labels = kcp.Spec.MachineTemplate.ObjectMeta.Labels machineConfigs[m.Name].Annotations = kcp.Spec.MachineTemplate.ObjectMeta.Annotations - f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesKubeadmBootstrapConfig(machineConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) }) } @@ -974,9 +985,9 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { func TestMatchesTemplateClonedFrom(t *testing.T) { t.Run("nil machine returns false", func(t *testing.T) { g := NewWithT(t) - g.Expect( - MatchesTemplateClonedFrom(nil, nil)(nil), - ).To(BeFalse()) + reason, match := matchesTemplateClonedFrom(nil, nil, nil) + g.Expect(match).To(BeFalse()) + g.Expect(reason).To(Equal("Machine cannot be compared with KCP.spec.machineTemplate.infrastructureRef: Machine is nil")) }) t.Run("returns true if machine not found", func(t *testing.T) { @@ -992,9 +1003,9 @@ func TestMatchesTemplateClonedFrom(t *testing.T) { }, }, } - g.Expect( - MatchesTemplateClonedFrom(map[string]*unstructured.Unstructured{}, kcp)(machine), - ).To(BeTrue()) + reason, match := matchesTemplateClonedFrom(map[string]*unstructured.Unstructured{}, kcp, machine) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("matches labels or annotations", func(t *testing.T) { @@ -1052,8 +1063,9 @@ func TestMatchesTemplateClonedFrom(t *testing.T) { clusterv1.TemplateClonedFromGroupKindAnnotation: "GenericMachineTemplate.generic.io", }) infraConfigs[m.Name].SetLabels(nil) - f := MatchesTemplateClonedFrom(infraConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesTemplateClonedFrom(infraConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("by returning true if only labels don't match", func(t *testing.T) { @@ -1064,8 +1076,9 @@ func TestMatchesTemplateClonedFrom(t *testing.T) { "test": "annotation", }) infraConfigs[m.Name].SetLabels(nil) - f := MatchesTemplateClonedFrom(infraConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesTemplateClonedFrom(infraConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("by returning true if only annotations don't match", func(t *testing.T) { @@ -1075,8 +1088,9 @@ func TestMatchesTemplateClonedFrom(t *testing.T) { clusterv1.TemplateClonedFromGroupKindAnnotation: "GenericMachineTemplate.generic.io", }) infraConfigs[m.Name].SetLabels(kcp.Spec.MachineTemplate.ObjectMeta.Labels) - f := MatchesTemplateClonedFrom(infraConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesTemplateClonedFrom(infraConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) t.Run("by returning true if both labels and annotations match", func(t *testing.T) { @@ -1087,8 +1101,9 @@ func TestMatchesTemplateClonedFrom(t *testing.T) { "test": "annotation", }) infraConfigs[m.Name].SetLabels(kcp.Spec.MachineTemplate.ObjectMeta.Labels) - f := MatchesTemplateClonedFrom(infraConfigs, kcp) - g.Expect(f(m)).To(BeTrue()) + reason, match := matchesTemplateClonedFrom(infraConfigs, kcp, m) + g.Expect(match).To(BeTrue()) + g.Expect(reason).To(BeEmpty()) }) }) } @@ -1120,9 +1135,10 @@ func TestMatchesTemplateClonedFrom_WithClonedFromAnnotations(t *testing.T) { }, } tests := []struct { - name string - annotations map[string]interface{} - expectMatch bool + name string + annotations map[string]interface{} + expectMatch bool + expectReason string }{ { name: "returns true if annotations don't exist", @@ -1135,7 +1151,8 @@ func TestMatchesTemplateClonedFrom_WithClonedFromAnnotations(t *testing.T) { clusterv1.TemplateClonedFromNameAnnotation: "barfoo1", clusterv1.TemplateClonedFromGroupKindAnnotation: "barfoo2", }, - expectMatch: false, + expectMatch: false, + expectReason: "Infrastructure template on KCP rotated from barfoo2 barfoo1 to GenericMachineTemplate.generic.io infra-foo", }, { name: "returns false if TemplateClonedFromNameAnnotation matches but TemplateClonedFromGroupKindAnnotation doesn't", @@ -1143,7 +1160,8 @@ func TestMatchesTemplateClonedFrom_WithClonedFromAnnotations(t *testing.T) { clusterv1.TemplateClonedFromNameAnnotation: "infra-foo", clusterv1.TemplateClonedFromGroupKindAnnotation: "barfoo2", }, - expectMatch: false, + expectMatch: false, + expectReason: "Infrastructure template on KCP rotated from barfoo2 infra-foo to GenericMachineTemplate.generic.io infra-foo", }, { name: "returns true if both annotations match", @@ -1171,9 +1189,9 @@ func TestMatchesTemplateClonedFrom_WithClonedFromAnnotations(t *testing.T) { }, }, } - g.Expect( - MatchesTemplateClonedFrom(infraConfigs, kcp)(machine), - ).To(Equal(tt.expectMatch)) + reason, match := matchesTemplateClonedFrom(infraConfigs, kcp, machine) + g.Expect(match).To(Equal(tt.expectMatch)) + g.Expect(reason).To(Equal(tt.expectReason)) }) } }