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

Skipping Reason #4738

Closed
jerop opened this issue Apr 6, 2022 · 6 comments
Closed

Skipping Reason #4738

jerop opened this issue Apr 6, 2022 · 6 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@jerop
Copy link
Member

jerop commented Apr 6, 2022

Feature Request

Surface the reason why a specific Task in a Pipeline was skipped.

Today, users only know that a PipelineTask was skipped, but they don't know for which exact reason. When users see a PipelineTask has been skipped without knowing the reason, they may get confused about the behavior of the PipelineRun e.g. #4571 and slack thread.

There are many reasons why a PipelineTask could be skipped, including:

  • at least one of its when expressions evaluated to false
  • at least one of its Conditions failed
  • at least one of the Results it was consuming was missing
  • at least one its parent PipelineTasks was skipped
  • the PipelineRun was in stopping state
  • the PipelineRun was gracefully cancelled
  • the PipelineRun was gracefully stopped

Use case

As a Pipeline user, I need to know why certain Tasks in my Pipeline have been skipped.

@jerop jerop added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 6, 2022
@dibyom dibyom added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Apr 11, 2022
@chitrangpatel
Copy link
Contributor

/assign

@chitrangpatel
Copy link
Contributor

chitrangpatel commented Apr 27, 2022

Hi, I'm working on this issue in this PR. At this point, all the unit tests pass and after adding the Reason field where necessary.
In addition, monitoring the status of the pipeline run using the following command:
kubectl get pipelinerun pipelinerun-skip-task-run-task -o yaml results in:

status:
  completionTime: "2022-04-27T14:31:37Z"
  conditions:
  - lastTransitionTime: "2022-04-27T14:31:37Z"
    message: 'Tasks Completed: 1 (Failed: 0, Cancelled 0), Skipped: 1'
    reason: Completed
    status: "True"
    type: Succeeded
  ...
  skippedTasks:
  - name: skip-this-task
    reason: WhenExpressionsSkip
    whenExpressions:
    - input: foo
      operator: in
      values:
      - foo1
  ...

Under skippedTasks, there is now a reason: WhenExpressionsSkip as expected showing the reason why the test was skipped.

@chitrangpatel
Copy link
Contributor

chitrangpatel commented Apr 27, 2022

Currently, when we use the tkn CLI as shown below, Skipped Tasks only displays the NAME. Would it also be useful to show the REASON for skipping the task?

chitrang-macbookpro:cli chitrang$ ./tkn pipelinerun describe pipelinerun-skip-task-run-task
Name:              pipelinerun-skip-task-run-task
Namespace:         default
Service Account:   default
Timeout:           1h0m0s
Labels:
 tekton.dev/pipeline=pipelinerun-skip-task-run-task

🌡️  Status

STARTED          DURATION   STATUS
52 minutes ago   7s         Succeeded(Completed)

🗂  Taskruns

 NAME                                             TASK NAME       STARTED          DURATION   STATUS
 ∙ pipelinerun-skip-task-run-task-run-this-task   run-this-task   52 minutes ago   7s         Succeeded

⏭️  Skipped Tasks

 NAME              
 ∙ skip-this-task  

@jerop
Copy link
Member Author

jerop commented May 2, 2022

@chitrangpatel - yes, showing Reason in the CLI will be useful - we can ask the CLI team to take a look after the feature is released (cc @piyush-garg @vinamra28 @vdemeester)

@lbernick
Copy link
Member

lbernick commented May 9, 2022

/close

this feature has been implemented in #4829. If we want to add this functionality to the CLI let's create an issue in that repo instead of tracking here.

@tekton-robot
Copy link
Collaborator

@lbernick: Closing this issue.

In response to this:

/close

this feature has been implemented in #4829. If we want to add this functionality to the CLI let's create an issue in that repo instead of tracking here.

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/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants