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

Feature to disable a task in a Pipeline #1797

Closed
chmouel opened this issue Dec 31, 2019 · 29 comments
Closed

Feature to disable a task in a Pipeline #1797

chmouel opened this issue Dec 31, 2019 · 29 comments
Assignees
Labels
kind/design Categorizes issue or PR as related to design. 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

@chmouel
Copy link
Member

chmouel commented Dec 31, 2019

Expected Behavior

It would be nice if it was possible to have a way to disable a task in a Pipline, like so:

disable: bool

to disable a task in a Pipeline be it temporary or for other reasons.

Actual Behavior

Sometime while developing some long Pipeline I have to comment some long running tasks, and being able to just disable them would be nicer.

Additional Info

Maybe that's something that can be achieve with Conditionals? But that seems to be more complexity for a simple directive.

@chmouel chmouel changed the title Ability to disable a task in a Pipeline Feature to disable a task in a Pipeline Dec 31, 2019
@vdemeester
Copy link
Member

@chmouel Yeah, I think it should be possible with Conditionals and a always false condition.

/cc @dibyom
/kind question

@tekton-robot tekton-robot added the kind/question Issues or PRs that are questions around the project or a particular feature label Jan 6, 2020
@dibyom
Copy link
Member

dibyom commented Jan 6, 2020

Yeah an always false should work. Though there are a couple of gotchas:

  1. Tasks that are dependent on that "skipped" task using runAfter or from are also not run.
  2. This could be premature optimization -- but conditionals run in their own pods. Initially we had the idea of some "built-in" conditionals that we can optimize. always-false seems to be a good candidate for this.

@pritidesai
Copy link
Member

  1. Tasks that are dependent on that "skipped" task using runAfter or from are also not run.

I think its a valid behavior even in case of disabling a task with disable: true (if we add such support). If a task is disabled either by conditions or by explicit disable key, all the dependent tasks should not be executed unless otherwise failure strategies are specified (see #1684)

@bobcatfish
Copy link
Collaborator

Being able to disable Tasks via the PipelineRun sounds reasonable to me! Like @pritidesai was saying it should cancel the entire branch though, i.e. anything depending on that Task

The downside of using Conditionals for this as far as I can tell is that you'd have to make the changes in the Pipeline definition itself to toggle on and off, I think? I like the idea of doing it at runtime.

@dibyom
Copy link
Member

dibyom commented Jan 16, 2020

If the field is called disable: bool it'd have to be in the pipeline task not the pipeline run.
If we want to do this at the pipelinerun level, we'd probably want a list of tasks to disable.

The downside of using Conditionals for this as far as I can tell is that you'd have to make the changes in the Pipeline definition itself to toggle on and off, I think? I like the idea of doing it at runtime.

One solution (or workaround rather 😁 ) would be to embed the pipeline spec inside the pipeline run

@bobcatfish bobcatfish added kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. and removed kind/question Issues or PRs that are questions around the project or a particular feature labels Jan 17, 2020
@bobcatfish
Copy link
Collaborator

Maybe this would be something we can add into the PipelineRun?

@jerop
Copy link
Member

jerop commented Jun 1, 2020

/assign

@pritidesai
Copy link
Member

thanks @jerop for taking this up, happy to help if needed 🥰

@jerop
Copy link
Member

jerop commented Jun 1, 2020

thanks @pritidesai 😄

we can collaborate on this design doc I just started

@tekton-robot
Copy link
Collaborator

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

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/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.
Mark the issue as fresh with /remove-lifecycle rotten.

/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.

@tekton-robot tekton-robot added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 14, 2020
@jerop
Copy link
Member

jerop commented Aug 14, 2020

will be solved using when expressions in conditions beta (ongoing work)

@vdemeester
Copy link
Member

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

@tekton-robot
Copy link
Collaborator

@vdemeester: Reopened this issue.

In response to this:

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

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.

@tekton-robot tekton-robot reopened this Aug 17, 2020
@tekton-robot tekton-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 17, 2020
@pritidesai
Copy link
Member

pritidesai commented Sep 9, 2020

This is now possible with When Expressions using Pipeline param, set a pipeline param runMyTask to false and check that variable in the Pipeline Task.

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: pipelinerun-to-skip-task
spec:
  params:
    - name: runMyTask
      value: "false"
  pipelineSpec:
    params:
      - name: runMyTask
        default: "true"
    tasks:
      - name: skip-this-task
        when:
          - input: $(params.runMyTask)
            operator: in
            values: ["true"]
        taskSpec:
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "Good Morning!"
      - name: execute-this-task
        taskSpec:
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "Good Night!"

@bobcatfish
Copy link
Collaborator

hey all! i want to reopen this to discuss further down the line: when expressions will allow you to disable particular tasks via a parameter, however to take advantage of this you need to add the when expressions

imo the feature request here is about getting this functionality out of the box; e.g. even without when expressions or conditionals, commenting out tasks in a pipeline is another way to achieve the same goal, the ask here is to have a way to do this at runtime that doesn't require changing the pipeline definition

@bobcatfish bobcatfish reopened this Oct 14, 2020
@pritidesai
Copy link
Member

have to explore pipelineRun to support it, something like @jerop's proposal on a list of disabled/skipped pipelineTasks

@pritidesai
Copy link
Member

pritidesai commented Oct 15, 2020

another possible way is to introduce status in pipelineRun CRD which can contain a pipelinetask name and desired state (skipped), can extend it in future to include other states.

@bobcatfish
Copy link
Collaborator

The biggest blocker for this one is convincing ourselves the functionality is worth adding - I happen to think it is but some folks don't, which is why we put this on hold. I want to revisit it at some point but understand why folks are reluctant to add it - it might make more sense for post v1 unless we have some strong use cases now (was talking to @coryrc about a potential use case with prow also)

@vdemeester
Copy link
Member

Naive question, does the following work ?

        when:
          - input: false
            operator: in
            values: ["true"]

Coupled with the continueOnSkip feature, this would be the way to skip temporarily a task in a pipeline. It's not a one-liner, but it would exist.

Anyway, I would like to gather samples on uses cases for this 👼

@bobcatfish
Copy link
Collaborator

@vdemeester that would work - but again it would mean changing the Pipeline spec itself, and for the use cases I'm imagining (e.g. say a pipeline failed and you want to rerun just part of it, or @coryrc's case where you have one pipeline with several branches but you want to run them separately, or you're developing the pipeline and want to run only part of it as you work on it, or you're debugging a faulty pipeline) id really like to see a feature where you can do this at runtime without having to change the pipeline definition

@vdemeester
Copy link
Member

@bobcatfish so the use case is then to "disable" a Task in a Pipeline at runtime (from a PipelineRun) — which is a bit different from the initial request.

@bobcatfish
Copy link
Collaborator

@vdemeester exactly! maybe it would make more sense to create a new issue and close this one?

@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 Jan 21, 2021
@bobcatfish
Copy link
Collaborator

hmmm i dont want to lose this, but the last discussion we had was to replace it with a new issue so maybe it's ok to leave as is :D

@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 Mar 4, 2021
@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
kind/design Categorizes issue or PR as related to design. 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

7 participants