Create tombstones

Azure · Dec 29, 2023 · 1676f90 · 1676f90
1 parent 0e08bc8
commit 1676f90
Show file tree

Hide file tree

Showing 4 changed files with 181 additions and 13 deletions.
diff --git a/api/v1/config/crd/eno.azure.io_resourceslices.yaml b/api/v1/config/crd/eno.azure.io_resourceslices.yaml
@@ -35,6 +35,10 @@ spec:
               resources:
                 items:
                   properties:
+                    deleted:
+                      description: Deleted is true when this manifest represents a
+                        "tombstone" - a resource that should no longer exist.
+                      type: boolean
                     manifest:
                       type: string
                     reconcileInterval:

diff --git a/api/v1/resourceslice.go b/api/v1/resourceslice.go
@@ -28,6 +28,9 @@ type Manifest struct {
 	// +required
 	Manifest string `json:"manifest,omitempty"`
 
+	// Deleted is true when this manifest represents a "tombstone" - a resource that should no longer exist.
+	Deleted bool `json:"deleted,omitempty"`
+
 	ReconcileInterval *metav1.Duration `json:"reconcileInterval,omitempty"`
 }
 

diff --git a/internal/controllers/synthesis/exec.go b/internal/controllers/synthesis/exec.go
@@ -160,7 +160,12 @@ func (c *execController) writeOutputToSlices(ctx context.Context, comp *apiv1.Co
 		return nil, reconcile.TerminalError(fmt.Errorf("parsing outputs: %w", err))
 	}
 
-	slices, err := buildResourceSlices(comp, outputs, maxSliceJsonBytes)
+	previous, err := c.fetchPreviousSlices(ctx, comp)
+	if err != nil {
+		return nil, err
+	}
+
+	slices, err := buildResourceSlices(comp, previous, outputs, maxSliceJsonBytes)
 	if err != nil {
 		return nil, err
 	}
@@ -181,30 +186,83 @@ func (c *execController) writeOutputToSlices(ctx context.Context, comp *apiv1.Co
 	return sliceRefs, nil
 }
 
-func buildResourceSlices(comp *apiv1.Composition, outputs []*unstructured.Unstructured, maxJsonBytes int) ([]*apiv1.ResourceSlice, error) {
-	var (
-		slices     []*apiv1.ResourceSlice
-		sliceBytes int
-		slice      *apiv1.ResourceSlice
-	)
+func (c *execController) fetchPreviousSlices(ctx context.Context, comp *apiv1.Composition) ([]*apiv1.ResourceSlice, error) {
+	if comp.Status.CurrentState == nil {
+		return nil, nil // nothing to fetch
+	}
+	logger := logr.FromContextOrDiscard(ctx)
+
+	slices := []*apiv1.ResourceSlice{}
+	for _, ref := range comp.Status.CurrentState.ResourceSlices {
+		slice := &apiv1.ResourceSlice{}
+		slice.Name = ref.Name
+		slice.Namespace = comp.Namespace
+		err := c.client.Get(ctx, client.ObjectKeyFromObject(slice), slice)
+		if errors.IsNotFound(err) {
+			logger.Info("resource slice referenced by composition was not found - skipping", "resourceSliceName", slice.Name)
+			continue
+		}
+		if err != nil {
+			return nil, fmt.Errorf("fetching current resource slice %q: %w", slice.Name, err)
+		}
+		slices = append(slices, slice)
+	}
 
+	return slices, nil
+}
+
+func buildResourceSlices(comp *apiv1.Composition, previous []*apiv1.ResourceSlice, outputs []*unstructured.Unstructured, maxJsonBytes int) ([]*apiv1.ResourceSlice, error) {
+	// Encode the given resources into manifest structs
+	refs := map[resourceRef]struct{}{}
+	manifests := []apiv1.Manifest{}
 	for i, output := range outputs {
 		js, err := output.MarshalJSON()
 		if err != nil {
 			return nil, reconcile.TerminalError(fmt.Errorf("encoding output %d: %w", i, err))
 		}
+		manifests = append(manifests, apiv1.Manifest{
+			Manifest:          string(js),
+			ReconcileInterval: consumeReconcileIntervalAnnotation(output),
+		})
+		refs[newResourceRef(output)] = struct{}{}
+	}
+
+	// Build tombstones by diffing the new state against the current state
+	// Existing tombstones are passed down if they haven't yet been reconciled to avoid orphaning resources
+	for _, slice := range previous {
+		for i, res := range slice.Spec.Resources {
+			res := res
+			obj := &unstructured.Unstructured{}
+			err := obj.UnmarshalJSON([]byte(res.Manifest))
+			if err != nil {
+				return nil, reconcile.TerminalError(fmt.Errorf("decoding resource %d of slice %s: %w", i, slice.Name, err))
+			}
+
+			if _, ok := refs[newResourceRef(obj)]; ok || (res.Deleted && slice.Status.Resources != nil && slice.Status.Resources[i].Reconciled) {
+				continue // still exists or has already been deleted
+			}
+
+			res.Deleted = true
+			manifests = append(manifests, res)
+		}
+	}
+
+	// Build the slice resources
+	var (
+		slices     []*apiv1.ResourceSlice
+		sliceBytes int
+		slice      *apiv1.ResourceSlice
+	)
+	for _, manifest := range manifests {
 		if slice == nil || sliceBytes >= maxJsonBytes {
 			sliceBytes = 0
 			slice = &apiv1.ResourceSlice{}
 			slice.GenerateName = comp.Name + "-"
 			slice.Namespace = comp.Namespace
 			slices = append(slices, slice)
 		}
-		sliceBytes += len(js)
-		slice.Spec.Resources = append(slice.Spec.Resources, apiv1.Manifest{
-			Manifest:          string(js),
-			ReconcileInterval: consumeReconcileIntervalAnnotation(output),
-		})
+		sliceBytes += len(manifest.Manifest)
+		slice.Spec.Resources = append(slice.Spec.Resources, manifest)
 	}
 
 	return slices, nil
@@ -297,3 +355,16 @@ func consumeReconcileIntervalAnnotation(obj client.Object) *metav1.Duration {
 	}
 	return &metav1.Duration{Duration: dur}
 }
+
+type resourceRef struct {
+	Name, Namespace, Kind, Group string
+}
+
+func newResourceRef(obj *unstructured.Unstructured) resourceRef {
+	return resourceRef{
+		Name:      obj.GetName(),
+		Namespace: obj.GetNamespace(),
+		Kind:      obj.GetKind(),
+		Group:     obj.GroupVersionKind().Group,
+	}
+}
diff --git a/internal/controllers/synthesis/exec_test.go b/internal/controllers/synthesis/exec_test.go
@@ -18,11 +18,101 @@ func TestBuildResourceSlicesOverflow(t *testing.T) {
 		outputs = append(outputs, &unstructured.Unstructured{})
 	}
 
-	slices, err := buildResourceSlices(&apiv1.Composition{}, outputs, 20)
+	slices, err := buildResourceSlices(&apiv1.Composition{}, []*apiv1.ResourceSlice{}, outputs, 20)
 	require.NoError(t, err)
 	assert.Len(t, slices, 4)
 }
 
+func TestBuildResourceSlicesTombstonesBasics(t *testing.T) {
+	outputs := []*unstructured.Unstructured{{
+		Object: map[string]interface{}{
+			"kind":       "Test",
+			"apiVersion": "mygroup/v1",
+			"metadata": map[string]interface{}{
+				"name":      "test-resource",
+				"namespace": "test-ns",
+			},
+		},
+	}}
+
+	slices, err := buildResourceSlices(&apiv1.Composition{}, []*apiv1.ResourceSlice{}, outputs, 100000)
+	require.NoError(t, err)
+	require.Len(t, slices, 1)
+	require.Len(t, slices[0].Spec.Resources, 1)
+	assert.False(t, slices[0].Spec.Resources[0].Deleted)
+
+	// Remove the resource - initial tombstone record is created
+	slices, err = buildResourceSlices(&apiv1.Composition{}, slices, []*unstructured.Unstructured{}, 100000)
+	require.NoError(t, err)
+	require.Len(t, slices, 1)
+	require.Len(t, slices[0].Spec.Resources, 1)
+	assert.True(t, slices[0].Spec.Resources[0].Deleted)
+
+	// The actual resource hasn't been reconciled (deleted) yet, so the tombstone will persist in new states
+	slices, err = buildResourceSlices(&apiv1.Composition{}, slices, []*unstructured.Unstructured{}, 100000)
+	require.NoError(t, err)
+	require.Len(t, slices, 1)
+	require.Len(t, slices[0].Spec.Resources, 1)
+	assert.True(t, slices[0].Spec.Resources[0].Deleted)
+
+	// The tombstone is removed once it has been reconciled
+	slices[0].Status.Resources = []apiv1.ResourceState{{Reconciled: true}}
+	slices, err = buildResourceSlices(&apiv1.Composition{}, slices, []*unstructured.Unstructured{}, 100000)
+	require.NoError(t, err)
+	require.Len(t, slices, 0)
+}
+
+func TestBuildResourceSlicesTombstonesVersionSemantics(t *testing.T) {
+	outputs := []*unstructured.Unstructured{{
+		Object: map[string]interface{}{
+			"kind":       "Test",
+			"apiVersion": "mygroup/v1",
+			"metadata": map[string]interface{}{
+				"name":      "test-resource",
+				"namespace": "test-ns",
+			},
+		},
+	}}
+	slices, err := buildResourceSlices(&apiv1.Composition{}, []*apiv1.ResourceSlice{}, outputs, 100000)
+	require.NoError(t, err)
+	require.Len(t, slices, 1)
+	require.Len(t, slices[0].Spec.Resources, 1)
+	assert.False(t, slices[0].Spec.Resources[0].Deleted)
+
+	// Upgrade to v2 - tombstone should not be created
+	outputs = []*unstructured.Unstructured{{
+		Object: map[string]interface{}{
+			"kind":       "Test",
+			"apiVersion": "mygroup/v2",
+			"metadata": map[string]interface{}{
+				"name":      "test-resource",
+				"namespace": "test-ns",
+			},
+		},
+	}}
+	slices, err = buildResourceSlices(&apiv1.Composition{}, slices, outputs, 100000)
+	require.NoError(t, err)
+	require.Len(t, slices, 1)
+	require.Len(t, slices[0].Spec.Resources, 1)
+	assert.False(t, slices[0].Spec.Resources[0].Deleted)
+
+	// Change group name - tombstone should be created
+	outputs = []*unstructured.Unstructured{{
+		Object: map[string]interface{}{
+			"kind":       "Test",
+			"apiVersion": "mygroup2/v2",
+			"metadata": map[string]interface{}{
+				"name":      "test-resource",
+				"namespace": "test-ns",
+			},
+		},
+	}}
+	slices, err = buildResourceSlices(&apiv1.Composition{}, slices, outputs, 100000)
+	require.NoError(t, err)
+	require.Len(t, slices, 1)
+	require.Len(t, slices[0].Spec.Resources, 2)
+}
+
 func TestBuildInputsJson(t *testing.T) {
 	cm := &corev1.ConfigMap{}
 	cm.Name = "test-cm"