-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Storing and updating PipelineSpec #3145
Comments
Some context:
I think the motivation for storing this at all is to be able to tell exactly what spec the pipeline was run with, since pipelines can be modified later
probably depends on why we're storing this information - i think it's less about being easy to see what happened and more about a historical record. if we mutated it before storing it, then the spec in this section would differ between runs |
Yup agree, it depends on how it's being consumed. The spec is populated right after reading it during the first reconciler cycle and not updated based on the informer's copy. But if there is no objection, the spec can be populated after resolving all params and pipeline resources during the first reconciler cycle so that it does not differ between runs. This also means that spec will not be available or populated if it fails any of the validation check (invalid DAG, invalid syntax, invalid resource bindings, invalid params, etc) unlike today. |
Duplicate in: #3140 |
@jerop: Closing this issue. In response to this:
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. |
"I was having trouble seeing unresolved when expressions in
status.pipelineSpec.tasks[]
and was trying to troubleshoot and discovered we actually store thepipelineSpec
as is even before resolving params and it's never updated for auditing purpose. But wouldn't it be clearer if its updated once after resolving all params (and pipeline resources)?" ~ @pritidesaiOriginally posted by @pritidesai in #3135 (comment)
The text was updated successfully, but these errors were encountered: