diff --git a/internal/controllers/synthesis/slicecleanup.go b/internal/controllers/synthesis/slicecleanup.go index ff3d1a98..562105e5 100644 --- a/internal/controllers/synthesis/slicecleanup.go +++ b/internal/controllers/synthesis/slicecleanup.go @@ -96,11 +96,20 @@ func shouldDeleteSlice(comp *apiv1.Composition, slice *apiv1.ResourceSlice) bool if comp.Status.CurrentSynthesis == nil || slice.Spec.CompositionGeneration > comp.Status.CurrentSynthesis.ObservedCompositionGeneration { return false // stale informer } - isOutdated := slice.Spec.Attempt != 0 && comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Attempts > slice.Spec.Attempt - isReferencedByComp := synthesisReferencesSlice(comp.Status.CurrentSynthesis, slice) || synthesisReferencesSlice(comp.Status.PreviousSynthesis, slice) - isSynthesized := comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Synthesized != nil - compIsDeleted := comp.DeletionTimestamp != nil - return isOutdated || (isSynthesized && compIsDeleted || (!isReferencedByComp && slice.Spec.CompositionGeneration < comp.Generation)) + + var ( + hasBeenRetried = slice.Spec.Attempt != 0 && comp.Status.CurrentSynthesis.Attempts > slice.Spec.Attempt && slice.Spec.SynthesisUUID == comp.Status.CurrentSynthesis.UUID + isReferencedByComp = synthesisReferencesSlice(comp.Status.CurrentSynthesis, slice) || synthesisReferencesSlice(comp.Status.PreviousSynthesis, slice) + isSynthesized = comp.Status.CurrentSynthesis.Synthesized != nil + compIsDeleted = comp.DeletionTimestamp != nil + fromOldComposition = slice.Spec.CompositionGeneration < comp.Status.CurrentSynthesis.ObservedCompositionGeneration + ) + + // We can only safely delete resource slices when either: + // - Another retry of the same synthesis has already started + // - Synthesis is complete and the composition is being deleted + // - The slice was derived from an older composition + return hasBeenRetried || (isSynthesized && compIsDeleted) || (!isReferencedByComp && fromOldComposition) } func shouldReleaseSliceFinalizer(comp *apiv1.Composition, slice *apiv1.ResourceSlice) bool { diff --git a/internal/controllers/synthesis/slicecleanup_test.go b/internal/controllers/synthesis/slicecleanup_test.go index 9c9e4db9..f8906111 100644 --- a/internal/controllers/synthesis/slicecleanup_test.go +++ b/internal/controllers/synthesis/slicecleanup_test.go @@ -95,7 +95,30 @@ func TestShouldDeleteSlice(t *testing.T) { expected: false, }, { - name: "slice is outdated", + name: "another attempt started for a different synthesis, old one still references the slice", + comp: &apiv1.Composition{ + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + Attempts: 5, + UUID: "the-next-one", + }, + PreviousSynthesis: &apiv1.Synthesis{ + ResourceSlices: []*apiv1.ResourceSliceRef{{Name: "test-slice"}}, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-slice", + }, + Spec: apiv1.ResourceSliceSpec{ + Attempt: 3, + }, + }, + expected: false, + }, + { + name: "another attempt started for the same synthesis", comp: &apiv1.Composition{ Status: apiv1.CompositionStatus{ CurrentSynthesis: &apiv1.Synthesis{ @@ -139,6 +162,162 @@ func TestShouldDeleteSlice(t *testing.T) { slice: &apiv1.ResourceSlice{}, expected: true, }, + { + name: "synthesis terminated and slice referenced", + comp: &apiv1.Composition{ + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + Synthesized: &metav1.Time{Time: time.Now()}, + ResourceSlices: []*apiv1.ResourceSliceRef{{Name: "test-slice"}}, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + }, + expected: false, + }, + { + name: "synthesis terminated, different composition generation, same synthesis", + comp: &apiv1.Composition{ + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + Synthesized: &metav1.Time{Time: time.Now()}, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: apiv1.ResourceSliceSpec{ + CompositionGeneration: 2, + }, + }, + expected: false, + }, + { + name: "synthesis terminated, same composition generation, different synthesis", + comp: &apiv1.Composition{ + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + Synthesized: &metav1.Time{Time: time.Now()}, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: apiv1.ResourceSliceSpec{ + SynthesisUUID: "foo", + }, + }, + expected: false, + }, + { + name: "synthesis terminated, newer composition generation, different synthesis", + comp: &apiv1.Composition{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 3, + }, + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + Synthesized: &metav1.Time{Time: time.Now()}, + ObservedCompositionGeneration: 2, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: apiv1.ResourceSliceSpec{ + SynthesisUUID: "foo", + CompositionGeneration: 1, + }, + }, + expected: true, + }, + { + name: "synthesis in-progress, newer composition generation, different synthesis", + comp: &apiv1.Composition{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 3, + }, + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + ObservedCompositionGeneration: 2, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: apiv1.ResourceSliceSpec{ + SynthesisUUID: "foo", + CompositionGeneration: 1, + }, + }, + expected: true, + }, + { + name: "synthesis in-progress, newer composition generation, same synthesis", + comp: &apiv1.Composition{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + }, + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + ObservedCompositionGeneration: 1, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: apiv1.ResourceSliceSpec{ + CompositionGeneration: 1, + }, + }, + expected: false, + }, + { + name: "synthesis terminated, newer composition and synthesis generation, different synthesis", + comp: &apiv1.Composition{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + }, + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + Synthesized: &metav1.Time{Time: time.Now()}, + ObservedCompositionGeneration: 2, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: apiv1.ResourceSliceSpec{ + SynthesisUUID: "foo", + CompositionGeneration: 1, + }, + }, + expected: true, + }, + { + name: "synthesis terminated, older composition generation, different synthesis", + comp: &apiv1.Composition{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + Status: apiv1.CompositionStatus{ + CurrentSynthesis: &apiv1.Synthesis{ + Synthesized: &metav1.Time{Time: time.Now()}, + ObservedCompositionGeneration: 1, + }, + }, + }, + slice: &apiv1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: apiv1.ResourceSliceSpec{ + SynthesisUUID: "foo", + CompositionGeneration: 2, + }, + }, + expected: false, + }, { name: "composition is deleted but synthesis not terminated", comp: &apiv1.Composition{