Skip to content

Commit

Permalink
Migrate Propagated Workspaces to Stable
Browse files Browse the repository at this point in the history
[Issue 6107](#6107) Migrate Propagated Workspaces to Stable

Prior to this, propagated workspaces was hidden behind the beta feature gate.
This feature is now a stable feature.
  • Loading branch information
chitrangpatel authored and tekton-robot committed Apr 4, 2023
1 parent 80b80c6 commit ecb11fc
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 137 deletions.
1 change: 0 additions & 1 deletion docs/additional-configs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 6 additions & 8 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
38 changes: 17 additions & 21 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,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)
Expand Down Expand Up @@ -767,25 +765,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)

Expand Down
40 changes: 16 additions & 24 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down
50 changes: 19 additions & 31 deletions pkg/workspace/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
50 changes: 4 additions & 46 deletions pkg/workspace/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
}

Expand Down

0 comments on commit ecb11fc

Please sign in to comment.