Skip to content

Commit 1755579

Browse files
jeroptekton-robot
authored andcommitted
TEP-0090: Matrix - Minimal Status is Required
[TEP-0090: Matrix][tep-0090] proposed executing a `PipelineTask` in parallel `TaskRuns` and `Runs` with substitutions from combinations of `Parameters` in a `Matrix`. The status of `PipelineRuns` with fanned-out `PipelineTasks` will list all the `TaskRuns` and `Runs` created. In [TEP-0100][tep-0100] we proposed changes to `PipelineRun` status to reduce the amount of information stored about the status of `TaskRuns` and `Runs` to improve performance, reduce memory bloat and improve extensibility. With the minimal `PipelineRun` status, we can handle `Matrix` without exacerbating the performance and storage issues that were there before. In this change, we validate that minimal embedded status is enabled when a `PipelineTask` has a `Matrix`. [tep-0090]: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md [tep-0100]: https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md
1 parent 881510f commit 1755579

File tree

7 files changed

+137
-4
lines changed

7 files changed

+137
-4
lines changed

docs/matrix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Documentation for specifying `Matrix` in a `Pipeline`:
3030

3131
> :seedling: **`Matrix` is an [alpha](install.md#alpha-features) feature.**
3232
> The `enable-api-fields` feature flag must be set to `"alpha"` to specify `Matrix` in a `PipelineTask`.
33+
> The `embedded-status` feature flag must be set to `"minimal"` to specify `Matrix` in a `PipelineTask`.
3334
>
3435
> :warning: This feature is in a preview mode.
3536
> It is still in a very early stage of development and is not yet fully functional.

pkg/apis/pipeline/v1beta1/pipeline_types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,9 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr
302302
// This is an alpha feature and will fail validation if it's used in a pipeline spec
303303
// when the enable-api-fields feature gate is anything but "alpha".
304304
errs = errs.Also(ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
305+
// Matrix requires "embedded-status" feature gate to be set to "minimal", and will fail
306+
// validation if it is anything but "minimal".
307+
errs = errs.Also(ValidateEmbeddedStatus(ctx, "matrix", config.MinimalEmbeddedStatus))
305308
errs = errs.Also(pt.validateMatrixCombinationsCount(ctx))
306309
}
307310
errs = errs.Also(validateParameterInOneOfMatrixOrParams(pt.Matrix, pt.Params))

pkg/apis/pipeline/v1beta1/pipeline_types_test.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -684,9 +684,10 @@ func TestPipelineTaskList_Validate(t *testing.T) {
684684

685685
func TestPipelineTask_validateMatrix(t *testing.T) {
686686
tests := []struct {
687-
name string
688-
pt *PipelineTask
689-
wantErrs *apis.FieldError
687+
name string
688+
pt *PipelineTask
689+
embeddedStatus string
690+
wantErrs *apis.FieldError
690691
}{{
691692
name: "parameter duplicated in matrix and params",
692693
pt: &PipelineTask{
@@ -770,11 +771,43 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
770771
Name: "browser", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}},
771772
}},
772773
},
774+
}, {
775+
name: "pipeline has a matrix but embedded status is full",
776+
pt: &PipelineTask{
777+
Name: "task",
778+
Matrix: []Param{{
779+
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
780+
}, {
781+
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
782+
}},
783+
},
784+
embeddedStatus: config.FullEmbeddedStatus,
785+
wantErrs: &apis.FieldError{
786+
Message: "matrix requires \"embedded-status\" feature gate to be \"minimal\" but it is \"full\"",
787+
},
788+
}, {
789+
name: "pipeline has a matrix but embedded status is both",
790+
pt: &PipelineTask{
791+
Name: "task",
792+
Matrix: []Param{{
793+
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
794+
}, {
795+
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
796+
}},
797+
},
798+
embeddedStatus: config.BothEmbeddedStatus,
799+
wantErrs: &apis.FieldError{
800+
Message: "matrix requires \"embedded-status\" feature gate to be \"minimal\" but it is \"both\"",
801+
},
773802
}}
774803
for _, tt := range tests {
775804
t.Run(tt.name, func(t *testing.T) {
805+
if tt.embeddedStatus == "" {
806+
tt.embeddedStatus = config.MinimalEmbeddedStatus
807+
}
776808
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
777809
"enable-api-fields": "alpha",
810+
"embedded-status": tt.embeddedStatus,
778811
})
779812
defaults := &config.Defaults{
780813
DefaultMaxMatrixCombinationsCount: 4,

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2749,6 +2749,7 @@ func TestMatrixIncompatibleAPIVersions(t *testing.T) {
27492749
ps := tt.spec
27502750
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
27512751
"enable-api-fields": version,
2752+
"embedded-status": "minimal",
27522753
})
27532754
defaults := &config.Defaults{
27542755
DefaultMaxMatrixCombinationsCount: 4,
@@ -2861,6 +2862,7 @@ func Test_validateMatrix(t *testing.T) {
28612862
t.Run(tt.name, func(t *testing.T) {
28622863
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
28632864
"enable-api-fields": "alpha",
2865+
"embedded-status": "minimal",
28642866
})
28652867
defaults := &config.Defaults{
28662868
DefaultMaxMatrixCombinationsCount: 4,
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
Copyright 2022 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1beta1
18+
19+
import (
20+
"context"
21+
"fmt"
22+
23+
"github.com/tektoncd/pipeline/pkg/apis/config"
24+
"knative.dev/pkg/apis"
25+
)
26+
27+
// ValidateEmbeddedStatus checks that the embedded-status feature gate is set to the wantEmbeddedStatus value and,
28+
// if not, returns an error stating which feature is dependent on the status and what the current status actually is.
29+
func ValidateEmbeddedStatus(ctx context.Context, featureName, wantEmbeddedStatus string) *apis.FieldError {
30+
embeddedStatus := config.FromContextOrDefaults(ctx).FeatureFlags.EmbeddedStatus
31+
if embeddedStatus != wantEmbeddedStatus {
32+
message := fmt.Sprintf(`%s requires "embedded-status" feature gate to be %q but it is %q`, featureName, wantEmbeddedStatus, embeddedStatus)
33+
return apis.ErrGeneric(message)
34+
}
35+
return nil
36+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
Copyright 2022 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1beta1
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
"github.com/tektoncd/pipeline/pkg/apis/config"
24+
)
25+
26+
func TestValidateEmbeddedStatus(t *testing.T) {
27+
status := "minimal"
28+
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
29+
"embedded-status": status,
30+
})
31+
if err != nil {
32+
t.Fatalf("error creating feature flags from map: %v", err)
33+
}
34+
cfg := &config.Config{
35+
FeatureFlags: flags,
36+
}
37+
ctx := config.ToContext(context.Background(), cfg)
38+
if err := ValidateEmbeddedStatus(ctx, "test feature", status); err != nil {
39+
t.Errorf("unexpected error for compatible feature gates: %q", err)
40+
}
41+
}
42+
43+
func TestValidateEmbeddedStatusError(t *testing.T) {
44+
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
45+
"embedded-status": config.FullEmbeddedStatus,
46+
})
47+
if err != nil {
48+
t.Fatalf("error creating feature flags from map: %v", err)
49+
}
50+
cfg := &config.Config{
51+
FeatureFlags: flags,
52+
}
53+
ctx := config.ToContext(context.Background(), cfg)
54+
err = ValidateEmbeddedStatus(ctx, "test feature", config.MinimalEmbeddedStatus)
55+
if err == nil {
56+
t.Errorf("error expected for incompatible feature gates")
57+
}
58+
}

pkg/reconciler/pipelinerun/pipelinerun_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7614,7 +7614,7 @@ spec:
76147614
`),
76157615
}
76167616

7617-
cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())}
7617+
cms := []*corev1.ConfigMap{withEmbeddedStatus(withEnabledAlphaAPIFields(newFeatureFlagsConfigMap()), config.MinimalEmbeddedStatus)}
76187618
cms = append(cms, withMaxMatrixCombinationsCount(newDefaultsConfigMap(), 10))
76197619

76207620
tests := []struct {

0 commit comments

Comments
 (0)