Add Deleted status property

api/v1/config/crd/eno.azure.io_resourceslices.yaml

@@ -56,6 +56,11 @@ spec:
                   spec.resources at the observed generation.
                 items:
                   properties:
+                    deleted:
+                      description: Deleted is true when the resource has been cleaned
+                        up, either because spec.deleted == true or the parent composition
+                        has been deleted.
+                      type: boolean
                     ready:
                       description: nil if Eno is unable to determine the readiness
                         of this resource. Otherwise it is true when the resource is

api/v1/resourceslice.go

@@ -48,6 +48,9 @@ type ResourceState struct {
 	// Otherwise it is true when the resource is ready, false otherwise.
 	// Like Reconciled, it latches and will never transition from true->false.
 	Ready *bool `json:"ready,omitempty"`
+
+	// Deleted is true when the resource has been cleaned up, either because spec.deleted == true or the parent composition has been deleted.
+	Deleted bool `json:"deleted,omitempty"`
 }
 
 type ResourceSliceRef struct {

internal/controllers/reconciliation/controller.go

@@ -111,12 +111,17 @@ func (c *Controller) Reconcile(ctx context.Context, req *reconstitution.Request)
 		return ctrl.Result{}, err
 	}
 
-	c.resourceClient.PatchStatusAsync(ctx, &req.Manifest, func(rs *apiv1.ResourceState) bool {
-		if rs.Reconciled {
-			return false // already in sync
+	c.resourceClient.PatchStatusAsync(ctx, &req.Manifest, func(rs *apiv1.ResourceState) (modified bool) {
+		deleted := current.GetDeletionTimestamp() != nil || resource.Manifest.Deleted
+		if deleted && !rs.Deleted {
+			rs.Deleted = true
+			modified = true
 		}
-		rs.Reconciled = true
-		return true
+		if !rs.Reconciled {
+			rs.Reconciled = true
+			modified = true
+		}
+		return
 	})
 
 	if resource != nil && resource.Manifest.ReconcileInterval != nil {
@@ -133,6 +138,8 @@ func (c *Controller) reconcileResource(ctx context.Context, prev, resource *reco
 			return nil // already deleted - nothing to do
 		}
 
+		// TODO: How to get resource slice deletion state here?
+
 		err := c.upstreamClient.Delete(ctx, resource.Object)
 		if err != nil {
 			return client.IgnoreNotFound(fmt.Errorf("deleting resource: %w", err))

internal/controllers/reconciliation/integration_test.go

@@ -15,6 +15,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	apiv1 "github.com/Azure/eno/api/v1"
 	testv1 "github.com/Azure/eno/internal/controllers/reconciliation/fixtures/v1"
@@ -349,7 +350,7 @@ func TestReconcileInterval(t *testing.T) {
 	comp.Spec.Synthesizer.Name = syn.Name
 	require.NoError(t, upstream.Create(ctx, comp))
 
-	// Wait for service to be created
+	// Wait for resource to be created
 	obj := &corev1.ConfigMap{}
 	testutil.Eventually(t, func() bool {
 		obj.SetName("test-obj")
@@ -446,3 +447,50 @@ func TestReconcileCacheRace(t *testing.T) {
 		time.Sleep(time.Millisecond * 50)
 	}
 }
+
+func TestReconcileStatus(t *testing.T) {
+	scheme := runtime.NewScheme()
+	corev1.SchemeBuilder.AddToScheme(scheme)
+	testv1.SchemeBuilder.AddToScheme(scheme)
+
+	ctx := testutil.NewContext(t)
+	mgr := testutil.NewManager(t)
+	upstream := mgr.GetClient()
+	downstream := mgr.DownstreamClient
+
+	rm, err := reconstitution.New(mgr.Manager, time.Millisecond)
+	require.NoError(t, err)
+
+	err = New(rm, mgr.DownstreamRestConfig, 5, testutil.AtLeastVersion(t, 15))
+	require.NoError(t, err)
+	mgr.Start(t)
+
+	comp := &apiv1.Composition{}
+	comp.Name = "test-comp"
+	comp.Namespace = "default"
+	require.NoError(t, upstream.Create(ctx, comp))
+
+	slice := &apiv1.ResourceSlice{}
+	slice.Name = "test-slice"
+	slice.Namespace = comp.Namespace
+	require.NoError(t, controllerutil.SetControllerReference(comp, slice, upstream.Scheme()))
+	slice.Spec.Resources = []apiv1.Manifest{
+		{Manifest: `{ "kind": "ConfigMap", "apiVersion": "v1", "metadata": { "name": "test", "namespace": "default" } }`},
+		{Deleted: true, Manifest: `{ "kind": "ConfigMap", "apiVersion": "v1", "metadata": { "name": "test-deleted", "namespace": "default" } }`},
+	}
+	require.NoError(t, upstream.Create(ctx, slice))
+
+	comp.Status.CurrentState = &apiv1.Synthesis{
+		Synthesized:    true,
+		ResourceSlices: []*apiv1.ResourceSliceRef{{Name: slice.Name}},
+	}
+	require.NoError(t, upstream.Status().Update(ctx, comp))
+
+	// Status should eventually reflect the reconciliation state
+	testutil.Eventually(t, func() bool {
+		err = downstream.Get(context.Background(), client.ObjectKeyFromObject(slice), slice)
+		return err == nil && len(slice.Status.Resources) == 2 &&
+			slice.Status.Resources[0].Reconciled && !slice.Status.Resources[0].Deleted &&
+			slice.Status.Resources[1].Reconciled && slice.Status.Resources[1].Deleted
+	})
+}

internal/controllers/resourceslice/cleanup.go

@@ -48,8 +48,7 @@ func (c *cleanupController) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	comp.Name = owner.Name
 	comp.Namespace = slice.Namespace
 	err = c.client.Get(ctx, client.ObjectKeyFromObject(comp), comp)
-	// TODO: This actually shouldn't use tombstones
-	if slice.DeletionTimestamp != nil && (errors.IsNotFound(err) || comp.DeletionTimestamp != nil) && tombstonesRemain(slice) {
+	if slice.DeletionTimestamp != nil && (errors.IsNotFound(err) || comp.DeletionTimestamp != nil) && resourcesRemain(slice) {
 		return ctrl.Result{}, nil // wait for tombstones to be reconciled to avoid leaking resources
 	}
 	if errors.IsNotFound(err) {
@@ -102,17 +101,12 @@ func (c *cleanupController) ensureFinalizerRemoved(ctx context.Context, slice *a
 	return ctrl.Result{}, nil
 }
 
-func tombstonesRemain(slice *apiv1.ResourceSlice) bool {
-	for i, manifest := range slice.Spec.Resources {
-		if !manifest.Deleted {
-			continue
-		}
-		if slice.Status.Resources == nil {
-			return true
-		}
-
-		state := slice.Status.Resources[i]
-		if !state.Reconciled {
+func resourcesRemain(slice *apiv1.ResourceSlice) bool {
+	if len(slice.Status.Resources) == 0 && len(slice.Spec.Resources) > 0 {
+		return true // status is lagging behind
+	}
+	for _, state := range slice.Status.Resources {
+		if !state.Deleted {
 			return true
 		}
 	}

internal/controllers/resourceslice/cleanup_test.go

@@ -74,7 +74,7 @@ func TestSliceControllerCompositionCleanup(t *testing.T) {
 	slice.Name = "test-1"
 	slice.Namespace = "default"
 	slice.Finalizers = []string{"eno.azure.io/cleanup"}
-	slice.Spec.Resources = []apiv1.Manifest{{Manifest: "test-tombstone", Deleted: true}}
+	slice.Spec.Resources = []apiv1.Manifest{{Manifest: "test-resource"}}
 	require.NoError(t, controllerutil.SetControllerReference(comp, slice, mgr.GetScheme()))
 	require.NoError(t, mgr.GetClient().Create(ctx, slice))
 
@@ -92,8 +92,8 @@ func TestSliceControllerCompositionCleanup(t *testing.T) {
 		return mgr.GetClient().Get(ctx, client.ObjectKeyFromObject(slice), slice) == nil
 	})
 
-	// Reconcile the tombstone
-	slice.Status.Resources = []apiv1.ResourceState{{Reconciled: true}}
+	// Remove the resource
+	slice.Status.Resources = []apiv1.ResourceState{{Deleted: true}}
 	require.NoError(t, mgr.GetClient().Status().Update(ctx, slice))
 
 	// Slice should eventually be released