From d664de52a070a6cee0876b247986a40507c089d9 Mon Sep 17 00:00:00 2001 From: Zichen Yu <1062955096@qq.com> Date: Wed, 16 Sep 2026 11:47:00 +0800 Subject: [PATCH 1/2] fix: preserve PipelineRun-owned resources during history pruning --- pkg/config/history_limiter.go | 18 +++++++- pkg/config/history_limiter_test.go | 68 ++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/pkg/config/history_limiter.go b/pkg/config/history_limiter.go index 808f00d1..2b99eabf 100644 --- a/pkg/config/history_limiter.go +++ b/pkg/config/history_limiter.go @@ -348,7 +348,7 @@ func (hl *HistoryLimiter) doResourceCleanup(ctx context.Context, resource metav1 // Filter resources by status (success/failed) resourcesFiltered := []metav1.Object{} for _, res := range resources { - if getResourceFilterFn(res) { + if getResourceFilterFn(res) && !isPipelineRunOwned(res) { resourcesFiltered = append(resourcesFiltered, res) } } @@ -422,3 +422,19 @@ func (hl *HistoryLimiter) doResourceCleanup(ctx context.Context, resource metav1 return nil } + +// isPipelineRunOwned reports whether a resource belongs to a PipelineRun and +// therefore must be reclaimed with its parent instead of by history limiting. +func isPipelineRunOwned(resource metav1.Object) bool { + if labels := resource.GetLabels(); labels != nil && labels[LabelPipelineRunName] != "" { + return true + } + + for _, ownerReference := range resource.GetOwnerReferences() { + if ownerReference.Kind == KindPipelineRun { + return true + } + } + + return false +} diff --git a/pkg/config/history_limiter_test.go b/pkg/config/history_limiter_test.go index a187a8fe..5512a4ad 100644 --- a/pkg/config/history_limiter_test.go +++ b/pkg/config/history_limiter_test.go @@ -338,3 +338,71 @@ func TestDoResourceCleanup(t *testing.T) { }) } } + +// TestDoResourceCleanupExcludesPipelineRunOwnedResources verifies that history +// limiting preserves completed children of a PipelineRun regardless of whether +// ownership is represented by the standard label or an ownerReference. +func TestDoResourceCleanupExcludesPipelineRunOwnedResources(t *testing.T) { + creationTime := time.Now().Add(-3 * time.Hour) + resources := []metav1.Object{ + &mockResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "standalone-old", + Namespace: "default", + CreationTimestamp: metav1.Time{Time: creationTime}, + }, + completed: true, + successful: true, + }, + &mockResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipeline-child-label", + Namespace: "default", + Labels: map[string]string{LabelPipelineRunName: "pipeline-run"}, + CreationTimestamp: metav1.Time{Time: creationTime.Add(time.Minute)}, + }, + completed: true, + successful: true, + }, + &mockResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipeline-child-owner", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{{ + Kind: KindPipelineRun, + Name: "pipeline-run", + }}, + CreationTimestamp: metav1.Time{Time: creationTime.Add(2 * time.Minute)}, + }, + completed: true, + successful: true, + }, + &mockResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "standalone-new", + Namespace: "default", + CreationTimestamp: metav1.Time{Time: creationTime.Add(3 * time.Minute)}, + }, + completed: true, + successful: true, + }, + } + mockFuncs := &mockResourceFuncs{ + resources: map[string][]metav1.Object{"default": resources}, + successLimit: ptr.Int32(1), + enforceLevel: EnforcedConfigLevelGlobal, + defaultLabelKey: "test.label/name", + } + hl, err := NewHistoryLimiter(mockFuncs) + assert.NoError(t, err) + + ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) + assert.NoError(t, hl.ProcessEvent(ctx, resources[3])) + + remaining, err := mockFuncs.List(ctx, "default", "") + assert.NoError(t, err) + assert.Len(t, remaining, 3) + assert.Equal(t, "pipeline-child-label", remaining[0].GetName()) + assert.Equal(t, "pipeline-child-owner", remaining[1].GetName()) + assert.Equal(t, "standalone-new", remaining[2].GetName()) +} From c4dac9f3fb372bb199f480001de73d5d9f37f098 Mon Sep 17 00:00:00 2001 From: Zichen Yu <1062955096@qq.com> Date: Thu, 8 Oct 2026 14:10:04 +0800 Subject: [PATCH 2/2] refactor(config): share PipelineRun ownership check Centralize label and ownerReference detection in config.IsPipelineRunOwned so history limiting and standalone TaskRun checks use the same logic. Cover both ownership signals, empty labels, and unrelated or mixed owners. Signed-off-by: Zichen Yu <1062955096@qq.com> Assisted-by: GPT-6 (via Codex) --- pkg/config/helper.go | 19 +++++ pkg/config/helper_test.go | 102 +++++++++++++++++++++++++++ pkg/config/history_limiter.go | 20 +----- pkg/reconciler/taskrun/controller.go | 19 +---- 4 files changed, 125 insertions(+), 35 deletions(-) create mode 100644 pkg/config/helper_test.go diff --git a/pkg/config/helper.go b/pkg/config/helper.go index 2e737e5a..debde4c4 100644 --- a/pkg/config/helper.go +++ b/pkg/config/helper.go @@ -24,6 +24,25 @@ import ( // common functions used across history limiter and ttl handler +// IsPipelineRunOwned reports whether a resource belongs to a PipelineRun, +// identified by its PipelineRun label or a PipelineRun ownerReference. +func IsPipelineRunOwned(resource metav1.Object) bool { + // labels contains the ownership signal added to PipelineRun children. + labels := resource.GetLabels() + if labels[LabelPipelineRunName] != "" { + return true + } + + // ownerReference also identifies children that do not carry the label. + for _, ownerReference := range resource.GetOwnerReferences() { + if ownerReference.Kind == KindPipelineRun { + return true + } + } + + return false +} + func getResourceNameLabelKey(resource metav1.Object, defaultLabelKey string) string { annotations := resource.GetAnnotations() // update user defined label key diff --git a/pkg/config/helper_test.go b/pkg/config/helper_test.go new file mode 100644 index 00000000..bf0a7189 --- /dev/null +++ b/pkg/config/helper_test.go @@ -0,0 +1,102 @@ +/* +Copyright 2026 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TestIsPipelineRunOwned verifies both ownership signals and ensures unrelated +// metadata does not prevent standalone resources from being pruned. +func TestIsPipelineRunOwned(t *testing.T) { + // tests covers ownership metadata shared by history and TaskRun processing. + tests := []struct { + name string // name describes the ownership scenario. + metadata metav1.ObjectMeta // metadata contains the labels and owners to check. + want bool // want indicates whether the resource has a PipelineRun parent. + }{ + { + name: "no ownership metadata", + }, + { + name: "empty labels", + metadata: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + }, + { + name: "empty PipelineRun label", + metadata: metav1.ObjectMeta{ + Labels: map[string]string{LabelPipelineRunName: ""}, + }, + }, + { + name: "PipelineRun label", + metadata: metav1.ObjectMeta{ + Labels: map[string]string{LabelPipelineRunName: "pipeline-run"}, + }, + want: true, + }, + { + name: "unrelated label", + metadata: metav1.ObjectMeta{ + Labels: map[string]string{LabelTaskRunName: "task-run"}, + }, + }, + { + name: "PipelineRun owner without label", + metadata: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{{Kind: KindPipelineRun}}, + }, + want: true, + }, + { + name: "unrelated owner", + metadata: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{{Kind: "Pod"}}, + }, + }, + { + name: "PipelineRun among multiple owners", + metadata: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {Kind: "Pod"}, + {Kind: KindPipelineRun}, + }, + }, + want: true, + }, + { + name: "PipelineRun owner with empty label", + metadata: metav1.ObjectMeta{ + Labels: map[string]string{LabelPipelineRunName: ""}, + OwnerReferences: []metav1.OwnerReference{{Kind: KindPipelineRun}}, + }, + want: true, + }, + } + + // testCase contains one ownership scenario and its expected result. + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + assert.Equal(t, testCase.want, IsPipelineRunOwned(&testCase.metadata)) + }) + } +} diff --git a/pkg/config/history_limiter.go b/pkg/config/history_limiter.go index 2b99eabf..a98ca1ee 100644 --- a/pkg/config/history_limiter.go +++ b/pkg/config/history_limiter.go @@ -345,10 +345,10 @@ func (hl *HistoryLimiter) doResourceCleanup(ctx context.Context, resource metav1 return err } - // Filter resources by status (success/failed) + // Filter by status and exclude children reclaimed with their PipelineRun. resourcesFiltered := []metav1.Object{} for _, res := range resources { - if getResourceFilterFn(res) && !isPipelineRunOwned(res) { + if getResourceFilterFn(res) && !IsPipelineRunOwned(res) { resourcesFiltered = append(resourcesFiltered, res) } } @@ -422,19 +422,3 @@ func (hl *HistoryLimiter) doResourceCleanup(ctx context.Context, resource metav1 return nil } - -// isPipelineRunOwned reports whether a resource belongs to a PipelineRun and -// therefore must be reclaimed with its parent instead of by history limiting. -func isPipelineRunOwned(resource metav1.Object) bool { - if labels := resource.GetLabels(); labels != nil && labels[LabelPipelineRunName] != "" { - return true - } - - for _, ownerReference := range resource.GetOwnerReferences() { - if ownerReference.Kind == KindPipelineRun { - return true - } - } - - return false -} diff --git a/pkg/reconciler/taskrun/controller.go b/pkg/reconciler/taskrun/controller.go index adaf3adc..aca09f05 100644 --- a/pkg/reconciler/taskrun/controller.go +++ b/pkg/reconciler/taskrun/controller.go @@ -88,22 +88,7 @@ func filterTaskRun(logger *zap.SugaredLogger, impl *controller.Impl) func(obj in } } -// returns true if the TaskRun is part of a PipelineRun +// isStandaloneTaskRun reports whether a TaskRun does not belong to a PipelineRun. func isStandaloneTaskRun(taskRun metav1.Object) bool { - // verify the taskRun is not part of a pipelineRun - if taskRun.GetLabels() != nil && taskRun.GetLabels()[config.LabelPipelineRunName] != "" { - return false - } - - // if the resource has owner reference as PipelineRun, it is not a standalone TaskRun - // if so, ignore this taskRun - if len(taskRun.GetOwnerReferences()) > 0 { - for _, ownerReference := range taskRun.GetOwnerReferences() { - if ownerReference.Kind == config.KindPipelineRun { - return false - } - } - } - - return true + return !config.IsPipelineRunOwned(taskRun) }