Skip to content

Commit 0c99578

Browse files
committed
Add Support for managedBy field in TaskRun and PipelineRun
Added a "managedBy" field to delegate responsibility of controlling the lifecycle of PipelineRuns/TaskRuns. The semantics of the field: Whenever the value is set, and it does not point to the built-in controller, then we skip the reconciliation. * The field is immutable * The field is not defaulted
1 parent c4cf850 commit 0c99578

File tree

11 files changed

+390
-21
lines changed

11 files changed

+390
-21
lines changed

pkg/apis/pipeline/register.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ const (
5555
// MemberOfLabelKey is used as the label identifier for a PipelineTask
5656
// Set to Tasks/Finally depending on the position of the PipelineTask
5757
MemberOfLabelKey = GroupName + "/memberOf"
58+
59+
// PipelineManagedBy is the value of the "managedBy" field for resources
60+
// managed by the Tekton Pipeline controller.
61+
PipelineManagedBy = GroupName + "/pipeline"
5862
)
5963

6064
var (

pkg/apis/pipeline/v1/pipelinerun_types.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ import (
3636
duckv1 "knative.dev/pkg/apis/duck/v1"
3737
)
3838

39+
const (
40+
// PipelineRunByTektonController is the value for the managedBy field to indicate
41+
// that the PipelineRun is managed by the Tekton Pipelines controller.
42+
PipelineRunByTektonController = "tekton.dev/pipeline"
43+
)
44+
3945
// +genclient
4046
// +genreconciler:krshapedlogic=false
4147
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -283,6 +289,10 @@ type PipelineRunSpec struct {
283289
// +optional
284290
// +listType=atomic
285291
TaskRunSpecs []PipelineTaskRunSpec `json:"taskRunSpecs,omitempty"`
292+
// ManagedBy is the platform that is managing this PipelineRun.
293+
// This field is immutable.
294+
// +optional
295+
ManagedBy string `json:"managedBy,omitempty"`
286296
}
287297

288298
// TimeoutFields allows granular specification of pipeline, task, and finally timeouts

pkg/apis/pipeline/v1/pipelinerun_validation.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,18 @@ func (ps *PipelineRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.Field
140140
old := &oldObj.Spec
141141

142142
// If already in the done state, the spec cannot be modified. Otherwise, only the status field can be modified.
143-
tips := "Once the PipelineRun is complete, no updates are allowed"
144-
if !oldObj.IsDone() {
145-
old = old.DeepCopy()
146-
old.Status = ps.Status
147-
tips = "Once the PipelineRun has started, only status updates are allowed"
148-
}
149-
if !equality.Semantic.DeepEqual(old, ps) {
150-
errs = errs.Also(apis.ErrInvalidValue(tips, ""))
143+
if old.ManagedBy != ps.ManagedBy {
144+
errs = errs.Also(apis.ErrInvalidValue("managedBy is immutable", "spec.managedBy"))
145+
} else {
146+
tips := "Once the PipelineRun is complete, no updates are allowed"
147+
if !oldObj.IsDone() {
148+
old = old.DeepCopy()
149+
old.Status = ps.Status
150+
tips = "Once the PipelineRun has started, only status updates are allowed"
151+
}
152+
if !equality.Semantic.DeepEqual(old, ps) {
153+
errs = errs.Also(apis.ErrInvalidValue(tips, ""))
154+
}
151155
}
152156

153157
return

pkg/apis/pipeline/v1/pipelinerun_validation_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,6 +1667,56 @@ func TestPipelineRunSpec_ValidateUpdate(t *testing.T) {
16671667
Message: `invalid value: Once the PipelineRun is complete, no updates are allowed`,
16681668
Paths: []string{""},
16691669
},
1670+
}, {
1671+
name: "is update ctx, baseline is not done, managedBy changes",
1672+
baselinePipelineRun: &v1.PipelineRun{
1673+
Spec: v1.PipelineRunSpec{
1674+
ManagedBy: "tekton.dev/pipeline",
1675+
},
1676+
Status: v1.PipelineRunStatus{
1677+
Status: duckv1.Status{
1678+
Conditions: duckv1.Conditions{
1679+
{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
1680+
},
1681+
},
1682+
},
1683+
},
1684+
pipelineRun: &v1.PipelineRun{
1685+
Spec: v1.PipelineRunSpec{
1686+
ManagedBy: "some-other-controller",
1687+
},
1688+
},
1689+
isCreate: false,
1690+
isUpdate: true,
1691+
expectedError: apis.FieldError{
1692+
Message: `invalid value: managedBy is immutable`,
1693+
Paths: []string{"spec.managedBy"},
1694+
},
1695+
}, {
1696+
name: "is update ctx, baseline is unknown, managedBy changes",
1697+
baselinePipelineRun: &v1.PipelineRun{
1698+
Spec: v1.PipelineRunSpec{
1699+
ManagedBy: "tekton.dev/pipeline",
1700+
},
1701+
Status: v1.PipelineRunStatus{
1702+
Status: duckv1.Status{
1703+
Conditions: duckv1.Conditions{
1704+
{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
1705+
},
1706+
},
1707+
},
1708+
},
1709+
pipelineRun: &v1.PipelineRun{
1710+
Spec: v1.PipelineRunSpec{
1711+
ManagedBy: "some-other-controller",
1712+
},
1713+
},
1714+
isCreate: false,
1715+
isUpdate: true,
1716+
expectedError: apis.FieldError{
1717+
Message: `invalid value: managedBy is immutable`,
1718+
Paths: []string{"spec.managedBy"},
1719+
},
16701720
},
16711721
}
16721722

pkg/apis/pipeline/v1/taskrun_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ type TaskRunSpec struct {
8585
SidecarSpecs []TaskRunSidecarSpec `json:"sidecarSpecs,omitempty"`
8686
// Compute resources to use for this TaskRun
8787
ComputeResources *corev1.ResourceRequirements `json:"computeResources,omitempty"`
88+
// ManagedBy is the platform that is managing this TaskRun.
89+
// This field is immutable.
90+
// +optional
91+
ManagedBy string `json:"managedBy,omitempty"`
8892
}
8993

9094
// TaskRunSpecStatus defines the TaskRun spec status the user can provide

pkg/apis/pipeline/v1/taskrun_validation.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,16 +135,20 @@ func (ts *TaskRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.FieldErro
135135

136136
// If already in the done state, the spec cannot be modified.
137137
// Otherwise, only the status, statusMessage field can be modified.
138-
tips := "Once the TaskRun is complete, no updates are allowed"
139-
if !oldObj.IsDone() {
140-
old = old.DeepCopy()
141-
old.Status = ts.Status
142-
old.StatusMessage = ts.StatusMessage
143-
tips = "Once the TaskRun has started, only status and statusMessage updates are allowed"
144-
}
138+
if old.ManagedBy != ts.ManagedBy {
139+
errs = errs.Also(apis.ErrInvalidValue("managedBy is immutable", "spec.managedBy"))
140+
} else {
141+
tips := "Once the TaskRun is complete, no updates are allowed"
142+
if !oldObj.IsDone() {
143+
old = old.DeepCopy()
144+
old.Status = ts.Status
145+
old.StatusMessage = ts.StatusMessage
146+
tips = "Once the TaskRun has started, only status and statusMessage updates are allowed"
147+
}
145148

146-
if !equality.Semantic.DeepEqual(old, ts) {
147-
errs = errs.Also(apis.ErrInvalidValue(tips, ""))
149+
if !equality.Semantic.DeepEqual(old, ts) {
150+
errs = errs.Also(apis.ErrInvalidValue(tips, ""))
151+
}
148152
}
149153

150154
return

pkg/apis/pipeline/v1/taskrun_validation_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,56 @@ func TestTaskRunSpec_ValidateUpdate(t *testing.T) {
11071107
Message: `invalid value: Once the TaskRun is complete, no updates are allowed`,
11081108
Paths: []string{""},
11091109
},
1110+
}, {
1111+
name: "is update ctx, baseline is not done, managedBy changes",
1112+
baselineTaskRun: &v1.TaskRun{
1113+
Spec: v1.TaskRunSpec{
1114+
ManagedBy: "tekton.dev/pipeline",
1115+
},
1116+
Status: v1.TaskRunStatus{
1117+
Status: duckv1.Status{
1118+
Conditions: duckv1.Conditions{
1119+
{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
1120+
},
1121+
},
1122+
},
1123+
},
1124+
taskRun: &v1.TaskRun{
1125+
Spec: v1.TaskRunSpec{
1126+
ManagedBy: "some-other-controller",
1127+
},
1128+
},
1129+
isCreate: false,
1130+
isUpdate: true,
1131+
expectedError: apis.FieldError{
1132+
Message: `invalid value: managedBy is immutable`,
1133+
Paths: []string{"spec.managedBy"},
1134+
},
1135+
}, {
1136+
name: "is update ctx, baseline is unknown, managedBy changes",
1137+
baselineTaskRun: &v1.TaskRun{
1138+
Spec: v1.TaskRunSpec{
1139+
ManagedBy: "tekton.dev/pipeline",
1140+
},
1141+
Status: v1.TaskRunStatus{
1142+
Status: duckv1.Status{
1143+
Conditions: duckv1.Conditions{
1144+
{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
1145+
},
1146+
},
1147+
},
1148+
},
1149+
taskRun: &v1.TaskRun{
1150+
Spec: v1.TaskRunSpec{
1151+
ManagedBy: "some-other-controller",
1152+
},
1153+
},
1154+
isCreate: false,
1155+
isUpdate: true,
1156+
expectedError: apis.FieldError{
1157+
Message: `invalid value: managedBy is immutable`,
1158+
Paths: []string{"spec.managedBy"},
1159+
},
11101160
},
11111161
}
11121162

pkg/reconciler/pipelinerun/controller.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,17 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock) func(contex
9696
logging.FromContext(ctx).Panicf("Couldn't register Secret informer event handler: %w", err)
9797
}
9898

99-
if _, err := pipelineRunInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)); err != nil {
99+
if _, err := pipelineRunInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
100+
FilterFunc: func(obj interface{}) bool {
101+
if pr, ok := obj.(*v1.PipelineRun); ok {
102+
if pr.Spec.ManagedBy != "" && pr.Spec.ManagedBy != v1.PipelineRunByTektonController {
103+
return false
104+
}
105+
}
106+
return true
107+
},
108+
Handler: controller.HandleAll(impl.Enqueue),
109+
}); err != nil {
100110
logging.FromContext(ctx).Panicf("Couldn't register PipelineRun informer event handler: %w", err)
101111
}
102112

pkg/reconciler/pipelinerun/pipelinerun_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources"
4848
taskresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
4949
th "github.com/tektoncd/pipeline/pkg/reconciler/testing"
50+
ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
5051
"github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim"
5152
resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
5253
remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
@@ -18222,3 +18223,89 @@ spec:
1822218223
})
1822318224
}
1822418225
}
18226+
18227+
func TestReconcile_ManagedBy(t *testing.T) {
18228+
namespace := "foo"
18229+
prManagedByTektonName := "test-pr-managed-by-tekton"
18230+
prManagedByOtherName := "test-pr-managed-by-other"
18231+
18232+
ps := []*v1.Pipeline{simpleHelloWorldPipeline}
18233+
ts := []*v1.Task{simpleHelloWorldTask}
18234+
18235+
prs := []*v1.PipelineRun{
18236+
parse.MustParseV1PipelineRun(t, fmt.Sprintf(`
18237+
metadata:
18238+
name: %s
18239+
namespace: %s
18240+
spec:
18241+
pipelineRef:
18242+
name: test-pipeline
18243+
managedBy: tekton.dev/pipeline
18244+
`, prManagedByTektonName, namespace)),
18245+
parse.MustParseV1PipelineRun(t, fmt.Sprintf(`
18246+
metadata:
18247+
name: %s
18248+
namespace: %s
18249+
spec:
18250+
pipelineRef:
18251+
name: test-pipeline
18252+
managedBy: other-controller
18253+
`, prManagedByOtherName, namespace)),
18254+
}
18255+
18256+
// This data is filtered and simulates what the informer would pass to the reconciler.
18257+
// It only contains the PipelineRun that should be reconciled.
18258+
filteredData := test.Data{
18259+
PipelineRuns: []*v1.PipelineRun{prs[0]}, // Only the one managed by Tekton
18260+
Pipelines: ps,
18261+
Tasks: ts,
18262+
ConfigMaps: th.NewFeatureFlagsConfigMapInSlice(),
18263+
}
18264+
18265+
// Initialize controller with filtered data for the listers
18266+
prt := newPipelineRunTest(t, filteredData)
18267+
defer prt.Cancel()
18268+
18269+
// Create clients with all the data for verification
18270+
ctx, _ := ttesting.SetupFakeContext(t)
18271+
clients, _ := test.SeedTestData(t, ctx, test.Data{
18272+
PipelineRuns: prs,
18273+
Pipelines: ps,
18274+
Tasks: ts,
18275+
ConfigMaps: th.NewFeatureFlagsConfigMapInSlice(),
18276+
})
18277+
18278+
// Verify no TaskRuns were created for the externally managed PipelineRun
18279+
taskRunsOther := getTaskRunsForPipelineRun(prt.TestAssets.Ctx, t, clients, namespace, prManagedByOtherName)
18280+
if len(taskRunsOther) != 0 {
18281+
t.Errorf("Expected no TaskRuns for externally managed PipelineRun, but found %d", len(taskRunsOther))
18282+
}
18283+
18284+
// Verify its status is unchanged
18285+
reconciledOther, err := clients.Pipeline.TektonV1().PipelineRuns(namespace).Get(prt.TestAssets.Ctx, prManagedByOtherName, metav1.GetOptions{})
18286+
if err != nil {
18287+
t.Fatalf("Failed to get externally managed PipelineRun: %v", err)
18288+
}
18289+
if len(reconciledOther.Status.Conditions) != 0 {
18290+
t.Errorf("Expected externally managed PipelineRun to have no conditions, but it did")
18291+
}
18292+
18293+
// 2. Reconcile the PipelineRun managed by "tekton.dev/pipeline"
18294+
// We expect this one to be processed normally.
18295+
wantEvents := []string{
18296+
"Normal Started",
18297+
"Normal Running Tasks Completed: 0",
18298+
}
18299+
reconciledTekton, clients := prt.reconcileRun(namespace, prManagedByTektonName, wantEvents, false)
18300+
18301+
// Verify a TaskRun was created for the Tekton-managed PipelineRun
18302+
taskRunsTekton := getTaskRunsForPipelineRun(prt.TestAssets.Ctx, t, clients, namespace, prManagedByTektonName)
18303+
if len(taskRunsTekton) != 1 {
18304+
t.Errorf("Expected 1 TaskRun for Tekton-managed PipelineRun, but found %d", len(taskRunsTekton))
18305+
}
18306+
18307+
// Verify its status was updated
18308+
if !reconciledTekton.Status.GetCondition(apis.ConditionSucceeded).IsUnknown() {
18309+
t.Errorf("Expected Tekton-managed PipelineRun to be running, but it was not")
18310+
}
18311+
}

pkg/reconciler/taskrun/controller.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,21 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock) func(contex
107107
logging.FromContext(ctx).Panicf("Couldn't register Secret informer event handler: %w", err)
108108
}
109109

110-
if _, err := taskRunInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)); err != nil {
110+
if _, err := taskRunInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
111+
FilterFunc: func(obj interface{}) bool {
112+
tr, ok := obj.(*v1.TaskRun)
113+
if !ok {
114+
// We don't care about anything other than TaskRuns.
115+
return true
116+
}
117+
// The taskrun-controller should not reconcile TaskRuns managed by other controllers.
118+
if tr.Spec.ManagedBy != "" && tr.Spec.ManagedBy != v1.PipelineRunByTektonController {
119+
return false
120+
}
121+
return true
122+
},
123+
Handler: controller.HandleAll(impl.Enqueue),
124+
}); err != nil {
111125
logging.FromContext(ctx).Panicf("Couldn't register TaskRun informer event handler: %w", err)
112126
}
113127

0 commit comments

Comments
 (0)