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

Allow Tasks in Pipelines to express a dependency on workspaces 'from' previous Tasks #3109

Open
bobcatfish opened this issue Aug 17, 2020 · 12 comments
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@bobcatfish
Copy link
Collaborator

Feature request

In features like conditions we distinguish between different kinds of dependencies:

  1. ordering (runAfter)
  2. resource dependencies (when a Task relies on a result or a PipelineResource from another Task).

Depending on a workspace that has been acted on by a previous Task is a case of (2) but since workspaces do not have a "from" or similar syntax we have no way of expressing (2) and it will always look like (1).

The syntax for this could look like this:

  tasks:
    - name: task1
      taskRef:
        name: write-to-workspace
      workspaces:
        - name: output
          workspace: pipeline-ws1
    - name: task2
      taskRef:
        name: read-from-workspace
      workspaces:
        - name: src
          workspace: $(tasks.task1.workspaces.output)

Or we could use a from syntax like we do with PipelineResources:

  tasks:
    - name: task1
      taskRef:
        name: write-to-workspace
      workspaces:
        - name: output
          workspace: pipeline-ws1
    - name: task2
      taskRef:
        name: read-from-workspace
      workspaces:
        - name: src
           from: $(tasks.task1.workspaces.output)

Probably we'd want to use the first syntax to be consistent with the syntax for results. I prefer from because it's slightly more explicit but that's probably a discussion for another issue!

Use case

Any Pipeline where you want one Task to act on a workspace and another to do something with it. Some examples:

  1. TaskA clones from git to a workspace, TaskB builds the cloned code
  2. TaskA builds a binary - but conditionally, only if the files the binary builds from have changed. TaskB runs a test with that binary, but should not run if TaskA was skipped
@bobcatfish bobcatfish added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 17, 2020
@ghost
Copy link

ghost commented Aug 18, 2020

I agree from might make more sense. In the first syntax snippet the user could reasonably assume that only the name of the workspace will be interpolated. But they'd be wrong - the name of the workspace will be injected and there's an implicit runAfter added to the pipelinetask. from gives a clearer signal, at least to me, that there's something else going on over just the string interpolation.

In fact, thinking about it some more I'm not totally convinced that this needs to be a variable. It isn't quite right to think of it as interpolation. Maybe it should just be a dot-notation without the $(...)? from: tasks.task1.workspaces.output. The reason being: what does it mean to interpolate a different variable into from? Like $(tasks.task1.results.foo)? I don't think that would work because Tekton would have to resolve that workspace name halfway through pipeline execution. I'm not sure that any other variable interpolation can be supported in the from field so I'm not sure we should make it "look" like regular variable interpolation. wdyt?

@jerop
Copy link
Member

jerop commented Sep 9, 2020

/assign

@jerop
Copy link
Member

jerop commented Nov 4, 2020

I prefer the from option as well, and agree with @sbwsg that it shouldn't be a variable but I think we don't even need the dot-notation in from. I explored some alternatives and found that we can solve this using from that references the pipelinetask that we expect to operate on that workspace before the current pipelinetask -- the syntax would look like:

tasks:
    - name: task1
      taskRef:
        name: write-to-workspace
      workspaces:
        - name: output
          workspace: pipeline-ws1
    - name: task2
      taskRef:
        name: read-from-workspace
      workspaces:
        - name: src
          workspace: pipeline-ws1
          from: task1

I implemented a poc for this design in 47d216c -- the example in the poc is a modification of this example (removed the runAfter and used from instead)

@sbwsg @bobcatfish please let me know what you think before i write up a tep on this

@ghost
Copy link

ghost commented Nov 4, 2020

      workspaces:
        - name: src
          workspace: pipeline-ws1
          from: task1

I think this approach makes sense. It simplifies the field to its core purpose of specifying the resource dependency without any of the variable-looking stuff. lgtm!

@bobcatfish
Copy link
Collaborator Author

I also prefer from but I'd like to introduce a couple other thoughts in case they change your opinion @jerop @sbwsg :

  1. We chose not to use from with results <-- slightly different because in that case we NEED variable interpolation and here we don't
  2. The only other place we have from is with pipelineresources - https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#using-the-from-parameter - if we remove pipelineresources then we'd be keeping this "from" syntax ONLY for this workspace feature (maybe thats ok?)

I'm also wondering if we might end up with any potential ambiguity e.g. in the examples above:

      workspaces:
        - name: src
           from: $(tasks.task1.workspaces.output) # note that this is explicitly saying WHICH resource ('output') FROM task1

vs

      workspaces:
        - name: src
          workspace: pipeline-ws1
          from: task1 # this is saying 'pipeline-ws1' from task1, but 'pipeline-ws1' could be bound to multiple workspaces

This might not actually be a problem: i think with pipelineresources it would be because we have an "automagic" volume that we create to share data between pipelineresources when "from" is used, so we need to know exactly which pipelineresource to get the data "from" - with this workspace from feature we aren't (yet???) adding any of this kind of optimization so maybe we don't need that level of detail?

Whether or not this is an issue probably depends on:

  1. Do we see it being useful to have more info (e.g. the exactly workspace binding)
  2. Do we know of any other features we want to add around this (I don't think so?)

@ghost
Copy link

ghost commented Nov 5, 2020

I'm trying to come up with use-cases that could lean on a precise mapping from one pipeline task's workspace to another's but I'm struggling to think of any. The edge in the DAG between the two tasks remains the same regardless of which of the two tasks' WorkspacePipelineTaskBindings are connected by their intent.

@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 Feb 3, 2021
@bobcatfish
Copy link
Collaborator Author

Ooooo yeah this ol' chestnut - I'm going to tentatively add this to the 2021 roadmap draft

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 4, 2021
@bobcatfish bobcatfish added the area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) label Feb 24, 2021
jerop added a commit to jerop/community that referenced this issue Apr 23, 2021
in this PR, we discuss the motivation, goals, non-goals and requirements
for `Workspace` dependencies so that we can align on the problem before
exploring the proposal and alternatives later

today, users cannot specify resource dependencies based on `Workspaces`.
We need to provide a way for users to specify resource dependencies
based on `Workspaces` to ensure that failure and skipping strategies
for common CI/CD use cases work and users don't get unexpected `Pipeline`
failures when we roll out those features

failure and skipping strategies:
- [TEP-0059: Skip Guarded Task Only](https://github.com/tektoncd/community/blob/main/teps/0059-skip-guarded-task-only.md)
- [TEP-0050: Ignore Task Failures](https://github.com/tektoncd/community/blob/main/teps/0050-ignore-task-failures.md)

issue: tektoncd/pipeline#3109
jerop added a commit to jerop/community that referenced this issue Apr 23, 2021
In this PR, we discuss the motivation, goals, non-goals and requirements
for `Workspace` dependencies so that we can align on the problem before
exploring the proposal and alternatives later

Today, users cannot specify resource dependencies based on `Workspaces`.
We need to provide a way for users to specify resource dependencies
based on `Workspaces` to ensure that failure and skipping strategies
for common CI/CD use cases work and users don't get unexpected `Pipeline`
failures when we roll out those features

Failure and skipping strategies:
- [TEP-0059: Skip Guarded Task Only](https://github.com/tektoncd/community/blob/main/teps/0059-skip-guarded-task-only.md)
- [TEP-0050: Ignore Task Failures](https://github.com/tektoncd/community/blob/main/teps/0050-ignore-task-failures.md)

Issue: tektoncd/pipeline#3109
jerop added a commit to jerop/community that referenced this issue Apr 26, 2021
In this PR, we discuss the motivation, goals, non-goals and requirements
for `Workspace` dependencies so that we can align on the problem before
exploring the proposal and alternatives later

`Tasks` can have either resource dependencies or ordering dependencies
between them. Resource dependencies are based on `Results` and
`Workspaces`, while ordering dependencies are defined using `runAfter`
to sequence `Tasks`.

Today, users cannot specify resource dependencies based on `Workspaces`,
that is, a `Task` should execute and use a given `Workspace` before
another `Task` executes and uses the same `Workspace`. We need to
provide a way for users to specify resource dependencies based on
`Workspaces` to ensure that failure and skipping strategies for
common CI/CD use cases work and users don't get unexpected `Pipeline`
failures when we roll out those features.

Failure and skipping strategies:
- [TEP-0059: Skip Guarded Task Only](https://github.com/tektoncd/community/blob/main/teps/0059-skip-guarded-task-only.md)
- [TEP-0050: Ignore Task Failures](https://github.com/tektoncd/community/blob/main/teps/0050-ignore-task-failures.md)

Issue: tektoncd/pipeline#3109
jerop added a commit to jerop/community that referenced this issue Apr 27, 2021
In this PR, we discuss the motivation, goals, non-goals and requirements
for `Workspace` dependencies so that we can align on the problem before
exploring the proposal and alternatives later

`Tasks` can have either resource dependencies or ordering dependencies
between them. Resource dependencies are based on `Results` and
`Workspaces`, while ordering dependencies are defined using `runAfter`
to sequence `Tasks`.

Today, users cannot specify resource dependencies based on `Workspaces`,
that is, a `Task` should execute and use a given `Workspace` before
another `Task` executes and uses the same `Workspace`. We need to
provide a way for users to specify resource dependencies based on
`Workspaces` to ensure that failure and skipping strategies for
common CI/CD use cases work and users don't get unexpected `Pipeline`
failures when we roll out those features.

Failure and skipping strategies:
- [TEP-0059: Skip Guarded Task Only](https://github.com/tektoncd/community/blob/main/teps/0059-skip-guarded-task-only.md)
- [TEP-0050: Ignore Task Failures](https://github.com/tektoncd/community/blob/main/teps/0050-ignore-task-failures.md)

Issue: tektoncd/pipeline#3109
jerop added a commit to jerop/community that referenced this issue May 10, 2021
In this PR, we discuss the motivation, goals, non-goals and requirements
for `Workspace` dependencies so that we can align on the problem before
exploring the proposal and alternatives later

`Tasks` can have either resource dependencies or ordering dependencies
between them. Resource dependencies are based on `Results` and
`Workspaces`, while ordering dependencies are defined using `runAfter`
to sequence `Tasks`.

Today, users cannot specify resource dependencies based on `Workspaces`,
that is, a `Task` should execute and use a given `Workspace` before
another `Task` executes and uses the same `Workspace`. We need to
provide a way for users to specify resource dependencies based on
`Workspaces` to ensure that failure and skipping strategies for
common CI/CD use cases work and users don't get unexpected `Pipeline`
failures when we roll out those features.

Failure and skipping strategies:
- [TEP-0059: Skip Guarded Task Only](https://github.com/tektoncd/community/blob/main/teps/0059-skip-guarded-task-only.md)
- [TEP-0050: Ignore Task Failures](https://github.com/tektoncd/community/blob/main/teps/0050-ignore-task-failures.md)

Issue: tektoncd/pipeline#3109
tekton-robot pushed a commit to tektoncd/community that referenced this issue May 10, 2021
In this PR, we discuss the motivation, goals, non-goals and requirements
for `Workspace` dependencies so that we can align on the problem before
exploring the proposal and alternatives later

`Tasks` can have either resource dependencies or ordering dependencies
between them. Resource dependencies are based on `Results` and
`Workspaces`, while ordering dependencies are defined using `runAfter`
to sequence `Tasks`.

Today, users cannot specify resource dependencies based on `Workspaces`,
that is, a `Task` should execute and use a given `Workspace` before
another `Task` executes and uses the same `Workspace`. We need to
provide a way for users to specify resource dependencies based on
`Workspaces` to ensure that failure and skipping strategies for
common CI/CD use cases work and users don't get unexpected `Pipeline`
failures when we roll out those features.

Failure and skipping strategies:
- [TEP-0059: Skip Guarded Task Only](https://github.com/tektoncd/community/blob/main/teps/0059-skip-guarded-task-only.md)
- [TEP-0050: Ignore Task Failures](https://github.com/tektoncd/community/blob/main/teps/0050-ignore-task-failures.md)

Issue: tektoncd/pipeline#3109
@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 May 25, 2021
@jerop
Copy link
Member

jerop commented May 25, 2021

/remove-lifecycle stale

actively working on this issue in TEP-0063: Workspace Dependencies

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 25, 2021
@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 Oct 15, 2021
@jerop
Copy link
Member

jerop commented Oct 15, 2021

@tekton-robot tekton-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 15, 2021
@jerop jerop removed their assignment Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
Status: Todo
Status: In Progress
Development

No branches or pull requests

3 participants