Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate Propagated Workspaces to Stable #6432

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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)
Expand Down Expand Up @@ -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)

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