diff --git a/api/v1/config/crd/eno.azure.io_resourceslices.yaml b/api/v1/config/crd/eno.azure.io_resourceslices.yaml index 8527772b..4a28acff 100644 --- 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 index e8582152..14bdf4a9 100644 --- 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 index 5aa1d156..c74948e5 100644 --- 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,18 +186,74 @@ 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{} @@ -200,11 +261,8 @@ func buildResourceSlices(comp *apiv1.Composition, outputs []*unstructured.Unstru 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 index 006534c0..57e9abd7 100644 --- 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"