From 3183cb3505b806eeaba579a651a20aac096eca0e Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Mon, 30 Jan 2023 13:10:14 -0500 Subject: [PATCH] This PR migrates the feature, Propagated Workspaces to Beta as per Issue #5505. No updates were made to the functionality of this feature except updating the feature flag FeatureFlags.EnableAPIFields check from alpha to not stable (meaning that it will only be enabled if the feature flag enable-api-field is alpha or beta). --- docs/pipelineruns.md | 2 +- docs/taskruns.md | 2 +- .../propagating-workspaces.yaml | 0 ..._workspaces_with_referenced_resources.yaml | 0 .../propagating_workspaces.yaml | 0 pkg/reconciler/pipelinerun/pipelinerun.go | 2 +- .../pipelinerun/pipelinerun_test.go | 2 +- pkg/reconciler/taskrun/taskrun.go | 6 +-- pkg/reconciler/taskrun/taskrun_test.go | 46 ++++++++++--------- pkg/workspace/apply.go | 8 ++-- pkg/workspace/apply_test.go | 42 ++++++++++++++++- 11 files changed, 75 insertions(+), 35 deletions(-) rename examples/v1beta1/pipelineruns/{alpha => beta}/propagating-workspaces.yaml (100%) rename examples/v1beta1/pipelineruns/{alpha => beta}/propagating_workspaces_with_referenced_resources.yaml (100%) rename examples/v1beta1/taskruns/{alpha => beta}/propagating_workspaces.yaml (100%) diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 1dafc70a0f0..65e219d0ce9 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -979,7 +979,7 @@ Consult the documentation of the custom task that you are using to determine whe #### Propagated Workspaces -**([alpha only](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#alpha-features))** +**[beta](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#beta-features))** When using an embedded spec, workspaces from the parent `PipelineRun` will be propagated to any inlined specs without needing to be explicitly defined. This diff --git a/docs/taskruns.md b/docs/taskruns.md index 5eec2cfc311..f85ee183861 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -502,7 +502,7 @@ For more information, see the following topics: #### Propagated Workspaces -**([alpha only](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#alpha-features))** +**[beta](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#beta-features))** When using an embedded spec, workspaces from the parent `TaskRun` will be propagated to any inlined specs without needing to be explicitly defined. This diff --git a/examples/v1beta1/pipelineruns/alpha/propagating-workspaces.yaml b/examples/v1beta1/pipelineruns/beta/propagating-workspaces.yaml similarity index 100% rename from examples/v1beta1/pipelineruns/alpha/propagating-workspaces.yaml rename to examples/v1beta1/pipelineruns/beta/propagating-workspaces.yaml diff --git a/examples/v1beta1/pipelineruns/alpha/propagating_workspaces_with_referenced_resources.yaml b/examples/v1beta1/pipelineruns/beta/propagating_workspaces_with_referenced_resources.yaml similarity index 100% rename from examples/v1beta1/pipelineruns/alpha/propagating_workspaces_with_referenced_resources.yaml rename to examples/v1beta1/pipelineruns/beta/propagating_workspaces_with_referenced_resources.yaml diff --git a/examples/v1beta1/taskruns/alpha/propagating_workspaces.yaml b/examples/v1beta1/taskruns/beta/propagating_workspaces.yaml similarity index 100% rename from examples/v1beta1/taskruns/alpha/propagating_workspaces.yaml rename to examples/v1beta1/taskruns/beta/propagating_workspaces.yaml diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index a6423a9f39a..8d1f2b42739 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -1102,7 +1102,7 @@ func getTaskrunWorkspaces(ctx context.Context, pr *v1beta1.PipelineRun, rpt *res pipelineRunWorkspaces[binding.Name] = binding } - if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields { + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields { // Propagate required workspaces from pipelineRun to the pipelineTasks if rpt.PipelineTask.TaskSpec != nil { var err error diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 30554cd00d3..26065d96df8 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -8358,7 +8358,7 @@ spec: }, }, } - ctx := config.EnableAlphaAPIFields(context.Background()) + ctx := config.EnableBetaAPIFields(context.Background()) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { _, _, err := getTaskrunWorkspaces(ctx, tt.pr, tt.rprt) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index bdeac1d8977..e3ca2f76512 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -421,9 +421,9 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 // Propagating workspaces allows users to skip declarations // In order to validate the workspace bindings we create declarations based on // the workspaces provided in the task run spec. This logic is hidden behind the - // alpha feature gate since propagating workspaces is behind that feature gate. + // alpha/beta feature gate since propagating workspaces is behind the beta feature gate. // In addition, we only allow this feature for embedded taskSpec. - if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields && tr.Spec.TaskSpec != nil { + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields && tr.Spec.TaskSpec != nil { for _, ws := range tr.Spec.Workspaces { wspaceDeclaration := v1beta1.WorkspaceDeclaration{Name: ws.Name} workspaceDeclarations = append(workspaceDeclarations, wspaceDeclaration) @@ -832,7 +832,7 @@ func applyParamsContextsResultsAndWorkspaces(ctx context.Context, tr *v1beta1.Ta ts = resources.ApplyStepExitCodePath(ts) // Apply workspace resource substitution - if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields { + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields { // propagate workspaces from taskrun to task. twn := []string{} for _, tw := range ts.Workspaces { diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index fd51e281762..b73f6f86584 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -2780,29 +2780,31 @@ spec: - emptyDir: {} name: tr-workspace `) - d := test.Data{ - TaskRuns: []*v1beta1.TaskRun{taskRun}, - } - d.ConfigMaps = []*corev1.ConfigMap{{ - ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()}, - Data: map[string]string{ - "enable-api-fields": config.AlphaAPIFields, - }, - }} - testAssets, cancel := getTaskRunController(t, d) - defer cancel() - createServiceAccount(t, testAssets, "default", taskRun.Namespace) - c := testAssets.Controller - if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err == nil { - t.Fatalf("Could not reconcile the taskrun: %v", err) - } - getTaskRun, _ := testAssets.Clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(testAssets.Ctx, taskRun.Name, metav1.GetOptions{}) + for _, apiField := range []string{config.BetaAPIFields, config.AlphaAPIFields} { + d := test.Data{ + TaskRuns: []*v1beta1.TaskRun{taskRun}, + } + d.ConfigMaps = []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-api-fields": apiField, + }, + }} + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + createServiceAccount(t, testAssets, "default", taskRun.Namespace) + c := testAssets.Controller + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err == nil { + t.Fatalf("Could not reconcile the taskrun: %v", err) + } + getTaskRun, _ := testAssets.Clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(testAssets.Ctx, taskRun.Name, metav1.GetOptions{}) - want := []v1beta1.WorkspaceDeclaration{{ - Name: "tr-workspace", - }} - if c := cmp.Diff(want, getTaskRun.Status.TaskSpec.Workspaces); c != "" { - t.Errorf("TestPropagatedWorkspaces errored with: %s", diff.PrintWantGot(c)) + want := []v1beta1.WorkspaceDeclaration{{ + Name: "tr-workspace", + }} + if c := cmp.Diff(want, getTaskRun.Status.TaskSpec.Workspaces); c != "" { + t.Errorf("TestPropagatedWorkspaces errored with: %s", diff.PrintWantGot(c)) + } } } diff --git a/pkg/workspace/apply.go b/pkg/workspace/apply.go index 613a025cb88..26b688c42d1 100644 --- a/pkg/workspace/apply.go +++ b/pkg/workspace/apply.go @@ -110,9 +110,9 @@ func Apply(ctx context.Context, ts v1beta1.TaskSpec, wb []v1beta1.WorkspaceBindi isolatedWorkspaces := sets.NewString() - alphaAPIEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields + alphaOrBetaEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields - if alphaAPIEnabled { + if alphaOrBetaEnabled { for _, step := range ts.Steps { for _, workspaceUsage := range step.Workspaces { isolatedWorkspaces.Insert(workspaceUsage.Name) @@ -126,7 +126,7 @@ func Apply(ctx context.Context, ts v1beta1.TaskSpec, wb []v1beta1.WorkspaceBindi } for i := range wb { - if alphaAPIEnabled { + if alphaOrBetaEnabled { // Propagate missing Workspaces addWorkspace := true for _, ws := range ts.Workspaces { @@ -153,7 +153,7 @@ func Apply(ctx context.Context, ts v1beta1.TaskSpec, wb []v1beta1.WorkspaceBindi ReadOnly: w.ReadOnly, } - if alphaAPIEnabled { + if alphaOrBetaEnabled { if isolatedWorkspaces.Has(w.Name) { mountAsIsolatedWorkspace(ts, w.Name, volumeMount) } else { diff --git a/pkg/workspace/apply_test.go b/pkg/workspace/apply_test.go index 95a257e2f40..516392ae997 100644 --- a/pkg/workspace/apply_test.go +++ b/pkg/workspace/apply_test.go @@ -704,8 +704,9 @@ func TestApply_PropagatedWorkspacesFromWorkspaceBindingToDeclarations(t *testing ts v1beta1.TaskSpec workspaces []v1beta1.WorkspaceBinding expectedTaskSpec v1beta1.TaskSpec + apiField string }{{ - name: "propagate workspaces", + name: "propagate workspaces alpha enabled", ts: v1beta1.TaskSpec{ Workspaces: []v1beta1.WorkspaceDeclaration{{ Name: "workspace1", @@ -739,11 +740,48 @@ func TestApply_PropagatedWorkspacesFromWorkspaceBindingToDeclarations(t *testing VolumeMounts: []v1.VolumeMount{{Name: "ws-9l9zj", MountPath: "/workspace/workspace2"}}, }, }, + apiField: config.AlphaAPIFields, + }, { + name: "propagate workspaces beta enabled", + ts: v1beta1.TaskSpec{ + Workspaces: []v1beta1.WorkspaceDeclaration{{ + Name: "workspace1", + }}, + }, + workspaces: []v1beta1.WorkspaceBinding{{ + Name: "workspace2", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }}, + expectedTaskSpec: v1beta1.TaskSpec{ + Volumes: []corev1.Volume{{ + Name: "ws-mz4c7", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }}, + Workspaces: []v1beta1.WorkspaceDeclaration{{ + Name: "workspace1", + MountPath: "", + ReadOnly: false, + }, { + Name: "workspace2", + MountPath: "", + ReadOnly: false, + }}, + StepTemplate: &v1beta1.StepTemplate{ + VolumeMounts: []v1.VolumeMount{{Name: "ws-mz4c7", MountPath: "/workspace/workspace2"}}, + }, + }, + apiField: config.BetaAPIFields, }} { t.Run(tc.name, func(t *testing.T) { ctx := config.ToContext(context.Background(), &config.Config{ FeatureFlags: &config.FeatureFlags{ - EnableAPIFields: "alpha", + EnableAPIFields: tc.apiField, }, }) vols := workspace.CreateVolumes(tc.workspaces)