From 5acb252c0e98fa13f867a652f83ca75a5a6da0b0 Mon Sep 17 00:00:00 2001 From: Jordan Olshevski Date: Thu, 14 Dec 2023 00:54:07 +0000 Subject: [PATCH] Correct logic around synthesis lifecycle --- internal/controllers/synthesis/integration_test.go | 2 +- internal/controllers/synthesis/lifecycle.go | 6 +++--- internal/controllers/synthesis/pod.go | 10 +++------- internal/controllers/synthesis/status.go | 6 ++---- internal/testutil/testutil.go | 2 +- 5 files changed, 10 insertions(+), 16 deletions(-) diff --git a/internal/controllers/synthesis/integration_test.go b/internal/controllers/synthesis/integration_test.go index b448d79f..face4c7a 100644 --- 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 index ed44e403..b40f9fa4 100644 --- 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 index c5d16e81..e55ab528 100644 --- 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 index 70b91bf4..3e9815a9 100644 --- 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 index d3e2d075..e1b79b94 100644 --- 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