diff --git a/pkg/codebase/usage.go b/pkg/codebase/usage.go index 4d408ec9..17708496 100644 --- a/pkg/codebase/usage.go +++ b/pkg/codebase/usage.go @@ -23,7 +23,10 @@ import ( // - Stage.Spec.QualityGates references a Codebase name via AutotestName // (autotest components). // -// Resources that are being deleted are not counted as usage. +// A CDPipeline counts as usage while it exists, terminating included: its Stage +// finalizers read the codebase's CodebaseImageStreams until it is gone. +// Terminating Stages do not count for autotest gates: stage teardown never +// reads the autotest codebase. func FindCodebaseUsage( ctx context.Context, c client.Client, @@ -31,7 +34,7 @@ func FindCodebaseUsage( ) ([]deploymentusage.Reference, error) { var refs []deploymentusage.Reference - pipelines, err := deploymentusage.ListActiveCDPipelines(ctx, c, codebase.Namespace) + pipelines, err := deploymentusage.ListCDPipelines(ctx, c, codebase.Namespace) if err != nil { return nil, err } @@ -50,10 +53,11 @@ func FindCodebaseUsage( } refs = append(refs, deploymentusage.Reference{ - Kind: deploymentusage.KindCDPipeline, - Name: pipeline.Name, - Field: field, - Reason: reason, + Kind: deploymentusage.KindCDPipeline, + Name: pipeline.Name, + Field: field, + Reason: reason, + Deleting: pipeline.DeletionTimestamp != nil, }) } diff --git a/pkg/codebase/usage_test.go b/pkg/codebase/usage_test.go index 0fa28920..18f52458 100644 --- a/pkg/codebase/usage_test.go +++ b/pkg/codebase/usage_test.go @@ -267,7 +267,8 @@ func TestFindCodebaseUsage_MultipleReferences(t *testing.T) { assert.Contains(t, descriptions, "Stage demo-dev of CDPipeline demo (autotest quality gate)") } -func TestFindCodebaseUsage_TerminatingPipelineIgnored(t *testing.T) { +// A terminating pipeline still counts as usage. +func TestFindCodebaseUsage_TerminatingPipelineStillCounts(t *testing.T) { pipeline := &pipelineApi.CDPipeline{ ObjectMeta: metav1.ObjectMeta{ Name: "demo", @@ -284,7 +285,8 @@ func TestFindCodebaseUsage_TerminatingPipelineIgnored(t *testing.T) { refs, err := FindCodebaseUsage(context.Background(), k8sClient, usageCodebase()) require.NoError(t, err) - assert.Empty(t, refs) + require.Len(t, refs, 1) + assert.Equal(t, "CDPipeline demo (applications, being deleted)", refs[0].String()) } func TestFindCodebaseUsage_Unused(t *testing.T) { diff --git a/pkg/codebasebranch/usage.go b/pkg/codebasebranch/usage.go index 49a4b49c..b6a505d3 100644 --- a/pkg/codebasebranch/usage.go +++ b/pkg/codebasebranch/usage.go @@ -30,7 +30,9 @@ type BranchUsageIndex struct { } // NewBranchUsageIndex reads the deployment resources of the namespace once and indexes -// the references they hold. Resources that are being deleted are not counted as usage. +// the references they hold. A CDPipeline counts while it exists, terminating +// included: its Stage finalizers read the CodebaseImageStreams until it is gone. +// Terminating Stages do not count for autotest gates. // // The snapshot is never refreshed, so callers that must not act on stale data build one // per request. @@ -40,7 +42,7 @@ func NewBranchUsageIndex(ctx context.Context, c client.Client, namespace string) byAutotest: make(map[autotestGate][]deploymentusage.Reference), } - pipelines, err := deploymentusage.ListActiveCDPipelines(ctx, c, namespace) + pipelines, err := deploymentusage.ListCDPipelines(ctx, c, namespace) if err != nil { return nil, err } @@ -74,10 +76,11 @@ func (i *BranchUsageIndex) addPipeline(pipeline *pipelineApi.CDPipeline) { seen[stream] = struct{}{} i.byStreamName[stream] = append(i.byStreamName[stream], deploymentusage.Reference{ - Kind: deploymentusage.KindCDPipeline, - Name: pipeline.Name, - Field: deploymentusage.FieldInputDockerStreams, - Reason: "inputDockerStreams", + Kind: deploymentusage.KindCDPipeline, + Name: pipeline.Name, + Field: deploymentusage.FieldInputDockerStreams, + Reason: "inputDockerStreams", + Deleting: pipeline.DeletionTimestamp != nil, }) } } diff --git a/pkg/codebasebranch/usage_test.go b/pkg/codebasebranch/usage_test.go index 4b7acd99..06aca2f0 100644 --- a/pkg/codebasebranch/usage_test.go +++ b/pkg/codebasebranch/usage_test.go @@ -129,7 +129,8 @@ func TestFindBranchUsage_MultipleReferences(t *testing.T) { assert.Contains(t, descriptions, "Stage demo-dev of CDPipeline demo (autotest quality gate)") } -func TestFindBranchUsage_TerminatingPipelineIgnored(t *testing.T) { +// A terminating pipeline still counts as usage. +func TestFindBranchUsage_TerminatingPipelineStillCounts(t *testing.T) { pipeline := &pipelineApi.CDPipeline{ ObjectMeta: metav1.ObjectMeta{ Name: "demo", @@ -146,7 +147,8 @@ func TestFindBranchUsage_TerminatingPipelineIgnored(t *testing.T) { refs, err := FindBranchUsage(context.Background(), k8sClient, usageBranch()) require.NoError(t, err) - assert.Empty(t, refs) + require.Len(t, refs, 1) + assert.Equal(t, "CDPipeline demo (inputDockerStreams, being deleted)", refs[0].String()) } func TestFindBranchUsage_Unused(t *testing.T) { diff --git a/pkg/deploymentusage/deploymentusage.go b/pkg/deploymentusage/deploymentusage.go index 02e4ebec..8945b697 100644 --- a/pkg/deploymentusage/deploymentusage.go +++ b/pkg/deploymentusage/deploymentusage.go @@ -46,15 +46,23 @@ type Reference struct { Reason string // ParentCDPipeline is set only when Kind is KindStage. ParentCDPipeline string + // Deleting is set when the referencing resource is terminating. It still + // blocks deletion; the denial advice switches from "remove" to "wait". + Deleting bool } // String renders the reference as it appears in the denial message shown to users. func (r Reference) String() string { + reason := r.Reason + if r.Deleting { + reason += ", being deleted" + } + if r.Kind == KindStage { - return fmt.Sprintf("%s %s of %s %s (%s)", KindStage, r.Name, KindCDPipeline, r.ParentCDPipeline, r.Reason) + return fmt.Sprintf("%s %s of %s %s (%s)", KindStage, r.Name, KindCDPipeline, r.ParentCDPipeline, reason) } - return fmt.Sprintf("%s %s (%s)", r.Kind, r.Name, r.Reason) + return fmt.Sprintf("%s %s (%s)", r.Kind, r.Name, reason) } // Join renders references as a single description, in the form used by both the @@ -69,10 +77,11 @@ func Join(refs []Reference) string { return strings.Join(descriptions, "; ") } -// ListActiveCDPipelines returns the CDPipelines in the given namespace that -// are not being deleted. When the CD pipeline CRDs are not installed in the -// cluster, it returns an empty, non-error result. -func ListActiveCDPipelines(ctx context.Context, c client.Client, namespace string) ([]pipelineApi.CDPipeline, error) { +// ListCDPipelines returns every CDPipeline in the given namespace, terminating +// included: Stage finalizers read the CodebaseImageStreams until the pipeline +// is gone. When the CD pipeline CRDs are not installed in the cluster, it +// returns an empty, non-error result. +func ListCDPipelines(ctx context.Context, c client.Client, namespace string) ([]pipelineApi.CDPipeline, error) { pipelines := &pipelineApi.CDPipelineList{} if err := c.List(ctx, pipelines, client.InNamespace(namespace)); err != nil { if IsKindUnavailable(err) { @@ -82,17 +91,7 @@ func ListActiveCDPipelines(ctx context.Context, c client.Client, namespace strin return nil, fmt.Errorf("failed to list CDPipelines: %w", err) } - active := make([]pipelineApi.CDPipeline, 0, len(pipelines.Items)) - - for i := range pipelines.Items { - if pipelines.Items[i].DeletionTimestamp != nil { - continue - } - - active = append(active, pipelines.Items[i]) - } - - return active, nil + return pipelines.Items, nil } // AutotestGate is a Stage quality gate that runs an autotest, together with the Reference diff --git a/pkg/webhook/codebase_webhook_delete_test.go b/pkg/webhook/codebase_webhook_delete_test.go index 38aa9cbc..dc880e65 100644 --- a/pkg/webhook/codebase_webhook_delete_test.go +++ b/pkg/webhook/codebase_webhook_delete_test.go @@ -113,7 +113,8 @@ func TestCodebaseValidationWebhook_ValidateDelete_CodebaseInUse(t *testing.T) { }, }, { - name: "allows deletion when referencing CDPipeline is terminating", + // A terminating pipeline still blocks deletion; the advice switches to "wait". + name: "rejects deletion while referencing CDPipeline is terminating", objects: []runtime.Object{ &pipelineApi.CDPipeline{ ObjectMeta: metav1.ObjectMeta{ @@ -125,6 +126,7 @@ func TestCodebaseValidationWebhook_ValidateDelete_CodebaseInUse(t *testing.T) { Spec: pipelineApi.CDPipelineSpec{Applications: []string{"app"}}, }, }, + wantErr: "used by CDPipeline demo (applications, being deleted); wait for the deletion to finish", }, { name: "allows deletion when CD pipeline CRDs are not installed", diff --git a/pkg/webhook/usage_error.go b/pkg/webhook/usage_error.go index 11bc0e85..dd3503a6 100644 --- a/pkg/webhook/usage_error.go +++ b/pkg/webhook/usage_error.go @@ -16,6 +16,16 @@ const ( codebaseBranchKind = "CodebaseBranch" ) +func allDeleting(refs []deploymentusage.Reference) bool { + for _, ref := range refs { + if !ref.Deleting { + return false + } + } + + return len(refs) > 0 +} + // newBlockedByUsageError builds a StatusError that denies deletion of a // resource still referenced by deployment resources. // @@ -43,9 +53,16 @@ func newBlockedByUsageError( }) } + // All blockers terminating: nothing left to remove; advise waiting. + advice := "remove it from the deployment first" + + if allDeleting(refs) { + advice = "wait for the deletion to finish" + } + message := fmt.Sprintf( - "%s %s cannot be deleted because it is used by %s; remove it from the deployment first", - kind, name, deploymentusage.Join(refs), + "%s %s cannot be deleted because it is used by %s; %s", + kind, name, deploymentusage.Join(refs), advice, ) return &apierrors.StatusError{