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

Add support for optional volumes in Tasks #4083

Closed
goldmann opened this issue Jul 9, 2021 · 11 comments
Closed

Add support for optional volumes in Tasks #4083

goldmann opened this issue Jul 9, 2021 · 11 comments
Labels
area/api Indicates an issue or PR that deals with the API. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@goldmann
Copy link

goldmann commented Jul 9, 2021

Feature request

Consider following Task:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: sample
spec:
  workspaces:
    - name: pipeline
  params:
    - name: config-name
      type: string
      description: Name of the ConfigMap containing optional configuration
      default: some-name
  volumes:
    - name: config
      configMap:
        name: $(params.config-name)
        optional: true
  steps:
    - name: execute
      image: dummy
      command: ["run"]
      volumeMounts:
        - name: config
          mountPath: /config

If it would be possible to specify optional: true for the volume we would have a way to mount ConfigMap and Secret only if these are available. This would not fail the run in case it is not available. And this is exactly what is the power of it.

Such feature is already is built-in into Kubernetes and would need to be exposed in Tekton. Docs aren't really great, but:

FWIW: I'm aware that there is support for optional workspaces, but it may not fit all use cases.

Use case

Imagine an established convention for configuration ConfigMap names ins some system. Such ConfigMap's would be always mounted with optional set to true. In case CM would exist - configuration would be provided. This would make it possible to have a plug-in configuration for Tasks. If ConfigMap would not be provided, defaults in the Task would be used.

@goldmann goldmann added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 9, 2021
@bobcatfish
Copy link
Collaborator

Hey @goldmann ! I'm surprised to see a feature being requested around volumes + volume mounts in tasks and would love to hear more about why you prefer these to using workspaces - ive actually been hoping we'll remove volumes + volumemounts in the long run, since they dont fit with the authoring time vs runtime distinction we've tried to draw between tasks and task runs - it looks like you've worked around that in your example by using a param for config-name, but I'm curious why not use workspaces (you mentioned that they may not fit all use cases - do you have some specific ones in mind?)

For exmaple:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: sample
spec:
  workspaces:
    - name: pipeline
    - name: config
      optional: true
      mountPath: /config
  steps:
    - name: execute
      image: dummy
      command: ["run"]

it seems like that would give you everything you need? is it that you want to ensure that the provided workspace is a configmap instead of some other volume? (if so, maybe what is missing is more type information in the workspace declaration?)

@bobcatfish bobcatfish added area/api Indicates an issue or PR that deals with the API. triage/needs-information Indicates an issue needs more information in order to work on it. labels Jul 9, 2021
@goldmann
Copy link
Author

Thanks for the answer! Let me try to go through this.

Lets assume following:

  • We use Pipelines to define our workflow
  • We use PipelineRuns to run these workflows
  • Tasks provide some functionality in a black-box manner and we may (or may not!) control this
  • Pipelines are composed out of Tasks.
  • And most importantly: an automated system that works on top of Tekton wires everything together.

Let's assume we use workspaces to provide the functionality. Task developer defines workspace requirement on the Task. Making it optional makes it possible to use in an automated system. But how to provide data that is understandable to the Task? We have a problem that can be summarized as: how an automated system could be smart enough to wire things together? Some starter questions:

  1. What should be provided as this workspace? Volume? ConfigMap? Secret maybe?
  2. What is the name of the resource to map? (this one is easy - we could have a convention)
  3. At runtime we would need to query Kubernetes and prepare the wiring? What if we use Tekton Triggers as well where we do not have the ability to provide additional logic?
  4. [...] And probably some more.

Now, let's think about using volumes.

Task developer can define explicitly the requirement: "in order to provide configuration, create a CM with XYZ data". If it exists - it is used, if not - nothing happens, Task will still run.

Now the best part - our Operator does not need to know about the volume requirement, it doesn't care even. The whole complexity of checking resources, guessing what to mount, problematic PipelineRun creation within Tekton Triggers just goes away - nothing like this is required anymore. We used built-in Kubernetes functionality to provide values for such systems instead of writing it ourselves. The only required thing would be to define a convention on resource names to prevent clashes, but this one is easy.

Does this help?

@vdemeester
Copy link
Member

Probably a bit related to this is one issue I created recently : #4055

@bobcatfish
Copy link
Collaborator

thanks for the detailed response @goldmann - that definitely helps!

What should be provided as this workspace? Volume? ConfigMap? Secret maybe?

it seems like maybe what is missing is something in the workspace expressing more about what it needs - exactly like @vdemeester is pointing out in that issue he linked.

am i understanding correctly: you need to be able to create/provide config-maps/volumes/secrets dynamically, which meet a task's requirements? (and volumes + volume mounts allow you to be more explicit about this so they work better for you than workspaces?) if the workspace declarations in a Task provided enough info that you could tell if they required a volume vs a configmap vs a secret, would that be enough for you to be able to use workspaces instead of volumes and volume mounts?

also, are you able to provide any more details of the specifics of what you're doing, e.g. an example of a task and what kind of value you are dynamically providing for it? no worries if you can't share but the more concrete details the bette (and the more likely we can add the functionality you need!)

@ghost
Copy link

ghost commented Jul 16, 2021

You can include a volumes section in your tasks today. And those can be any VolumeSource you like - Configmap w/ optional param is already supported. Here's some example yaml with a bit of volume configuration:

kind: Task
apiVersion: tekton.dev/v1beta1
metadata:
  name: footask
spec:
  volumes:
  - name: conf
    configMap:
      name: foo-bar-config # this doesnt exist in my cluster. the dir will be empty.
      optional: true
  steps:
  - name: foostep
    image: alpine:latest
    volumeMounts:
    - name: conf
      mountPath: /conf
    script: |
      if [ -d /conf ] ; then
        echo "YES THERE IS AN OPTIONAL CONF DIRECTORY!!!"
        ls -lah /conf
      else
        # never reached. there's always a dir, even if no configmap exists.
        echo "NO THERE IS NOT AN OPTIONAL CONF DIRECTORY!!"
      fi
      echo "hello world"
---
kind: TaskRun
apiVersion: tekton.dev/v1beta1
metadata:
  name: footaskrun-task-ref
spec:
  taskRef:
    name: footask

Continuing the thread of conversation though - while ConfigMap and Secrets do have an optional param I don't think any other volume types have it. I'm not sure how we'd support this for other volume types without sort of "forking" them from Kubernetes to add the optional field.

The concept of optional volumes also has the side-effect of making the outcome of a run less deterministic: for ex a pod w/ optional secret volume can race with an automated secret creation process. Which state of the volume (populated/unpopulated) will the code running inside the pod end up seeing? Optional Workspaces don't suffer this same racing problem - either they're provided explicitly in the run (and therefore must exist prior to pod creation) or they're not, and the pod will be created without the volume attachment (assuming the workspace is optional ofc).


Wiring workspaces together in a pipeline is still very much not solved though. I do still strongly suspect that there's a solution here that involves workspaces, we just need to keep poking at it I think. Offering a default workspace binding inside a task's or pipeline's workspace declaration sounds like a passable approach to me. The issues I guess that raises are:

  1. Are we ok embedding k8s-specific types into more parts of our task/pipeline apis?
  2. How do we ensure Catalog Tasks don't start including wholly inappropriate default volume configuration? (e.g. a user pushes a catalog task w/ a default volumeClaimTemplate that doesn't work well across all platforms - don't want to start forking the catalog entries by platform)
  3. How does this interact with "optional" Workspaces? It doesn't make sense for an "optional" workspace to also have a default workspace binding. This is possible just a simple validation issue.

Also, pulling in @vdemeester's suggestion from #4055:

Add a way to attach "meaning" to a Workspace in definitions, a bit like metadatas (labels ?). Idea here would be that, by convention, we could "label" a workspace as main (the main sharing data workspace), on with cache or `credentials", …

Automated systems and orgs could decide precisely how they map "meaning" to "volume type". I like this idea a lot but I'm also finding myself poking holes in it. e.g. how do we deal with: mismatched meaning labels between tasks in the catalog, ensuring authors include meaning labels on these tasks in the first place, providing the right set of meaning labels that covers enough use-cases without forcing automated systems to deal with combinatorial explosion, etc etc.

Note: Edited to inline a yaml code example at the top.

@bobcatfish bobcatfish removed the triage/needs-information Indicates an issue needs more information in order to work on it. label Jul 19, 2021
@goldmann
Copy link
Author

Sorry for the delay!

@bobcatfish:

am i understanding correctly: you need to be able to create/provide config-maps/volumes/secrets dynamically, which meet a task's requirements? (and volumes + volume mounts allow you to be more explicit about this so they work better for you than workspaces?)

In my case I consider a ConfigMap or Secret as a configuration for the Task. Task defines what it needs, for example if the implementation of a Task is to connect to some service that requires authentication - a Secret would be the way to satisfy this requirement. This requirement would be specified on the Task level because this is where we need to have it.

Now, continuing with the example - let's look at ConfigMap. Here, the Task can define optional configuration for the Task. For example if you want to override the URL to the service to connect to - you could create a ConfigMap like this:

apiVersion: v1
kind: ConfigMap
metadata:
  name: task-xyz-config
data:
  serviceUrl: http://192.1.2.3/context

If this is provided, it is mounted in the Task. If it's not provided - empty dir is mounted -> file does not exist -> defaults from the Task implementation are used (for example: https://well-known-url.io/blah).

The thing is that the orchestrator that wires everything together is now totally free from trying to understand requirements and providing them. This may be very tricky in some cases.

if the workspace declarations in a Task provided enough info that you could tell if they required a volume vs a configmap vs a secret, would that be enough for you to be able to use workspaces instead of volumes and volume mounts?

This would shift the responsibility of finding out (guessing?) what is needed to the orchestrator. If Task would define a workspace requirement for a CM - we would need to know what CM should be provided there. This could be partially solved by implementing a convention for names - this would limit number of available items to mount, but there is still uncertainity which exactly CM should be mounted. Just a reminder - orchestrator is an automated system. For a manually crafted Pipeline/Run - it's easier.


@sbwsg:

Uhm, ehm, huh? 😕 I'm not exactly sure how it happened, but I was able to apply mine (and yours) example onto my cluster now. I wonder if some upgrades of the components made some difference here. I've updated Kubernetes and Tekton to newer versions.

The concept of optional volumes also has the side-effect of making the outcome of a run less deterministic: for ex a pod w/ optional secret volume can race with an automated secret creation process.

Good catch. To be fair, my biggest use case for optional volumes are ConfigMaps as "optional configuration" (as shown above).

How do we ensure Catalog Tasks don't start including wholly inappropriate default volume configuration?

Slight offtopic here:

If we are touching the Hub... Currently there are no rules what workspaces are requested, meaning: we know only name and some description in the doc, nothing else. If we would like to reuse it in an automated system we have a big problem. I'm exactly in this situation now.

Generally I find Hub tasks not ready for reuse and limited, just two examples:

  • Consider different security context settings and Tasks failing because of (for example) user id mismatch
  • Consider different uid usage - one Task in Pipeline prepares content which is later not readable by other Task in Pipeline.

But back to topic.

If a workspace would have:

  • Declaration of the type (kind)
  • Declaration of the (default?) name

I think this would make it possible to move from volumes into workspaces. What I don't want to do in an automated system is the guessing part.

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: sample
spec:
  workspaces:
    - name: config
      defaultRef:
        kind: ConfigMap
        name: sample-config
      optional: true
...
  • If TaskRun doesn't provide the config workspace:
    • If sample-config ConfigMap exists: use it.
    • If it doesn't - provide emptyDir.
  • If TaskRun provides the config workspace:
    • Use use it.

In general defaultRef (geez, I'm bad at naming...) is used when the workspace is not provided.

Dunno, just an idea.

@bobcatfish
Copy link
Collaborator

just a quick update @goldmann - @sbwsg has opened a proposal to add workspace "type hinting" which might be relevant to your needs: tektoncd/community#510

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 14, 2021
@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 13, 2022
@tekton-robot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Indicates an issue or PR that deals with the API. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

4 participants