Remove EmbeddedStatus Feature Flag

This commit removes the `EmbeddedStatus` feature flag. It removes all the test cases related with the `full` and `both` `EmbeddedStatus` and only kept the `minimal` test cases as default. In detail, the test cases that checks the `taskruns` or `runs` status from previous was changed to checking referenced `taskruns` or `runs` from the `ChildReferences`.
tektoncd · Feb 3, 2023 · d2f075a · d2f075a
1 parent 7ab2479
commit d2f075a
Show file tree

Hide file tree

Showing 36 changed files with 566 additions and 3,081 deletions.
diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml
@@ -85,11 +85,6 @@ data:
   # in the TaskRun/PipelineRun such as the source from where a remote Task/Pipeline
   # definition was fetched.
   enable-provenance-in-status: "false"
-  # Setting this flag to "full" to enable full embedding of `TaskRun` and `Run` statuses in the
-  # `PipelineRun` status. Set it to "minimal" to populate the `ChildReferences` field in the
-  # `PipelineRun` status with name, kind, and API version information for each `TaskRun` and
-  # `Run` in the `PipelineRun` instead. Set it to "both" to do both.
-  embedded-status: "minimal"
   # Setting this flag will determine the version for custom tasks created by PipelineRuns.
   # Acceptable values are "v1beta1" and "v1alpha1".
   # The default is "v1beta1".

diff --git a/docs/install.md b/docs/install.md
@@ -475,11 +475,6 @@ The default is `false`. For more information, see the [associated issue](https:/
 most stable features to be used. Set it to "alpha" to allow [alpha
 features](#alpha-features) to be used.
 
-- `embedded-status`: set this flag to "full" to enable full embedding of `TaskRun` and `Run` statuses in the
- `PipelineRun` status. Set it to "minimal" to populate the `ChildReferences` field in the `PipelineRun` status with
-  name, kind, and API version information for each `TaskRun` and `Run` in the `PipelineRun` instead. Set it to "both" to
-  do both. For more information, see [Configuring usage of `TaskRun` and `Run` embedded statuses](pipelineruns.md#configuring-usage-of-taskrun-and-run-embedded-statuses).
-
 - `resource-verification-mode`: Setting this flag to "enforce" will enforce verification of tasks/pipeline. Failing to verify will fail the taskrun/pipelinerun. "warn" will only log the err message and "skip" will skip the whole verification.
 - `results-from`: set this flag to "termination-message" to use the container's termination message to fetch results from. This is the default method of extracting results. Set it to "sidecar-logs" to enable use of a results sidecar logs to extract results instead of termination message.
 
@@ -526,7 +521,6 @@ Features currently in "alpha" are:
 | [Debug](./debug.md)                                                                                 | [TEP-0042](https://github.com/tektoncd/community/blob/main/teps/0042-taskrun-breakpoint-on-failure.md)                     | [v0.26.0](https://github.com/tektoncd/pipeline/releases/tag/v0.26.0) |                               |
 | [Step and Sidecar Overrides](./taskruns.md#overriding-task-steps-and-sidecars)                      | [TEP-0094](https://github.com/tektoncd/community/blob/main/teps/0094-specifying-resource-requirements-at-runtime.md)       | [v0.34.0](https://github.com/tektoncd/pipeline/releases/tag/v0.34.0) |                               |
 | [Matrix](./matrix.md)                                                                               | [TEP-0090](https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md)                                            | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) |                               |
-| [Embedded Statuses](pipelineruns.md#configuring-usage-of-taskrun-and-run-embedded-statuses)         | [TEP-0100](https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md) | [v0.35.0](https://github.com/tektoncd/pipeline/releases/tag/v0.35.0) | `embedded-status`             |
 | [Task-level Resource Requirements](compute-resources.md#task-level-compute-resources-configuration) | [TEP-0104](https://github.com/tektoncd/community/blob/main/teps/0104-tasklevel-resource-requirements.md)                   | [v0.39.0](https://github.com/tektoncd/pipeline/releases/tag/v0.39.0) |                               |
 | [Object Params and Results](pipelineruns.md#specifying-parameters)                                  | [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md)                     | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) |                               |
 | [Array Results](pipelineruns.md#specifying-parameters)                                              | [TEP-0076](https://github.com/tektoncd/community/blob/main/teps/0076-array-result-types.md)                                | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) |                               |

diff --git a/docs/matrix.md b/docs/matrix.md
@@ -33,7 +33,6 @@ 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`.
 
 ## Configuring a Matrix
 

diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md
@@ -32,8 +32,7 @@ weight: 500
     - [Specifying <code>LimitRange</code> values](#specifying-limitrange-values)
     - [Configuring a failure timeout](#configuring-a-failure-timeout)
   - [<code>PipelineRun</code> status](#pipelinerun-status)
-    - [The <code>status</code> field](#the-status-field) 
-    - [Configuring usage of <code>TaskRun</code> and <code>Run</code> embedded statuses](#configuring-usage-of-taskrun-and-run-embedded-statuses)
+    - [The <code>status</code> field](#the-status-field)
     - [Monitoring execution status](#monitoring-execution-status)
   - [Cancelling a <code>PipelineRun</code>](#cancelling-a-pipelinerun)
   - [Gracefully cancelling a <code>PipelineRun</code>](#gracefully-cancelling-a-pipelinerun)
@@ -1389,21 +1388,6 @@ Your `PipelineRun`'s `status` field can contain the following fields:
   - `finallyStartTime`- The time at which the PipelineRun's `finally` Tasks, if any, began
   executing, in [RFC3339](https://tools.ietf.org/html/rfc3339) format.
 
-### Configuring usage of `TaskRun` and `Run` embedded statuses
-
-Currently, the default behavior is to pupulate `status.childReferences` with references to the `TaskRun`s and
-`Run`s, which can be used to look up their statuses.
-
-This behavior can be controlled by changing the `embedded-status` feature flag in the `feature-flags`
-config map. See [`install.md`](./install.md#customizing-the-pipelines-controller-behavior) for more
-information on feature flags. The possible values for `embedded-status` are:
-- `minimal` - The current default behavior, populate `status.childReferences`, not `status.taskRuns` or `status.runs`.
-- `full` - Populating `status.taskRuns` and `status.runs`, without populating `status.childReferences`.
-- `both` - Populate `status.childReferences` as well as `status.taskRuns` and `status.runs`.
-
-*Note that after the `PipelineRunStatus` migration as planned in [TEP-100](https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md#2-deprecate-and-remove-full-embedded-status),
-[the `full` and `both` `embedded-status` options will be removed](https://github.com/tektoncd/pipeline/blob/main/docs/deprecations.md).
-
 ### Monitoring execution status
 
 As your `PipelineRun` executes, its `status` field accumulates information on the execution of each `TaskRun`

diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go
@@ -33,15 +33,6 @@ const (
 	AlphaAPIFields = "alpha"
 	// BetaAPIFields is the value used for "enable-api-fields" when beta APIs should be usable as well.
 	BetaAPIFields = "beta"
-	// FullEmbeddedStatus is the value used for "embedded-status" when the full statuses of TaskRuns and Runs should be
-	// embedded in PipelineRunStatusFields, but ChildReferences should not be used.
-	FullEmbeddedStatus = "full"
-	// BothEmbeddedStatus is the value used for "embedded-status" when full embedded statuses of TaskRuns and Runs as
-	// well as ChildReferences should be used in PipelineRunStatusFields.
-	BothEmbeddedStatus = "both"
-	// MinimalEmbeddedStatus is the value used for "embedded-status" when only ChildReferences should be used in
-	// PipelineRunStatusFields.
-	MinimalEmbeddedStatus = "minimal"
 	// EnforceResourceVerificationMode is the value used for "resource-verification-mode" when verification is applied and fail the
 	// TaskRun or PipelineRun when verification fails
 	EnforceResourceVerificationMode = "enforce"
@@ -76,8 +67,6 @@ const (
 	DefaultEnableAPIFields = StableAPIFields
 	// DefaultSendCloudEventsForRuns is the default value for "send-cloudevents-for-runs".
 	DefaultSendCloudEventsForRuns = false
-	// DefaultEmbeddedStatus is the default value for "embedded-status".
-	DefaultEmbeddedStatus = MinimalEmbeddedStatus
 	// EnforceNonfalsifiabilityWithSpire is the value used for  "enable-nonfalsifiability" when SPIRE is used to enable non-falsifiability.
 	EnforceNonfalsifiabilityWithSpire = "spire"
 	// EnforceNonfalsifiabilityNone is the value used for  "enable-nonfalsifiability" when non-falsifiability is not enabled.
@@ -103,7 +92,6 @@ const (
 	enableTektonOCIBundles              = "enable-tekton-oci-bundles"
 	enableAPIFields                     = "enable-api-fields"
 	sendCloudEventsForRuns              = "send-cloudevents-for-runs"
-	embeddedStatus                      = "embedded-status"
 	enforceNonfalsifiability            = "enforce-nonfalsifiability"
 	verificationMode                    = "resource-verification-mode"
 	enableProvenanceInStatus            = "enable-provenance-in-status"
@@ -124,7 +112,6 @@ type FeatureFlags struct {
 	EnableAPIFields                  string
 	SendCloudEventsForRuns           bool
 	AwaitSidecarReadiness            bool
-	EmbeddedStatus                   string
 	EnforceNonfalsifiability         string
 	ResourceVerificationMode         string
 	EnableProvenanceInStatus         bool
@@ -195,9 +182,6 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
 	if err := setFeature(sendCloudEventsForRuns, DefaultSendCloudEventsForRuns, &tc.SendCloudEventsForRuns); err != nil {
 		return nil, err
 	}
-	if err := setEmbeddedStatus(cfgMap, DefaultEmbeddedStatus, &tc.EmbeddedStatus); err != nil {
-		return nil, err
-	}
 	if err := setResourceVerificationMode(cfgMap, DefaultResourceVerificationMode, &tc.ResourceVerificationMode); err != nil {
 		return nil, err
 	}
@@ -257,22 +241,6 @@ func setEnabledAPIFields(cfgMap map[string]string, defaultValue string, feature
 	return nil
 }
 
-// setEmbeddedStatus sets the "embedded-status" flag based on the content of a given map.
-// If the feature gate is invalid or missing then an error is returned.
-func setEmbeddedStatus(cfgMap map[string]string, defaultValue string, feature *string) error {
-	value := defaultValue
-	if cfg, ok := cfgMap[embeddedStatus]; ok {
-		value = strings.ToLower(cfg)
-	}
-	switch value {
-	case FullEmbeddedStatus, BothEmbeddedStatus, MinimalEmbeddedStatus:
-		*feature = value
-	default:
-		return fmt.Errorf("invalid value for feature flag %q: %q", embeddedStatus, value)
-	}
-	return nil
-}
-
 // setResultExtractionMethod sets the "results-from" flag based on the content of a given map.
 // If the feature gate is invalid or missing then an error is returned.
 func setResultExtractionMethod(cfgMap map[string]string, defaultValue string, feature *string) error {

diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go
@@ -47,7 +47,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				EnableTektonOCIBundles:   config.DefaultEnableTektonOciBundles,
 				EnableAPIFields:          config.DefaultEnableAPIFields,
 				SendCloudEventsForRuns:   config.DefaultSendCloudEventsForRuns,
-				EmbeddedStatus:           config.DefaultEmbeddedStatus,
 				ResourceVerificationMode: config.DefaultResourceVerificationMode,
 				EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus,
 				ResultExtractionMethod:   config.DefaultResultExtractionMethod,
@@ -65,7 +64,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				EnableTektonOCIBundles:           true,
 				EnableAPIFields:                  "alpha",
 				SendCloudEventsForRuns:           true,
-				EmbeddedStatus:                   "both",
 				EnforceNonfalsifiability:         "spire",
 				ResourceVerificationMode:         "enforce",
 				EnableProvenanceInStatus:         true,
@@ -88,7 +86,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				AwaitSidecarReadiness:            config.DefaultAwaitSidecarReadiness,
 				RequireGitSSHSecretKnownHosts:    config.DefaultRequireGitSSHSecretKnownHosts,
 				SendCloudEventsForRuns:           config.DefaultSendCloudEventsForRuns,
-				EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 				ResourceVerificationMode:         config.DefaultResourceVerificationMode,
 				ResultExtractionMethod:           config.DefaultResultExtractionMethod,
 				MaxResultSize:                    config.DefaultMaxResultSize,
@@ -107,7 +104,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				AwaitSidecarReadiness:            config.DefaultAwaitSidecarReadiness,
 				RequireGitSSHSecretKnownHosts:    config.DefaultRequireGitSSHSecretKnownHosts,
 				SendCloudEventsForRuns:           config.DefaultSendCloudEventsForRuns,
-				EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 				ResourceVerificationMode:         config.DefaultResourceVerificationMode,
 				ResultExtractionMethod:           config.DefaultResultExtractionMethod,
 				MaxResultSize:                    config.DefaultMaxResultSize,
@@ -126,7 +122,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				AwaitSidecarReadiness:            config.DefaultAwaitSidecarReadiness,
 				RequireGitSSHSecretKnownHosts:    config.DefaultRequireGitSSHSecretKnownHosts,
 				SendCloudEventsForRuns:           config.DefaultSendCloudEventsForRuns,
-				EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 				ResourceVerificationMode:         config.DefaultResourceVerificationMode,
 				ResultExtractionMethod:           config.DefaultResultExtractionMethod,
 				MaxResultSize:                    config.DefaultMaxResultSize,
@@ -137,7 +132,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 		{
 			expectedConfig: &config.FeatureFlags{
 				EnableAPIFields:                  "alpha",
-				EmbeddedStatus:                   "minimal",
 				EnforceNonfalsifiability:         "spire",
 				EnableTektonOCIBundles:           true,
 				ResourceVerificationMode:         config.DefaultResourceVerificationMode,
@@ -152,7 +146,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 		{
 			expectedConfig: &config.FeatureFlags{
 				EnableAPIFields:                  "stable",
-				EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 				ResourceVerificationMode:         config.DefaultResourceVerificationMode,
 				RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
 				AwaitSidecarReadiness:            config.DefaultAwaitSidecarReadiness,
@@ -184,7 +177,6 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) {
 		EnableTektonOCIBundles:           config.DefaultEnableTektonOciBundles,
 		EnableAPIFields:                  config.DefaultEnableAPIFields,
 		SendCloudEventsForRuns:           config.DefaultSendCloudEventsForRuns,
-		EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 		EnforceNonfalsifiability:         config.DefaultEnforceNonfalsifiability,
 		ResourceVerificationMode:         config.DefaultResourceVerificationMode,
 		EnableProvenanceInStatus:         config.DefaultEnableProvenanceInStatus,
@@ -229,8 +221,6 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) {
 		fileName: "feature-flags-invalid-boolean",
 	}, {
 		fileName: "feature-flags-invalid-enable-api-fields",
-	}, {
-		fileName: "feature-flags-invalid-embedded-status",
 	}, {
 		fileName: "feature-flags-invalid-resource-verification-mode",
 	}, {

diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml
@@ -26,7 +26,6 @@ data:
   enable-custom-tasks: "true"
   enable-api-fields: "alpha"
   send-cloudevents-for-runs: "true"
-  embedded-status: "both"
   enforce-nonfalsifiability: "spire"
   resource-verification-mode: "enforce"
   enable-provenance-in-status: "true"

diff --git a/pkg/apis/config/testdata/feature-flags-invalid-embedded-status.yaml b/pkg/apis/config/testdata/feature-flags-invalid-embedded-status.yaml
diff --git a/pkg/apis/config/testing/feature_flags.go b/pkg/apis/config/testing/feature_flags.go
diff --git a/pkg/apis/pipeline/v1/pipeline_types.go b/pkg/apis/pipeline/v1/pipeline_types.go
@@ -283,9 +283,6 @@ 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(version.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))

diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go
@@ -591,10 +591,9 @@ func TestPipelineTaskList_Deps(t *testing.T) {
 
 func TestPipelineTask_validateMatrix(t *testing.T) {
 	tests := []struct {
-		name           string
-		pt             *PipelineTask
-		embeddedStatus string
-		wantErrs       *apis.FieldError
+		name     string
+		pt       *PipelineTask
+		wantErrs *apis.FieldError
 	}{{
 		name: "parameter duplicated in matrix and params",
 		pt: &PipelineTask{
@@ -681,45 +680,11 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
 					Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}},
 				}}},
 		},
-	}, {
-		name: "pipeline has a matrix but embedded status is full",
-		pt: &PipelineTask{
-			Name: "task",
-			Matrix: &Matrix{
-				Params: []Param{{
-					Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
-				}, {
-					Name: "barfoo", Value: ParamValue{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: &Matrix{
-				Params: []Param{{
-					Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
-				}, {
-					Name: "barfoo", Value: ParamValue{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,