-
Notifications
You must be signed in to change notification settings - Fork 220
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
TEP-0156 : when-in-steps #1147
base: main
Are you sure you want to change the base?
TEP-0156 : when-in-steps #1147
Conversation
42b9464
to
f841b49
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/kind tep
/kind tep |
/assign |
teps/0155-whenexpressions-in-step.md
Outdated
|
||
### Supported Expression Languages | ||
|
||
Just like when expressions in Task Level, when expressions in Step Level will also be supporting both Operator-based expressions and CEL(Common Expression Language) expressions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be also add a paragraph on which fields are we applying when expressions on?
- all task based fields(e.g. Task Results, Task Conditions etc.)
- Workspaces
- Params
- StepResults
- StepStatus(?):
TerminationReasonFields: Error, Continued, Skipped, TimeOut, Cancelled etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a Variable Substitution
section to explains which fields can be referenced in step.when
.
StepStatus(?)
It seems that we haven't StepStatus implemented yet, should we include in this tep? This seems a different scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok keeping it out for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, we should include some way to allow conditional execution on step status. Otherwise usage of When expression in guarding a step
will be limited. I think one major advantage of a When expression guarding a step is to check the status of preview steps. Otherwise we are forcing the user to "mimic" this functionality by exporting a pseudo status of step to workspace/result to mimic this functionality.
Secondly, is it possible to check attributes of files in a workspace in When expressions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @haroonc
Thanks for the review! I've added step status variable substitution when evaluating When expressions in the design.
For checking file attributes, as far as i know Tekton doesn't support that directly. But, you can use a script in a step to get attributes and store them as results. Then, use those results in When expressions in later steps.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chitrangpatel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@ericzzzzzzz can you please leave a short description of the TEP in the opening comment? |
@tektoncd/core-maintainers PTAL 🙏 |
done! Thanks |
/assign |
7ca83ad
to
7b6f7d5
Compare
/assign @vdemeester |
- End-to-end tests covering common use cases and error conditions. | ||
|
||
## Alternatives | ||
An alternative approach would be to evaluate When Expressions before creating the pods for the steps. If a When Expression evaluates to false, the corresponding step container would not be created at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant creating step containers inside the pods for the Task
. There are no pods for containers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant tekton can evaluate 'when expressions' when building pod definition, if the evaluation result is false, then we don't have to add the corresponding step container to the pod 'spec.containers'
## Motivation | ||
Currently, Tekton Tasks execute steps sequentially without the ability to control their execution based on conditions within the Task itself. This limitation restricts the ability to create dynamic workflows that adapt to different scenarios or respond to the outcomes of previous steps. | ||
By introducing When Expressions to Steps, users will be able to: | ||
- **Create conditional workflows**: Execute steps only when specific conditions are met, such as the success or failure of a previous step, the value of a result, or the presence of a specific parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related with this: the new v2alpha4 formatter from Tekton Chains now is processing all the type-hinted results from StepResults associated with a given PipelineRun or TaskRun. With the new WhenExpressions in Steps
feature, I'm guessing we'll need to do a change in Chains to process only the executed steps, and not all of them; processing not executed steps might result in an error (?). With that said I think we could open a new issue in Chains once this TEP is approved to track the needed effort there. @chitrangpatel @ericzzzzzzz please feel free to correct me if I'm wrong with my previous thoughts. Thanks!
@vdemeester PTAL when you have a chance 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lines between Task
and Pipeline
are bluring more and more with this.
An additionnal question, what happens if a step that write a TaskResult
is skipped ?
|
||
## Summary | ||
This TEP proposes the addition of When Expressions to individual Steps within a Task. This will allow for fine-grained control over the execution of steps based on conditions like the status of previous step results. This enhancement will provide greater flexibility and control in defining task logic, enabling more complex and dynamic workflows. | ||
## Motivation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anything described here can be achieved by "relatively" smart scripts in Steps, am I right ?
The "main" difference is in one case it's expressed with the tekton API, and on the other case, it's in "a script" or "in code".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only for inlined steps. This allows us to control the execution of stepActions where we don't have the ability to edit the scripts.
I think that's why it wasn't needed when we only had inlined steps. It is needed now that we have StepActions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I can hear that 😛 But one could also "condition" this in the StepAction
with a parameter (probably a bit convoluted)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a valid point, however, one can make same argument about when
expression in Tasks 😉.
IMHO, declarative conditionals in a CI/CD pipeline are more readable and improve maintainability of the pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@haroonc on the other hand, it make the "split" between local build and CI/CD bigger 😛 but that's a completely different topic/debate 👼🏼
I think the core difference between the fact that Pipeline can fan-out (Matrix, DAG) vs Task is sequential will always remain. So it might appear that lines are being blurred but what you can do with Pipelines vs Tasks will always be different. If Tasks was a DAG of Steps then I agree that the lines would be truly blurred. I think the outcome will be the same as if the Step producing a TaskResult errored out. The Task author is controlling the flow of the StepAction/Step that is producing the result so they can choose. In this case, I think that the Task will error with the message that it could not produce a result (whatever the current behavior is if the Task does not generate a result)? |
Can we discuss this in the TEP ? |
I added a section to capture this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, one more thing, can we dig a bit more into examples of use case it would fullfill ?
For example
- having "optional" set of caching steps (using StepAction) if a parameter is the
Task
- having a "report" and fail step if the previous step fails
- … other use case we envision with this feature.
Hi, I added some examples. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the example 👍🏼 I have a few more comments / questions 🙏🏼
|
||
### When Expressions Evaluation | ||
|
||
When expressions are evaluated at the entrypointer level. This allows us to use step results produced from previous steps to determine if a step should be skipped. To indicate a skipped step, the exit code will be 0 (showing a step is complete) and the TerminationReason will be Skipped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we forsee any impact on the entrypoint "performance" ? The entrypoint is doing more and more work (with step results, …) and I think we never measured or "thought" about possible performance impact (and if they are noticeable or not) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, adding Step-level When Expressions will have some cost, but it's important to consider the value it brings. This cost is only incurred when Step-level When Expressions are actually configured. In comparison to the benefits of having this finer-grained control over workflow and the potential to skip long-running steps for performance optimization, the cost is relatively minimal. Even Task-level When Expressions also have a performance cost, and the Step-level implementation offers a significantly greater potential for performance gains through more flexible workflow control.
teps/0156-whenexpressions-in-step.md
Outdated
echo "reporting timeout!" | ||
``` | ||
|
||
Here, the `report-timeout` step will only run if the `main-task` step terminates with a "TimeoutExceeded" status. This allows you to specifically handle failures without executing the reporting logic every time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, the Task
would still be marked as a failure, isn't it ?
If yes, is there a case where we wouldn't want that behavior ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One potential use case could be a task that performs multiple integration test via separate stepActions for each integration service. We may want to file a bug report for each integration failure, mainly because those will be investigate by different teams. Report filing might be another step Action (e.g. jira-open-bug). We can declaratively control bug filing for each integration run via when
expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just changed this example a little bit, it turns out with the existing implementation, timeout is a error that will always fail subsequent steps, https://github.com/tektoncd/pipeline/blob/9c315909a373a600eb42f343584badeea818607c/pkg/entrypoint/entrypointer.go#L255-L257 .
Task authors should use "onError:continue" on the steps that could potentially fail to allow the follow-up steps to execute, authors can decide if the task should fail on the step handling the error.
By introducing When Expressions to Steps, users will be able to: | ||
- **Create conditional workflows**: Execute steps only when specific conditions are met, such as the success or failure of a previous step, the value of a result, or the presence of a specific parameter. | ||
- **Improve error handling**: Implement robust error handling strategies by executing specific steps only when errors occur in previous steps. | ||
- **Enhance reusability**: Develop more versatile Tasks that can adapt their behavior based on different input parameters or runtime conditions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, This feature will also enhance usability of StepActions. This will enable authors of StepActions to write more generic StepActions and users will be able to control invocation of these steps based on the execution state.
- Task-based fields: | ||
- Task Results: You can access the results of any previously executed Task within the same TaskRun using the syntax $(tasks.<taskName>.results.<resultName>). | ||
- Workspaces: You can reference the names of workspaces declared in the Task using the syntax $(workspaces.<workspaceName>.bound). This allows you to check if a workspace is bound and make decisions based on its availability. | ||
- Params: Access input parameters defined in the Task using the familiar syntax $(params.<paramName>). This enables conditional logic based on the values passed into the Task. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will it also support parameters defined in the Pipeline (and implicitly propagated to the current task) or previously executed Steps?
teps/0156-whenexpressions-in-step.md
Outdated
echo "reporting timeout!" | ||
``` | ||
|
||
Here, the `report-timeout` step will only run if the `main-task` step terminates with a "TimeoutExceeded" status. This allows you to specifically handle failures without executing the reporting logic every time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One potential use case could be a task that performs multiple integration test via separate stepActions for each integration service. We may want to file a bug report for each integration failure, mainly because those will be investigate by different teams. Report filing might be another step Action (e.g. jira-open-bug). We can declaratively control bug filing for each integration run via when
expression.
|
||
## Summary | ||
This TEP proposes the addition of When Expressions to individual Steps within a Task. This will allow for fine-grained control over the execution of steps based on conditions like the status of previous step results. This enhancement will provide greater flexibility and control in defining task logic, enabling more complex and dynamic workflows. | ||
## Motivation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a valid point, however, one can make same argument about when
expression in Tasks 😉.
IMHO, declarative conditionals in a CI/CD pipeline are more readable and improve maintainability of the pipeline.
Description
This pull request proposes the ability to define When Expressions directly within Steps. This allows for fine-grained control over step execution based on conditions like the status of previous steps, enabling more dynamic and adaptable workflows.
Overview