From 1218f1452ce80a9e2ad1228371e35ad16d8a672a Mon Sep 17 00:00:00 2001
From: Chih-Sheng Huang <pedoe1989@gmail.com>
Date: Fri, 15 Nov 2024 14:53:46 -0800
Subject: [PATCH] Delete the composition when synthesizer is missing (#247)

Resolves #227

Delete the composition when synthesizer is missing
---
 internal/controllers/synthesis/lifecycle.go   |  9 ++-
 .../controllers/synthesis/lifecycle_test.go   | 78 +++++++++++++++++++
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/internal/controllers/synthesis/lifecycle.go b/internal/controllers/synthesis/lifecycle.go
index fcf91618..1a724bf8 100644
--- a/internal/controllers/synthesis/lifecycle.go
+++ b/internal/controllers/synthesis/lifecycle.go
@@ -232,8 +232,15 @@ func (c *podLifecycleController) reconcileDeletedComposition(ctx context.Context
 		comp.Status.CurrentSynthesis.ObservedCompositionGeneration = comp.Generation
 		comp.Status.CurrentSynthesis.Ready = nil
 		comp.Status.CurrentSynthesis.UUID = uuid.NewString()
-		comp.Status.CurrentSynthesis.Reconciled = nil
 		now := metav1.Now()
+		if (comp.Status.PreviousSynthesis == nil || comp.Status.PreviousSynthesis.Synthesized == nil) &&
+			comp.Status.CurrentSynthesis.Synthesized == nil {
+			// In this case, the composition is not reconciling due to the synthesizer is missing and the composition finalizer should be removed.
+			// If not, the composition will stuck in waiting for reconciliation to be completed and the it can't be deleted.
+			comp.Status.CurrentSynthesis.Reconciled = &now
+		} else {
+			comp.Status.CurrentSynthesis.Reconciled = nil
+		}
 		comp.Status.CurrentSynthesis.Synthesized = &now
 		err := c.client.Status().Update(ctx, comp)
 		if err != nil {
diff --git a/internal/controllers/synthesis/lifecycle_test.go b/internal/controllers/synthesis/lifecycle_test.go
index f800f219..e659e0f8 100644
--- a/internal/controllers/synthesis/lifecycle_test.go
+++ b/internal/controllers/synthesis/lifecycle_test.go
@@ -102,6 +102,84 @@ func TestCompositionDeletion(t *testing.T) {
 	})
 }
 
+// TestDeleteCompositionWhenSynthesizerMissing proves that a composition's finalizer will be removed if the synthesizer is missing.
+func TestDeleteCompositionWhenSynthesizerMissing(t *testing.T) {
+	ctx := testutil.NewContext(t)
+	mgr := testutil.NewManager(t)
+	cli := mgr.GetClient()
+
+	testutil.WithFakeExecutor(t, mgr, func(ctx context.Context, s *apiv1.Synthesizer, input *krmv1.ResourceList) (*krmv1.ResourceList, error) {
+		output := &krmv1.ResourceList{}
+		output.Items = []*unstructured.Unstructured{{
+			Object: map[string]any{
+				"apiVersion": "v1",
+				"kind":       "ConfigMap",
+				"metadata": map[string]string{
+					"name":      "test",
+					"namespace": "default",
+				},
+			},
+		}}
+		return output, nil
+	})
+
+	require.NoError(t, NewPodLifecycleController(mgr.Manager, minimalTestConfig))
+	require.NoError(t, NewSliceCleanupController(mgr.Manager))
+	require.NoError(t, flowcontrol.NewSynthesisConcurrencyLimiter(mgr.Manager, 10, 0))
+	mgr.Start(t)
+
+	syn := &apiv1.Synthesizer{}
+	syn.Name = "test-syn-1"
+	syn.Spec.Image = "initial-image"
+	require.NoError(t, cli.Create(ctx, syn))
+
+	comp := &apiv1.Composition{}
+	comp.Name = "test-comp"
+	comp.Namespace = "default"
+	comp.Spec.Synthesizer.Name = syn.Name
+	require.NoError(t, cli.Create(ctx, comp))
+
+	// Create the composition's resource slice
+	testutil.Eventually(t, func() bool {
+		require.NoError(t, client.IgnoreNotFound(cli.Get(ctx, client.ObjectKeyFromObject(comp), comp)))
+		return comp.Status.CurrentSynthesis != nil && len(comp.Status.CurrentSynthesis.ResourceSlices) > 0
+	})
+	assert.NotNil(t, comp.Status.CurrentSynthesis.Initialized, "initialized timestamp is set")
+
+	// Wait for the resource slice to be created
+	testutil.Eventually(t, func() bool {
+		require.NoError(t, client.IgnoreNotFound(cli.Get(ctx, client.ObjectKeyFromObject(comp), comp)))
+		return comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.ResourceSlices != nil
+	})
+
+	// Delete the composition
+	require.NoError(t, cli.Delete(ctx, comp))
+	deleteGen := comp.Generation
+
+	// The generation should be updated
+	testutil.Eventually(t, func() bool {
+		require.NoError(t, client.IgnoreNotFound(cli.Get(ctx, client.ObjectKeyFromObject(comp), comp)))
+		return comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.ObservedCompositionGeneration >= deleteGen
+	})
+
+	// The composition should still exist after a bit
+	time.Sleep(time.Millisecond * 100)
+	require.NoError(t, cli.Get(ctx, client.ObjectKeyFromObject(comp), comp))
+
+	// Mark the synthesized time as nil to simulate the synthesizer is missing
+	err := retry.RetryOnConflict(testutil.Backoff, func() error {
+		cli.Get(ctx, client.ObjectKeyFromObject(comp), comp)
+		comp.Status.CurrentSynthesis.Synthesized = nil
+		return cli.Status().Update(ctx, comp)
+	})
+	require.NoError(t, err)
+
+	// The composition should eventually be released even the composition is waiting for reconciliation to be completed
+	testutil.Eventually(t, func() bool {
+		return errors.IsNotFound(cli.Get(ctx, client.ObjectKeyFromObject(comp), comp))
+	})
+}
+
 func TestNonExistentComposition(t *testing.T) {
 	ctx := testutil.NewContext(t)
 	mgr := testutil.NewManager(t)