Correct logic around synthesis lifecycle

Azure · Dec 14, 2023 · 5acb252 · 5acb252
1 parent 4a327ad
commit 5acb252
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 16 deletions.
diff --git a/internal/controllers/synthesis/integration_test.go b/internal/controllers/synthesis/integration_test.go
@@ -204,7 +204,7 @@ func TestControllerSynthesizerRollout(t *testing.T) {
 	// The first synthesizer update should be applied to the composition
 	testutil.Eventually(t, func() bool {
 		require.NoError(t, client.IgnoreNotFound(cli.Get(ctx, client.ObjectKeyFromObject(comp), comp)))
-		return comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedSynthesizerGeneration >= syn.Generation
+		return comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedSynthesizerGeneration == syn.Generation
 	})
 
 	// Wait for the informer cache to know about the last update

diff --git a/internal/controllers/synthesis/lifecycle.go b/internal/controllers/synthesis/lifecycle.go
@@ -93,7 +93,7 @@ func (c *podLifecycleController) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	// No need to create a pod if everything is in sync
-	if comp.Status.CurrentState != nil && comp.Status.CurrentState.ResourceSlices != nil {
+	if comp.Status.CurrentState != nil && comp.Status.CurrentState.Synthesized {
 		return ctrl.Result{}, nil
 	}
 
@@ -117,7 +117,7 @@ func (c *podLifecycleController) shouldDeletePod(logger logr.Logger, comp *apiv1
 	var activeLatest bool
 	for _, pod := range pods.Items {
 		pod := pod
-		if pod.DeletionTimestamp != nil || !podDerivedFrom(comp, syn, &pod) {
+		if pod.DeletionTimestamp != nil || !podDerivedFrom(comp, &pod) {
 			continue
 		}
 		if activeLatest {
@@ -131,7 +131,7 @@ func (c *podLifecycleController) shouldDeletePod(logger logr.Logger, comp *apiv1
 	for _, pod := range pods.Items {
 		pod := pod
 		reason, shouldDelete := c.podStatusTerminal(&pod)
-		isCurrent := podDerivedFrom(comp, syn, &pod) && len(comp.Status.CurrentState.ResourceSlices) != 0
+		isCurrent := podDerivedFrom(comp, &pod)
 
 		// If the current pod is being deleted it's safe to create a new one if needed
 		// Avoid getting stuck by pods that fail to delete

diff --git a/internal/controllers/synthesis/pod.go b/internal/controllers/synthesis/pod.go
@@ -92,15 +92,11 @@ func newPod(cfg *Config, scheme *runtime.Scheme, comp *apiv1.Composition, syn *a
 	return pod
 }
 
-func podDerivedFrom(comp *apiv1.Composition, syn *apiv1.Synthesizer, pod *corev1.Pod) bool {
+func podDerivedFrom(comp *apiv1.Composition, pod *corev1.Pod) bool {
 	if pod.Annotations == nil {
 		return false
 	}
 
-	var (
-		compGen, _ = strconv.ParseInt(pod.Annotations["eno.azure.io/composition-generation"], 10, 0)
-		synGen, _  = strconv.ParseInt(pod.Annotations["eno.azure.io/synthesizer-generation"], 10, 0)
-	)
-
-	return compGen == comp.Generation && synGen == syn.Generation
+	compGen, _ := strconv.ParseInt(pod.Annotations["eno.azure.io/composition-generation"], 10, 0)
+	return compGen == comp.Generation
 }
diff --git a/internal/controllers/synthesis/status.go b/internal/controllers/synthesis/status.go
@@ -64,7 +64,7 @@ func (c *statusController) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("getting synthesizer: %w", err)
 	}
-	logger = logger.WithValues("synthesizerName", syn.Name)
+	logger = logger.WithValues("synthesizerName", syn.Name, "podName", pod.Name)
 
 	// Update composition status
 	var (
@@ -97,7 +97,5 @@ func (c *statusController) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
 func shouldWriteStatus(comp *apiv1.Composition, podCompGen, podSynGen int64) bool {
 	current := comp.Status.CurrentState
-	isOldPod := current == nil || current.ObservedCompositionGeneration != podCompGen
-	isInSync := current != nil && current.PodCreation != nil
-	return !isOldPod && !isInSync
+	return current == nil || (current.ObservedCompositionGeneration == podCompGen && (current.PodCreation == nil || current.ObservedSynthesizerGeneration == 0))
 }
diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go
@@ -255,7 +255,7 @@ func NewPodController(t testing.TB, mgr ctrl.Manager, fn func(*apiv1.Composition
 			// Write all of the resource slices, update the resource slice count accordingly
 			// TODO: We need a controller to remove failed/outdated resource slice writes
 			sliceRefs := []*apiv1.ResourceSliceRef{}
-			if comp.Status.CurrentState.ResourceSlices == nil {
+			if !comp.Status.CurrentState.Synthesized {
 				for _, slice := range slices {
 					cp := slice.DeepCopy()
 					cp.Spec.CompositionGeneration = comp.Generation