-
Notifications
You must be signed in to change notification settings - Fork 42
CI: separate JJBB definitions for pull requests, PR merge and external e2e calls #3253
Conversation
This pull request does not have a backport label. Could you fix it @pazone? 🙏
|
there is no need to backport the |
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.
What's the reason for using two new MBPs rather than the traditional MBP with the same pipeline for branches and PRs?
@v1v To have separate Jenkinsfiles. Can we use one jjb for multiple pipeline scripts? |
We need a pipeline job per branch and PR so MBP fit the use case. Also, the MBP creates the jobs on GitHub events and registers the triggers, with regular pipelines in case of reprovision of the instance we will lose the triggers and we have to manually trigger the jobs. It is not the real reason why @pazone did it, but it is a good reason to do it for jobs that have triggers. |
I don't understand the reason yet for two different Multibranch pipelines, one related to the PRs and another one for the branches. I understand there is no need to use a As far as I see, the proposal for PRs and branches could be merged in the same multibranch pipeline, aka MBP, as we normally do in other places. Can you please elaborate what's the difference between the |
@v1v pull request flow will be a bit more complicated. |
If no ansible stuff is changed, what's the flow in place for both cases (prs and branches)? IIUC, those are not independent pipelines but stages, will something like the below pseudo-code could work?
I somehow prefer to use the same pipeline for branches and PRs. Maybe there is a need to:
And the pipeline ansible stuff will trigger the non-ansible stuff so it can test other things given the AMIs. I'm just guessing, as I don't know the implementation details about the pipeline specifics. In any case, I'm not blocking this PR, but commenting about this |
.ci/jobs/e2e-testing-mbp.yml
Outdated
branch-discovery: no-pr | ||
head-filter-regex: '(main|PR-.*|8\.\d+|7\.17|feature-.*)' | ||
head-filter-regex: '(main|8\.\d+|7\.17|feature-.*)' |
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 changes the current behavior on the stable branch without a change in the pipelines, this breaks the CI. the PR will not be processed until all new pipelines are develop.
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.
A proposal here, do not touch the main pipeline (.ci/jobs/e2e-testing-mbp.yml) until everything is working. So, create a new file like this one with a different job.name
, and use it until you have a process refined and working. For that, you will have to limit the head-filter-regex
to a specific branch(where you are working) and PR (only your test PRs). then we can see which changes we can incorporate on the main branch without issues.
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.
WYT @v1v
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.
@kuisathaverat Great solution! Could you please take a look 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.
we still making changes in .ci/jobs/e2e-testing-mbp.yml that is something we do not want
@@ -9,7 +9,7 @@ | |||
scm: | |||
- github: | |||
branch-discovery: no-pr | |||
head-filter-regex: '(main|8\.\d|7\.17|feature-.*)' | |||
head-filter-regex: '(main|PR-.*|8\.\d|7\.17|feature-.*)' |
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.
no here
.ci/jobs/e2e-testing-mbp.yml
Outdated
@@ -1,8 +1,8 @@ | |||
--- |
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.
create a new job from scratch
.ci/jobs/e2e-testing-mbp.yml
Outdated
branch-discovery: no-pr | ||
head-filter-regex: '(main|PR-.*|8\.\d+|7\.17|feature-.*)' | ||
head-filter-regex: '(main|8\.\d+|7\.17|feature-.*)' |
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.
limit the branches to your working branches and PRs
💔 Build Failed
Expand to view the summary
Build stats
Pipeline error
❕ Flaky test reportNo test was executed to be analysed. 🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
What does this PR do?
Creates 3 separate JJBB workflows:
e2e-testing-mbp.yml
- For external e2e tests executions ONLY. When another pipeline triggers the testse2e-testing-pr-mbp.yml
- For pull requests to this repo ONLYe2e-testing-pr-merge-mbp.yml
- For PR merges tomain
,8.x
or7.17
Why is it important?
Single Jenkinsfile for all operations is overcomplicated
Related issues