Merge pull request #3277 from fabriziopandini/cleanup-machinesNeedingRollout

🌱 Cleanup machinesNeedingRollout in KCP

Author: k8s-ci-robot

controlplane/kubeadm/controllers/helpers.go

@@ -259,20 +259,14 @@ func (r *KubeadmControlPlaneReconciler) generateMachine(ctx context.Context, kcp
 
 // machinesNeedingRollout return a list of machines that need to be rolled out.
 func (r *KubeadmControlPlaneReconciler) machinesNeedingRollout(ctx context.Context, c *internal.ControlPlane) internal.FilterableMachineCollection {
-	now := metav1.Now()
-
 	// Ignore machines to be deleted.
 	machines := c.Machines.Filter(machinefilters.Not(machinefilters.HasDeletionTimestamp))
 
-	// Return machines if their creation timestamp is older than the KCP.Spec.UpgradeAfter, or any machine with an outdated configuration.
-	if c.KCP.Spec.UpgradeAfter != nil && c.KCP.Spec.UpgradeAfter.Before(&now) {
-		return machines.Filter(machinefilters.Or(
-			// Machines that are old.
-			machinefilters.OlderThan(c.KCP.Spec.UpgradeAfter),
-			// Machines that do not match with KCP config.
-			machinefilters.Not(machinefilters.MatchesKCPConfiguration(ctx, r.Client, *c.KCP, *c.Cluster)),
-		))
-	}
-
-	return machines.Filter(machinefilters.Not(machinefilters.MatchesKCPConfiguration(ctx, r.Client, *c.KCP, *c.Cluster)))
+	// Return machines if they are scheduled for rollout or if with an outdated configuration.
+	return machines.AnyFilter(
+		// Machines that are scheduled for rollout (KCP.Spec.UpgradeAfter set, the UpgradeAfter deadline is expired, and the machine was created before the deadline).
+		machinefilters.ShouldRolloutAfter(c.KCP.Spec.UpgradeAfter),
+		// Machines that do not match with KCP config.
+		machinefilters.Not(machinefilters.MatchesKCPConfiguration(ctx, r.Client, *c.KCP, *c.Cluster)),
+	)
 }

controlplane/kubeadm/internal/machinefilters/machine_filters.go

@@ -166,13 +166,17 @@ func IsReady() Func {
 	}
 }
 
-// OlderThan returns a filter to find all machines
-// that have a CreationTimestamp earlier than the given time.
-func OlderThan(t *metav1.Time) Func {
+// ShouldRolloutAfter returns a filter to find all machines
+// that should rollout after the given time.
+func ShouldRolloutAfter(t *metav1.Time) Func {
+	now := metav1.Now()
 	return func(machine *clusterv1.Machine) bool {
 		if machine == nil {
 			return false
 		}
+		if t == nil || !t.Before(&now) {
+			return false
+		}
 		return machine.CreationTimestamp.Before(t)
 	}
 }

controlplane/kubeadm/internal/machinefilters/machine_filters_test.go

@@ -118,27 +118,33 @@ func TestMatchesConfigurationHash(t *testing.T) {
 	})
 }
 
-func TestOlderThan(t *testing.T) {
-	t.Run("machine with creation timestamp older than given returns true", func(t *testing.T) {
+func TestShouldRolloutAfter(t *testing.T) {
+	t.Run("if upgradeAfter is nil, return false", func(t *testing.T) {
 		g := NewWithT(t)
 		m := &clusterv1.Machine{}
-		m.SetCreationTimestamp(metav1.NewTime(time.Now().Add(-1 * time.Hour)))
-		now := metav1.Now()
-		g.Expect(machinefilters.OlderThan(&now)(m)).To(BeTrue())
+		m.SetCreationTimestamp(metav1.NewTime(time.Now().Add(-2 * time.Hour)))
+		g.Expect(machinefilters.ShouldRolloutAfter(nil)(m)).To(BeFalse())
 	})
-	t.Run("machine with creation timestamp equal to given returns false", func(t *testing.T) {
+	t.Run("if upgradeAfter is in the future, return false", func(t *testing.T) {
 		g := NewWithT(t)
 		m := &clusterv1.Machine{}
-		now := metav1.Now()
-		m.SetCreationTimestamp(now)
-		g.Expect(machinefilters.OlderThan(&now)(m)).To(BeFalse())
+		m.SetCreationTimestamp(metav1.NewTime(time.Now().Add(-2 * time.Hour)))
+		upgradeAfter := metav1.NewTime(time.Now().Add(+1 * time.Hour)) // upgrade after in the future
+		g.Expect(machinefilters.ShouldRolloutAfter(&upgradeAfter)(m)).To(BeFalse())
 	})
-	t.Run("machine with creation timestamp after given returns false", func(t *testing.T) {
+	t.Run("if upgradeAfter is in the past and the machine was created before upgradeAfter, return true", func(t *testing.T) {
 		g := NewWithT(t)
 		m := &clusterv1.Machine{}
-		m.SetCreationTimestamp(metav1.NewTime(time.Now().Add(+1 * time.Hour)))
-		now := metav1.Now()
-		g.Expect(machinefilters.OlderThan(&now)(m)).To(BeFalse())
+		m.SetCreationTimestamp(metav1.NewTime(time.Now().Add(-2 * time.Hour))) // machine was created before upgradeAfter
+		upgradeAfter := metav1.NewTime(time.Now().Add(-1 * time.Hour))         // upgrade after in the past
+		g.Expect(machinefilters.ShouldRolloutAfter(&upgradeAfter)(m)).To(BeTrue())
+	})
+	t.Run("if upgradeAfter is in the past and the machine was created after upgradeAfter, return false", func(t *testing.T) {
+		g := NewWithT(t)
+		m := &clusterv1.Machine{}
+		m.SetCreationTimestamp(metav1.NewTime(time.Now().Add(+1 * time.Hour))) // machine was created after upgradeAfter
+		upgradeAfter := metav1.NewTime(time.Now().Add(-1 * time.Hour))         // upgrade after in the past
+		g.Expect(machinefilters.ShouldRolloutAfter(&upgradeAfter)(m)).To(BeFalse())
 	})
 }