diff --git a/docs/additional-configs.md b/docs/additional-configs.md index 9ec469efa6e..e4b8ab0c928 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -307,7 +307,6 @@ Features currently in "beta" are: | Feature | Proposal | Alpha Release | Beta Release | Individual Flag | |:-------------------------------------------------------------------|:------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------|:---------------------------------------------------------------------|:----------------| -| [Propagated `Workspaces`](./pipelineruns.md#propagated-workspaces) | [TEP-0111](https://github.com/tektoncd/community/blob/main/teps/0111-propagating-workspaces.md) | [v0.40.0](https://github.com/tektoncd/pipeline/releases/tag/v0.40.0) | [v0.45.0](https://github.com/tektoncd/pipeline/releases/tag/v0.45.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) | [v0.45.0](https://github.com/tektoncd/pipeline/releases/tag/v0.45.0) | | ## Enabling larger results using sidecar logs diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 9f79a8953da..e017a5ae69a 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -855,8 +855,6 @@ Consult the documentation of the custom task that you are using to determine whe #### Propagated Workspaces -**[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 allows authors to simplify specs by automatically propagating top-level diff --git a/docs/taskruns.md b/docs/taskruns.md index 14c18b9456f..f11ede2eb19 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -453,8 +453,6 @@ For more information, see the following topics: #### Propagated Workspaces -**[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 allows authors to simplify specs by automatically propagating top-level diff --git a/examples/v1beta1/pipelineruns/beta/propagating-workspaces.yaml b/examples/v1beta1/pipelineruns/propagating-workspaces.yaml similarity index 100% rename from examples/v1beta1/pipelineruns/beta/propagating-workspaces.yaml rename to examples/v1beta1/pipelineruns/propagating-workspaces.yaml diff --git a/examples/v1beta1/pipelineruns/beta/propagating_workspaces_with_referenced_resources.yaml b/examples/v1beta1/pipelineruns/propagating_workspaces_with_referenced_resources.yaml similarity index 100% rename from examples/v1beta1/pipelineruns/beta/propagating_workspaces_with_referenced_resources.yaml rename to examples/v1beta1/pipelineruns/propagating_workspaces_with_referenced_resources.yaml diff --git a/examples/v1beta1/taskruns/beta/propagating_workspaces.yaml b/examples/v1beta1/taskruns/propagating_workspaces.yaml similarity index 100% rename from examples/v1beta1/taskruns/beta/propagating_workspaces.yaml rename to examples/v1beta1/taskruns/propagating_workspaces.yaml diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 54824f05c1a..c701c7ecf68 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -1033,14 +1033,12 @@ func getTaskrunWorkspaces(ctx context.Context, pr *v1beta1.PipelineRun, rpt *res pipelineRunWorkspaces[binding.Name] = binding } - if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields { - // Propagate required workspaces from pipelineRun to the pipelineTasks - if rpt.PipelineTask.TaskSpec != nil { - rpt, err = propagateWorkspaces(rpt) - if err != nil { - // This error cannot be recovered without modifying the TaskSpec - return nil, "", controller.NewPermanentError(err) - } + // Propagate required workspaces from pipelineRun to the pipelineTasks + if rpt.PipelineTask.TaskSpec != nil { + rpt, err = propagateWorkspaces(rpt) + if err != nil { + // This error cannot be recovered without modifying the TaskSpec + return nil, "", controller.NewPermanentError(err) } } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 2462224bbc3..aab5a0ea1d4 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -7362,10 +7362,9 @@ spec: }, }, } - ctx := config.EnableBetaAPIFields(context.Background()) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, _, err := getTaskrunWorkspaces(ctx, tt.pr, tt.rprt) + _, _, err := getTaskrunWorkspaces(context.Background(), tt.pr, tt.rprt) if err != nil { t.Errorf("Pipeline.getTaskrunWorkspaces() returned error for valid pipeline: %v", err) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 168077bacdf..b442a9a4a93 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -395,10 +395,8 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 var workspaceDeclarations []v1beta1.WorkspaceDeclaration // 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/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.StableAPIFields && tr.Spec.TaskSpec != nil { + // the workspaces provided in the task run spec. We only allow this feature for embedded taskSpec. + if tr.Spec.TaskSpec != nil { for _, ws := range tr.Spec.Workspaces { wspaceDeclaration := v1beta1.WorkspaceDeclaration{Name: ws.Name} workspaceDeclarations = append(workspaceDeclarations, wspaceDeclaration) @@ -777,25 +775,23 @@ func applyParamsContextsResultsAndWorkspaces(ctx context.Context, tr *v1beta1.Ta ts = resources.ApplyStepExitCodePath(ts) // Apply workspace resource substitution - if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields { - // propagate workspaces from taskrun to task. - twn := []string{} - for _, tw := range ts.Workspaces { - twn = append(twn, tw.Name) - } - - for _, trw := range tr.Spec.Workspaces { - skip := false - for _, tw := range twn { - if tw == trw.Name { - skip = true - break - } - } - if !skip { - ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: trw.Name}) + // propagate workspaces from taskrun to task. + twn := []string{} + for _, tw := range ts.Workspaces { + twn = append(twn, tw.Name) + } + + for _, trw := range tr.Spec.Workspaces { + skip := false + for _, tw := range twn { + if tw == trw.Name { + skip = true + break } } + if !skip { + ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: trw.Name}) + } } ts = resources.ApplyWorkspaces(ctx, ts, ts.Workspaces, tr.Spec.Workspaces, workspaceVolumes) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index de6b97d203e..1c53e63d620 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -2532,31 +2532,23 @@ spec: - emptyDir: {} name: tr-workspace `) - 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{}) + d := test.Data{ + TaskRuns: []*v1beta1.TaskRun{taskRun}, + } + 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 26b688c42d1..bbdf42b4b42 100644 --- a/pkg/workspace/apply.go +++ b/pkg/workspace/apply.go @@ -20,7 +20,6 @@ import ( "context" "fmt" - "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" "github.com/tektoncd/pipeline/pkg/substitution" @@ -110,35 +109,29 @@ func Apply(ctx context.Context, ts v1beta1.TaskSpec, wb []v1beta1.WorkspaceBindi isolatedWorkspaces := sets.NewString() - alphaOrBetaEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields - - if alphaOrBetaEnabled { - for _, step := range ts.Steps { - for _, workspaceUsage := range step.Workspaces { - isolatedWorkspaces.Insert(workspaceUsage.Name) - } + for _, step := range ts.Steps { + for _, workspaceUsage := range step.Workspaces { + isolatedWorkspaces.Insert(workspaceUsage.Name) } - for _, sidecar := range ts.Sidecars { - for _, workspaceUsage := range sidecar.Workspaces { - isolatedWorkspaces.Insert(workspaceUsage.Name) - } + } + for _, sidecar := range ts.Sidecars { + for _, workspaceUsage := range sidecar.Workspaces { + isolatedWorkspaces.Insert(workspaceUsage.Name) } } for i := range wb { - if alphaOrBetaEnabled { - // Propagate missing Workspaces - addWorkspace := true - for _, ws := range ts.Workspaces { - if ws.Name == wb[i].Name { - addWorkspace = false - break - } - } - if addWorkspace { - ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: wb[i].Name}) + // Propagate missing Workspaces + addWorkspace := true + for _, ws := range ts.Workspaces { + if ws.Name == wb[i].Name { + addWorkspace = false + break } } + if addWorkspace { + ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: wb[i].Name}) + } w, err := getDeclaredWorkspace(wb[i].Name, ts.Workspaces) if err != nil { return nil, err @@ -153,15 +146,10 @@ func Apply(ctx context.Context, ts v1beta1.TaskSpec, wb []v1beta1.WorkspaceBindi ReadOnly: w.ReadOnly, } - if alphaOrBetaEnabled { - if isolatedWorkspaces.Has(w.Name) { - mountAsIsolatedWorkspace(ts, w.Name, volumeMount) - } else { - mountAsSharedWorkspace(ts, volumeMount) - } + if isolatedWorkspaces.Has(w.Name) { + mountAsIsolatedWorkspace(ts, w.Name, volumeMount) } else { - // Prior to the alpha feature gate only Steps may receive workspaces. - ts.StepTemplate.VolumeMounts = append(ts.StepTemplate.VolumeMounts, volumeMount) + mountAsSharedWorkspace(ts, volumeMount) } // Only add this volume if it hasn't already been added diff --git a/pkg/workspace/apply_test.go b/pkg/workspace/apply_test.go index 516392ae997..85324cc0c63 100644 --- a/pkg/workspace/apply_test.go +++ b/pkg/workspace/apply_test.go @@ -706,7 +706,7 @@ func TestApply_PropagatedWorkspacesFromWorkspaceBindingToDeclarations(t *testing expectedTaskSpec v1beta1.TaskSpec apiField string }{{ - name: "propagate workspaces alpha enabled", + name: "propagate workspaces", ts: v1beta1.TaskSpec{ Workspaces: []v1beta1.WorkspaceDeclaration{{ Name: "workspace1", @@ -740,52 +740,10 @@ 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: tc.apiField, - }, - }) vols := workspace.CreateVolumes(tc.workspaces) - ts, err := workspace.Apply(ctx, tc.ts, tc.workspaces, vols) + ts, err := workspace.Apply(context.Background(), tc.ts, tc.workspaces, vols) if err != nil { t.Fatalf("Did not expect error but got %v", err) } @@ -1135,8 +1093,8 @@ func TestApplyWithMissingWorkspaceDeclaration(t *testing.T) { }, }} vols := workspace.CreateVolumes(bindings) - if _, err := workspace.Apply(context.Background(), ts, bindings, vols); err == nil { - t.Errorf("Expected error because workspace doesnt exist.") + if _, err := workspace.Apply(context.Background(), ts, bindings, vols); err != nil { + t.Errorf("Did not expect error because of workspace propagation but got %v", err) } }