From 28ff3c9d28277556e2d57007f6cfc53a21eee3f9 Mon Sep 17 00:00:00 2001 From: Sergiy Kulanov Date: Thu, 27 Aug 2026 16:33:00 +0300 Subject: [PATCH] EPMDEDP-17289: fix: Count terminating CDPipelines as usage in the deletion webhooks A CDPipeline blocks Codebase and CodebaseBranch deletion for as long as the object exists, terminating included. The previous DeletionTimestamp filter opened a window during pipeline teardown: the Codebase delete was admitted, the owner-reference cascade removed the CodebaseImageStreams, and the remaining Stage finalizers, which read those streams to strip environment labels serially last-to-first, wedged the Stage and the CDPipeline in Terminating forever. Denials now name the terminating blocker ("being deleted") and advise waiting for the teardown to finish instead of removing the reference. Terminating Stages stay out of autotest-gate usage: stage teardown never reads the autotest codebase. Signed-off-by: Sergiy Kulanov --- pkg/codebase/usage.go | 16 ++++++---- pkg/codebase/usage_test.go | 6 ++-- pkg/codebasebranch/usage.go | 15 ++++++---- pkg/codebasebranch/usage_test.go | 6 ++-- pkg/deploymentusage/deploymentusage.go | 33 ++++++++++----------- pkg/webhook/codebase_webhook_delete_test.go | 4 ++- pkg/webhook/usage_error.go | 21 +++++++++++-- 7 files changed, 65 insertions(+), 36 deletions(-) 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{