-
Notifications
You must be signed in to change notification settings - Fork 24
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
ROX-20232: Reduce duplication in Collector Konflux pipelines #1676
ROX-20232: Reduce duplication in Collector Konflux pipelines #1676
Conversation
0316110
to
e8dd121
Compare
/retest |
e8dd121
to
46e03ef
Compare
Just used an IDE command, no other manual changes.
and use it everywhere.
Only used IDE command, no manual changes on top.
46e03ef
to
7ebd9e4
Compare
/retest |
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.
Full disclosure, I understand very little of what's going on in this PR, so this is a "blind trust LGTM"
- name: TARGET_STAGE | ||
value: $(params.build-target-stage) | ||
runAfter: | ||
- prefetch-dependencies |
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.
This seems to be slightly different from the original version, where prefetch-dependencies
was run after prepare-rhel-rpm-subscriptions
. Just out of curiosity, is the order here important?
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 order matters in general but not here.
Both prefetch-dependencies
and prepare-rhel-rpm-subscriptions
(as well as earlier clone-repository
and determine-image-tag
) are prerequisites for build-container
. They were put sequentially because, well, we could. They can also be put parallel because they don't have a dependency on each other.
In fact, in .tekton/collector-(pull-request|push).yaml
they were already in parallel and this caused no issue.
.tekton/collector-slim-(pull-request|push).yaml
were simply not made consistent.
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.
BTW, getting rid of these discrepancies was a side-goal of the task.
/retest |
1 similar comment
/retest |
Finally, green enough Konflux CI. Merging. |
Description
Follows the pattern set in stackrox/stackrox#10866.
YAML formatting applied from stackrox/scanner#1515.
It's best to review this change by commits where I tried supplying meaningful messages.
Checklist
[ ] Updated documentation accordinglyAutomated testing
[ ] Added unit tests[ ] Added integration tests[ ] Added regression testsIf any of these don't apply, please comment below.Testing Performed
Only looking at CI.