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

Make emptyDir the default volume source for workspaces in TaskRun #2398

Closed
jlpettersson opened this issue Apr 15, 2020 · 4 comments · Fixed by #2930
Closed

Make emptyDir the default volume source for workspaces in TaskRun #2398

jlpettersson opened this issue Apr 15, 2020 · 4 comments · Fixed by #2930
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@jlpettersson
Copy link
Member

In the case when we have a Task with a workspace, e.g.

piVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: task
spec:
      steps:
      - name: write
        image: ubuntu
        script: echo $(workspaces.task-ws.volume) > $(workspaces.task-ws.path)/foo
      workspaces:
      - name: task-ws

Expected Behavior

That Tekton could provide me with good default values when possible and the convention is a good fit for the problem.

When I create a TaskRun without defining a workspace volume, I can get a working solution by convention.

Example (YAML) but use case is useful for e.g. cli as well:

apiVersion: tekton.dev/v1beta1
kind: TaskRun
metadata:
  generateName: tr
spec:
  taskRef:
    name: task
  workspaces:
  - name: task-ws

And the TaskRun should successfully run.

Actual Behavior

A volume for the workspace must be specified.

kubectl create -f tr.yaml 
Error from server (BadRequest): error when creating "tr.yaml": admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: expected exactly one, got neither: workspace.configmap, workspace.emptydir, workspace.persistentvolumeclaim, workspace.secret, workspace.volumeclaimtemplate

A user can solve it by adding emptyDir{} for the workspace in the TaskRun

Example:

apiVersion: tekton.dev/v1beta1
kind: TaskRun
metadata:
  generateName: tr
spec:
  taskRef:
    name: task
  workspaces:
  - name: task-ws
    emptyDir: {}

Proposed solution

For a TaskRun an emptyDir{} is almost always possbile to use in most clusters (as what I know) and it is a fair use case, if the workspace is used for passing files between steps

However, for PipelineRun an emptyDir{} is less useful as a workspace since it is combined of multiple pods.

Providing an emptyDir{} volume as default was suggested in the discussion about "Auto workspace"

When e.g. debugging a task in an existing pipeline, this is a useful, quick way to run a task.

@vdemeester
Copy link
Member

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 15, 2020
@ghost
Copy link

ghost commented Apr 15, 2020

Thanks for writing this up! Two thoughts on this:

  • I'd like defaults such as this to be configurable when at all possible. Relying purely on convention should, in my view, be left to higher level tools. So adding some support to our defaults configmap would be preferable over a baked-in constant in the controller code. My question then would become - should we have a separate default for TaskRun default workspace and PipelineRun default workspace?
  • I have been thinking about adding an optional field on workspaces in Tasks, which would allow a Task to declare whether a given workspace is required or not, skipping validation when not. This has been in the context of credentials where a git Task may support ~/.ssh creds as well as ~/.git-credentials creds, but doesnt require both of them. But I think this feature could also be more widely useful so I am still thinking through how it would interact with this feature.

@jlpettersson
Copy link
Member Author

I'd like defaults such as this to be configurable when at all possible. Relying purely on convention should, in my view, be left to higher level tools. So adding some support to our defaults configmap would be preferable over a baked-in constant in the controller code.

I understand your thoughts. But in the case of workspace volume for a TaskRun, I think this would almost always be configured to use an emptydir. Not many other options is useful here. E.g. it is not so useful to allocate storage (PVC) for a single TaskRun.

Here I am assuming that a workspace is most commonly used for passing files used within a run, not when it is pre-populated with files (e.g. dependency cache). Many solutions uses buckets for that use case.

The case for PipelineRun has many more valid options, therefore I wanted to only have TaskRun in scope now.

I have been thinking about adding an optional field on workspaces in Tasks, which would allow a Task to declare whether a given workspace is required or not, skipping validation when not. This has been in the context of credentials where a git Task may support ~/.ssh creds as well as ~/.git-credentials creds, but doesnt require both of them. But I think this feature could also be more widely useful so I am still thinking through how it would interact with this feature.

This is interesting. I think it is a different question - but may be related.

But couldn't the task handle ~/.ssh and ~/.git-credentials as if they are stored on the same volume here? e.g. the same Secret. If not, we could use a projected volume for this (e.g. combining multiple secrets, seen as a single volume for the pod.

I wanted to open a new issue to support projected volume anyway, for another case. I think it makes most sense to have it for a specific task: I want to have support for serviceAccountToken, in the form of Service Account Token Volume Projection. This is a newer form of tokens, that can be renewed often (more secure) and it possibly can be used to federate service accounts to other clusters. This is very useful when you want to deploy to other clusters than were the CI/CD is running (a common case in larger organizations). It is actually useful for multiple tasks, e.g. test-cluster, staging-cluster and prod-cluster.

@ghost
Copy link

ghost commented Apr 15, 2020

But in the case of workspace volume for a TaskRun, I think this would almost always be configured to use an emptydir. Not many other options is useful here. E.g. it is not so useful to allocate storage (PVC) for a single TaskRun.

Yeah, I completely agree with you about the common case. In isolation I think this would make absolute sense to be the default. To expand further on the reasons I have against the convention:

  1. We're in beta now and have an existing behaviour, which is to fail by default if a workspace is not provided in a TaskRun. Making this change to the default would likely be better but would be a surprise for upgrading users. I would therefore prefer to add this as opt-in configuration now and then plan a to make it the default after some amount of time. In other words I want the best of both worlds - an additive change we can make today which supports this desirable feature, and a clear deprecation process for the existing behaviour.

  2. emptyDir is a Kubernetes-ism. It's not discussed very often given the k8s-centric perspective of our community but it is intended that Tekton's specs can be leveraged to execute on non-Kubernetes platforms as well. It would be excellent if workspaces could be expanded to include non-Kubernetes non-VolumeSource sources by an interested platform dev team. We could, for example, allow a wildcard workspace type that lets you embed platform-specific config that the underlying infra would be responsible for spinning up. Regardless, emptyDir simply may not be supported at all on the infra that Tekton executes in. But the default workspace configuration would still be very desirable - it just might not be emptyDir that's the desired default.

  3. Finally, it's possible to disallow empyDir volumes using a PodSecurityPolicy. I have never heard from a team that disallows this but the fact that it's supported by Kubernetes suggests to me that we should at least account for it.

This is interesting. I think it is a different question - but may be related.

I agree that it's a different question and, having thought about it some more, I actually think having a default would be entirely complimentary to optional workspaces.

But couldn't the task handle ~/.ssh and ~/.git-credentials as if they are stored on the same volume here? e.g. the same Secret. If not, we could use a projected volume for this (e.g. combining multiple secrets, seen as a single volume for the pod.

Projected Volumes and Secrets get mounted as read-only. So a git Task that asks for a single Secret to mount into ~ would not be able to clone a repo into ~/src.

Looking forward to your Projected Volumes proposal, I definitely think their inclusion makes sense. I hadn't considered the deploying-to-other-clusters scenario wrt token projection. Good stuff!

jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

Partially fixes tektoncd#2398
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Partially fixes tektoncd#2398
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Partially fixes tektoncd#2398
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Partially fixes tektoncd#2398
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 28, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
jerop pushed a commit to jerop/pipeline that referenced this issue Jul 28, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
tekton-robot pushed a commit that referenced this issue Jul 28, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes #2398 and partially fixes #2595.
Peaorl pushed a commit to Peaorl/pipeline that referenced this issue Aug 10, 2020
Currently, users have to completely specify `Workspaces` they don't care
about or whose configuration should be entirely in the hands of admins.

This PR enables users to specify default `Workspaces` for `TaskRuns`,
for example they can use `emptyDir` by default.

The `WorkspaceBinding` can be set in the `config-defaults` ConfigMap
in `default-task-run-workspace-binding`.

Fixes tektoncd#2398 and partially fixes tektoncd#2595.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants