Skip to content

Commit

Permalink
ChildReference status helper functions should support both CustomRuns…
Browse files Browse the repository at this point in the history
… and Runs

fixes #5933

I updated `pkg/status` to support `CustomRun`s early in the work for supporting `CustomRun`s in the `PipelineRun` reconciler, but forgot to add back support for `Run`s as well when we moved to the reconciler supporting both, depending on a feature flag. This broke tektoncd/cli#1838. This change brings back support for `Run`s in the status helper functions, alongside the support for `CustomRun`s.

Signed-off-by: Andrew Bayer <[email protected]>
  • Loading branch information
abayer authored and tekton-robot committed Jan 5, 2023
1 parent 8587b7b commit a1bac54
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 19 deletions.
50 changes: 40 additions & 10 deletions pkg/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
runv1beta1 "github.com/tektoncd/pipeline/pkg/apis/run/v1beta1"
"github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -47,19 +48,33 @@ func GetTaskRunStatusForPipelineTask(ctx context.Context, client versioned.Inter
// GetRunStatusForPipelineTask takes a minimal embedded status child reference and returns the actual CustomRunStatus for the
// PipelineTask. It returns an error if the child reference's kind isn't CustomRun.
func GetRunStatusForPipelineTask(ctx context.Context, client versioned.Interface, ns string, childRef v1beta1.ChildStatusReference) (*v1beta1.CustomRunStatus, error) {
if childRef.Kind != "CustomRun" {
return nil, fmt.Errorf("could not fetch status for PipelineTask %s: should have kind CustomRun, but is %s", childRef.PipelineTaskName, childRef.Kind)
}
var runStatus *v1beta1.CustomRunStatus

r, err := client.TektonV1beta1().CustomRuns(ns).Get(ctx, childRef.Name, metav1.GetOptions{})
if err != nil && !errors.IsNotFound(err) {
return nil, err
}
if r == nil {
return nil, nil
switch childRef.Kind {
case "CustomRun":
r, err := client.TektonV1beta1().CustomRuns(ns).Get(ctx, childRef.Name, metav1.GetOptions{})
if err != nil && !errors.IsNotFound(err) {
return nil, err
}
if r == nil {
return nil, nil
}
runStatus = &r.Status
case "Run":
r, err := client.TektonV1alpha1().Runs(ns).Get(ctx, childRef.Name, metav1.GetOptions{})
if err != nil && !errors.IsNotFound(err) {
return nil, err
}
if r == nil {
return nil, nil
}
asCustomRunStatus := runv1beta1.FromRunStatus(r.Status)
runStatus = &asCustomRunStatus
default:
return nil, fmt.Errorf("could not fetch status for PipelineTask %s: should have kind Run or CustomRun, but is %s", childRef.PipelineTaskName, childRef.Kind)
}

return &r.Status, nil
return runStatus, nil
}

// GetFullPipelineTaskStatuses returns populated TaskRun and Run status maps for a PipelineRun from its ChildReferences.
Expand Down Expand Up @@ -109,6 +124,21 @@ func GetFullPipelineTaskStatuses(ctx context.Context, client versioned.Interface
if r != nil {
runStatuses[cr.Name].Status = &r.Status
}
case "Run":
r, err := client.TektonV1alpha1().Runs(ns).Get(ctx, cr.Name, metav1.GetOptions{})
if err != nil && !errors.IsNotFound(err) {
return nil, nil, err
}

runStatuses[cr.Name] = &v1beta1.PipelineRunRunStatus{
PipelineTaskName: cr.PipelineTaskName,
WhenExpressions: cr.WhenExpressions,
}

if r != nil {
asCustomRunStatus := runv1beta1.FromRunStatus(r.Status)
runStatuses[cr.Name].Status = &asCustomRunStatus
}
default:
// Don't do anything for unknown types.
}
Expand Down
133 changes: 124 additions & 9 deletions pkg/status/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
"github.com/tektoncd/pipeline/test"
Expand Down Expand Up @@ -124,7 +125,7 @@ status:
func TestGetRunStatusForPipelineTask(t *testing.T) {
testCases := []struct {
name string
run *v1beta1.CustomRun
run v1beta1.RunObject
childRef v1beta1.ChildStatusReference
expectedStatus *v1beta1.CustomRunStatus
expectedErr error
Expand All @@ -137,7 +138,7 @@ func TestGetRunStatusForPipelineTask(t *testing.T) {
},
PipelineTaskName: "some-task",
},
expectedErr: errors.New("could not fetch status for PipelineTask some-task: should have kind CustomRun, but is something-else"),
expectedErr: errors.New("could not fetch status for PipelineTask some-task: should have kind Run or CustomRun, but is something-else"),
}, {
name: "run not found",
childRef: v1beta1.ChildStatusReference{
Expand Down Expand Up @@ -171,6 +172,30 @@ status:
}},
},
},
}, {
name: "success with alpha run",
run: parse.MustParseRun(t, `
metadata:
name: some-run
spec: {}
status:
conditions:
- status: "False"
type: Succeeded
`),
childRef: v1beta1.ChildStatusReference{
TypeMeta: runtime.TypeMeta{Kind: "Run"},
Name: "some-run",
PipelineTaskName: "some-task",
},
expectedStatus: &v1beta1.CustomRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
}},
},
},
},
}

Expand All @@ -179,7 +204,12 @@ status:
ctx, _ := ttesting.SetupFakeContext(t)
d := test.Data{}
if tc.run != nil {
d.CustomRuns = []*v1beta1.CustomRun{tc.run}
switch ro := tc.run.(type) {
case *v1beta1.CustomRun:
d.CustomRuns = []*v1beta1.CustomRun{ro}
case *v1alpha1.Run:
d.Runs = []*v1alpha1.Run{ro}
}
}
clients, _ := test.SeedTestData(t, ctx, d)

Expand Down Expand Up @@ -218,10 +248,25 @@ status:
value: aResultValue
`)

run1 := parse.MustParseCustomRun(t, `
customRun1 := parse.MustParseCustomRun(t, `
metadata:
name: pr-run-1
spec: {}
status:
conditions:
- status: "True"
type: Succeeded
results:
- name: foo
value: oof
- name: bar
value: rab
`)

run2 := parse.MustParseRun(t, `
metadata:
name: pr-run-2
spec: {}
status:
conditions:
- status: "True"
Expand All @@ -237,7 +282,7 @@ status:
name string
originalPR *v1beta1.PipelineRun
taskRuns []*v1beta1.TaskRun
runs []*v1beta1.CustomRun
runs []v1beta1.RunObject
expectedTRStatuses map[string]*v1beta1.PipelineRunTaskRunStatus
expectedRunStatuses map[string]*v1beta1.PipelineRunRunStatus
expectedErr error
Expand Down Expand Up @@ -265,14 +310,18 @@ status:
kind: CustomRun
name: pr-run-1
pipelineTaskName: run-1
- apiVersion: tekton.dev/v1alpha1
kind: Run
name: pr-run-2
pipelineTaskName: run-2
conditions:
- message: Not all Tasks in the Pipeline have finished executing
reason: Running
status: Unknown
type: Succeeded
`),
taskRuns: []*v1beta1.TaskRun{tr1},
runs: []*v1beta1.CustomRun{run1},
runs: []v1beta1.RunObject{customRun1, run2},
expectedTRStatuses: mustParseTaskRunStatusMap(t, `
pr-task-1:
pipelineTaskName: task-1
Expand All @@ -296,6 +345,17 @@ pr-run-1:
value: oof
- name: bar
value: rab
pr-run-2:
pipelineTaskName: run-2
status:
conditions:
- status: "True"
type: Succeeded
results:
- name: foo
value: oof
- name: bar
value: rab
`),
expectedErr: nil,
}, {
Expand Down Expand Up @@ -327,14 +387,25 @@ status:
value: oof
- name: bar
value: rab
pr-run-2:
pipelineTaskName: run-2
status:
conditions:
- status: "True"
type: Succeeded
results:
- name: foo
value: oof
- name: bar
value: rab
conditions:
- message: Not all Tasks in the Pipeline have finished executing
reason: Running
status: Unknown
type: Succeeded
`),
taskRuns: []*v1beta1.TaskRun{tr1},
runs: []*v1beta1.CustomRun{run1},
runs: []v1beta1.RunObject{customRun1, run2},
expectedTRStatuses: mustParseTaskRunStatusMap(t, `
pr-task-1:
pipelineTaskName: task-1
Expand All @@ -358,6 +429,17 @@ pr-run-1:
value: oof
- name: bar
value: rab
pr-run-2:
pipelineTaskName: run-2
status:
conditions:
- status: "True"
type: Succeeded
results:
- name: foo
value: oof
- name: bar
value: rab
`),
expectedErr: nil,
}, {
Expand Down Expand Up @@ -389,13 +471,28 @@ status:
value: oof
- name: bar
value: rab
pr-run-2:
pipelineTaskName: run-2
status:
conditions:
- status: "True"
type: Succeeded
results:
- name: foo
value: oof
- name: bar
value: rab
childReferences:
- apiVersion: tekton.dev/v1beta1
kind: TaskRun
name: pr-task-2
pipelineTaskName: task-2
- apiVersion: tekton.dev/v1beta1
kind: CustomRun
name: pr-run-1
pipelineTaskName: run-1
- apiVersion: tekton.dev/v1alpha1
kind: Run
name: pr-run-2
pipelineTaskName: run-2
conditions:
Expand All @@ -405,7 +502,7 @@ status:
type: Succeeded
`),
taskRuns: []*v1beta1.TaskRun{tr1},
runs: []*v1beta1.CustomRun{run1},
runs: []v1beta1.RunObject{customRun1, run2},
expectedTRStatuses: mustParseTaskRunStatusMap(t, `
pr-task-1:
pipelineTaskName: task-1
Expand All @@ -429,6 +526,17 @@ pr-run-1:
value: oof
- name: bar
value: rab
pr-run-2:
pipelineTaskName: run-2
status:
conditions:
- status: "True"
type: Succeeded
results:
- name: foo
value: oof
- name: bar
value: rab
`),
expectedErr: nil,
}, {
Expand Down Expand Up @@ -486,7 +594,14 @@ pr-run-1:
d.TaskRuns = tc.taskRuns
}
if len(tc.runs) > 0 {
d.CustomRuns = tc.runs
for _, r := range tc.runs {
switch ro := r.(type) {
case *v1beta1.CustomRun:
d.CustomRuns = append(d.CustomRuns, ro)
case *v1alpha1.Run:
d.Runs = append(d.Runs, ro)
}
}
}

clients, _ := test.SeedTestData(t, ctx, d)
Expand Down

0 comments on commit a1bac54

Please sign in to comment.