Skip to content

Commit

Permalink
TEP-0090: Matrix - Minimal Status is Required
Browse files Browse the repository at this point in the history
[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
  • Loading branch information
jerop authored and tekton-robot committed Jun 22, 2022
1 parent 881510f commit 1755579
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 4 deletions.
1 change: 1 addition & 0 deletions docs/matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Documentation for specifying `Matrix` in a `Pipeline`:

> :seedling: **`Matrix` is an [alpha](install.md#alpha-features) feature.**
> The `enable-api-fields` feature flag must be set to `"alpha"` to specify `Matrix` in a `PipelineTask`.
> The `embedded-status` feature flag must be set to `"minimal"` to specify `Matrix` in a `PipelineTask`.
>
> :warning: This feature is in a preview mode.
> It is still in a very early stage of development and is not yet fully functional.
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr
// This is an alpha feature and will fail validation if it's used in a pipeline spec
// when the enable-api-fields feature gate is anything but "alpha".
errs = errs.Also(ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
// Matrix requires "embedded-status" feature gate to be set to "minimal", and will fail
// validation if it is anything but "minimal".
errs = errs.Also(ValidateEmbeddedStatus(ctx, "matrix", config.MinimalEmbeddedStatus))
errs = errs.Also(pt.validateMatrixCombinationsCount(ctx))
}
errs = errs.Also(validateParameterInOneOfMatrixOrParams(pt.Matrix, pt.Params))
Expand Down
39 changes: 36 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,9 +684,10 @@ func TestPipelineTaskList_Validate(t *testing.T) {

func TestPipelineTask_validateMatrix(t *testing.T) {
tests := []struct {
name string
pt *PipelineTask
wantErrs *apis.FieldError
name string
pt *PipelineTask
embeddedStatus string
wantErrs *apis.FieldError
}{{
name: "parameter duplicated in matrix and params",
pt: &PipelineTask{
Expand Down Expand Up @@ -770,11 +771,43 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
Name: "browser", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}},
}},
},
}, {
name: "pipeline has a matrix but embedded status is full",
pt: &PipelineTask{
Name: "task",
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}},
},
embeddedStatus: config.FullEmbeddedStatus,
wantErrs: &apis.FieldError{
Message: "matrix requires \"embedded-status\" feature gate to be \"minimal\" but it is \"full\"",
},
}, {
name: "pipeline has a matrix but embedded status is both",
pt: &PipelineTask{
Name: "task",
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}},
},
embeddedStatus: config.BothEmbeddedStatus,
wantErrs: &apis.FieldError{
Message: "matrix requires \"embedded-status\" feature gate to be \"minimal\" but it is \"both\"",
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.embeddedStatus == "" {
tt.embeddedStatus = config.MinimalEmbeddedStatus
}
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "alpha",
"embedded-status": tt.embeddedStatus,
})
defaults := &config.Defaults{
DefaultMaxMatrixCombinationsCount: 4,
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2749,6 +2749,7 @@ func TestMatrixIncompatibleAPIVersions(t *testing.T) {
ps := tt.spec
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": version,
"embedded-status": "minimal",
})
defaults := &config.Defaults{
DefaultMaxMatrixCombinationsCount: 4,
Expand Down Expand Up @@ -2861,6 +2862,7 @@ func Test_validateMatrix(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "alpha",
"embedded-status": "minimal",
})
defaults := &config.Defaults{
DefaultMaxMatrixCombinationsCount: 4,
Expand Down
36 changes: 36 additions & 0 deletions pkg/apis/pipeline/v1beta1/status_validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
Copyright 2022 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 v1beta1

import (
"context"
"fmt"

"github.com/tektoncd/pipeline/pkg/apis/config"
"knative.dev/pkg/apis"
)

// ValidateEmbeddedStatus checks that the embedded-status feature gate is set to the wantEmbeddedStatus value and,
// if not, returns an error stating which feature is dependent on the status and what the current status actually is.
func ValidateEmbeddedStatus(ctx context.Context, featureName, wantEmbeddedStatus string) *apis.FieldError {
embeddedStatus := config.FromContextOrDefaults(ctx).FeatureFlags.EmbeddedStatus
if embeddedStatus != wantEmbeddedStatus {
message := fmt.Sprintf(`%s requires "embedded-status" feature gate to be %q but it is %q`, featureName, wantEmbeddedStatus, embeddedStatus)
return apis.ErrGeneric(message)
}
return nil
}
58 changes: 58 additions & 0 deletions pkg/apis/pipeline/v1beta1/status_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright 2022 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 v1beta1

import (
"context"
"testing"

"github.com/tektoncd/pipeline/pkg/apis/config"
)

func TestValidateEmbeddedStatus(t *testing.T) {
status := "minimal"
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
"embedded-status": status,
})
if err != nil {
t.Fatalf("error creating feature flags from map: %v", err)
}
cfg := &config.Config{
FeatureFlags: flags,
}
ctx := config.ToContext(context.Background(), cfg)
if err := ValidateEmbeddedStatus(ctx, "test feature", status); err != nil {
t.Errorf("unexpected error for compatible feature gates: %q", err)
}
}

func TestValidateEmbeddedStatusError(t *testing.T) {
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
"embedded-status": config.FullEmbeddedStatus,
})
if err != nil {
t.Fatalf("error creating feature flags from map: %v", err)
}
cfg := &config.Config{
FeatureFlags: flags,
}
ctx := config.ToContext(context.Background(), cfg)
err = ValidateEmbeddedStatus(ctx, "test feature", config.MinimalEmbeddedStatus)
if err == nil {
t.Errorf("error expected for incompatible feature gates")
}
}
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7614,7 +7614,7 @@ spec:
`),
}

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

tests := []struct {
Expand Down

0 comments on commit 1755579

Please sign in to comment.