[BACKPORT] Ensure re-sync is triggered

Backport from 61e76e2

Signed-off-by: David Cassany <[email protected]>

Author: davidcassany

controllers/managedosversionchannel_controller.go

@@ -187,12 +187,12 @@ func (r *ManagedOSVersionChannelReconciler) reconcile(ctx context.Context, manag
 
 	if readyCondition.Status == metav1.ConditionTrue {
 		logger.Info("synchronization already done", "lastSync", lastSync)
-		return ctrl.Result{}, nil
+		return ctrl.Result{RequeueAfter: time.Until(lastSync.Add(interval))}, nil
 	}
 
 	if managedOSVersionChannel.Status.FailedSynchronizationAttempts > maxConscutiveFailures {
 		logger.Error(fmt.Errorf("stop retrying"), "sychronization failed consecutively too many times", "failed attempts", managedOSVersionChannel.Status.FailedSynchronizationAttempts)
-		return ctrl.Result{}, nil
+		return ctrl.Result{RequeueAfter: time.Until(lastSync.Add(interval))}, nil
 	}
 
 	pod := &corev1.Pod{}
@@ -453,6 +453,11 @@ func (r *ManagedOSVersionChannelReconciler) createSyncerPod(ctx context.Context,
 	return nil
 }
 
+// filterChannelEvents is a method that filters reconcile requests events for the channels reconciler.
+// ManagedOSVersionChannelReconciler watches channels and owned pods. This filter ignores pod
+// create/delete/generic events and only reacts on pod phase updates. Channel update events are
+// only reconciled if the update includes a new generation of the resource, all other events are not
+// filtered.
 func filterChannelEvents() predicate.Funcs {
 	return predicate.Funcs{
 		// Process only new generation updates for channels and new phase for pods updates
@@ -500,5 +505,16 @@ func filterChannelEvents() predicate.Funcs {
 			logger.V(log.DebugDepth).Info("Processing generic event", "Obj", e.Object.GetName())
 			return true
 		},
+		// Ignore pods creation
+		CreateFunc: func(e event.CreateEvent) bool {
+			logger := ctrl.LoggerFrom(context.Background())
+
+			if _, ok := e.Object.(*corev1.Pod); ok {
+				return false
+			}
+			// Return true in case it watches other types
+			logger.V(log.DebugDepth).Info("Processing create event", "Obj", e.Object.GetName())
+			return true
+		},
 	}
 }

controllers/managedosversionchannel_controller_test.go

@@ -193,10 +193,11 @@ var _ = Describe("reconcile managed os version channel", func() {
 			Namespace: pod.Namespace,
 		}, pod)).NotTo(Succeed())
 
-		// No re-sync can be triggered before the minium time between syncs
+		// Re-sync is triggered to interval
 		res, err = r.Reconcile(ctx, reconcile.Request{NamespacedName: name})
 		Expect(err).ToNot(HaveOccurred())
-		Expect(res.RequeueAfter).To(Equal(0 * time.Second))
+		Expect(res.RequeueAfter).To(BeNumerically("<", 1*time.Minute))
+		Expect(res.RequeueAfter).To(BeNumerically(">", 59*time.Second))
 	})
 
 	It("should reconcile managed os version channel object without a type", func() {
@@ -533,14 +534,14 @@ var _ = Describe("managed os version channel controller integration tests", func
 				Namespace: ch.Namespace,
 			}, pod)
 			return err != nil && apierrors.IsNotFound(err)
-		}, 12*time.Second, 2*time.Second).Should(BeTrue())
+		}, 6*time.Second, 1*time.Second).Should(BeTrue())
 
 		// Simulate a channel content change
 		syncerProvider.SetJSON(updatedJSON)
 
-		// Updating the channel after the minimum time between syncs causes an automatic update
+		// Updating the channel causes an automatic update
 		patchBase := client.MergeFrom(ch.DeepCopy())
-		ch.Spec.SyncInterval = "10m"
+		ch.Spec.SyncInterval = "10s"
 		Expect(cl.Patch(ctx, ch, patchBase)).To(Succeed())
 
 		// Pod is created
@@ -550,7 +551,7 @@ var _ = Describe("managed os version channel controller integration tests", func
 				Namespace: ch.Namespace,
 			}, pod)
 			return err == nil
-		}, 12*time.Second, 2*time.Second).Should(BeTrue())
+		}, 6*time.Second, 1*time.Second).Should(BeTrue())
 		setPodPhase(pod, corev1.PodSucceeded)
 
 		// New added versions are synced
@@ -560,14 +561,62 @@ var _ = Describe("managed os version channel controller integration tests", func
 				Namespace: ch.Namespace,
 			}, managedOSVersion)
 			return err == nil
-		}, 12*time.Second, 2*time.Second).Should(BeTrue())
+		}, 6*time.Second, 1*time.Second).Should(BeTrue())
 
 		// After channel update already existing versions were patched
 		Expect(cl.Get(ctx, client.ObjectKey{
 			Name:      "v0.1.0",
 			Namespace: ch.Namespace,
 		}, managedOSVersion)).To(Succeed())
 		Expect(managedOSVersion.Spec.Version).To(Equal("v0.1.0-patched"))
+
+		// Pod is deleted
+		Eventually(func() bool {
+			err := cl.Get(ctx, client.ObjectKey{
+				Name:      ch.Name,
+				Namespace: ch.Namespace,
+			}, pod)
+			return err != nil && apierrors.IsNotFound(err)
+		}, 2*time.Second, 1*time.Second).Should(BeTrue())
+
+		Expect(cl.Get(ctx, client.ObjectKey{
+			Name:      ch.Name,
+			Namespace: ch.Namespace,
+		}, ch)).To(Succeed())
+
+		// Simulate another channel content change
+		syncerProvider.SetJSON(syncJSON)
+
+		timeout := time.Until(ch.Status.LastSyncedTime.Add(10*time.Second)) - 1*time.Second
+
+		// No pod is created during the interval
+		Consistently(func() bool {
+			err := cl.Get(ctx, client.ObjectKey{
+				Name:      ch.Name,
+				Namespace: ch.Namespace,
+			}, pod)
+			return apierrors.IsNotFound(err)
+		}, timeout, 1*time.Second).Should(BeTrue())
+
+		// Pod is created once the resync is triggered automatically
+		Eventually(func() bool {
+			err := cl.Get(ctx, client.ObjectKey{
+				Name:      ch.Name,
+				Namespace: ch.Namespace,
+			}, pod)
+			return err == nil
+		}, 4*time.Second, 1*time.Second).Should(BeTrue())
+		setPodPhase(pod, corev1.PodSucceeded)
+
+		// v0.1.0 is updated
+		Eventually(func() bool {
+			Expect(cl.Get(ctx, client.ObjectKey{
+				Name:      "v0.1.0",
+				Namespace: ch.Namespace,
+			}, managedOSVersion)).To(Succeed())
+
+			return managedOSVersion.Spec.Version == "v0.1.0"
+		}, 6*time.Second, 1*time.Second).Should(BeTrue())
 	})
 
 	It("should not reconcile again if it errors during pod lifecycle", func() {