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.

Sign up for GitHub

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

Jump to bottom

ci: prevent breakage when including certain commands in the PR #4751

Merged
merged 2 commits into from
Aug 9, 2024
Merged

ci: prevent breakage when including certain commands in the PR #4751

merged 2 commits into from
Aug 9, 2024

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Aug 7, 2024 •
edited
Loading

[paragraph removed due to CI blockage, see the commit description]

This causes Mergify to add the ok-to-test label again, instructing a
GitHub Action to add comments to start jobs in Jenkins. Once all
comments have been posted, the ok-to-test label is removed. Mergify
then notices that the jobs were not run, and adds the ok-to-test label
again.... Endlessly looping of adding the label, commenting and removing
the label as a result.

By reporting the brokenness of the PR description and marking the PR as
Draft, the looping is prevented.

See-also: #4749
skip-checks: false

@nixpanic
ci: detect [skip ci] in PR descriptions and prevent breakage
b40a6c0 
When Jenkins jobs are started by a comment, and if the PR contains
`[skip ci]` in the description, Jenkins does not run the requested job,
nor set a status for the job in the PR.

This causes Mergify to add the `ok-to-test` label again, instructing a
GitHub Action to add comments to start jobs in Jenkins. Once all
comments have been posted, the `ok-to-test` label is removed. Mergify
then notices that the jobs were not run, and adds the `ok-to-test` label
again.... Endlessly looping of adding the label, commenting and removing
the label as a result.

By reporting the brokenness of the PR description and marking the PR as
Draft, the looping is prevented.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
@mergify mergify bot added the component/testing Additional test cases or CI work label Aug 7, 2024
@nixpanic nixpanic marked this pull request as draft August 7, 2024 08:00
@nixpanic nixpanic added ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures labels Aug 7, 2024
@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

/test ci/centos/k8s-e2e-external-storage/1.28

@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

/test ci/centos/k8s-e2e-external-storage/1.28

In the Jenkins logs, this is reported:

Pull request #4,751 was updated/initialized on ceph/ceph-csi at 8/7/24, 8:02 AM by Niels de Vos (comment)
Aug 07, 2024 8:02:14 AM INFO org.jenkinsci.plugins.ghprb.GhprbPullRequest checkSkipBuild
Pull request commented with .*\[skip\W+ci\].* skipBuildPhrase. Hence skipping the build.

@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

/test ci/centos/k8s-e2e-external-storage/1.28

@nixpanic nixpanic changed the title ci: detect [skip ci] in PR descriptions and prevent breakage ci: prevent breakage when including certain commands in the PR Aug 7, 2024
@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

/test ci/centos/k8s-e2e-external-storage/1.28

Possibly even in the subject of the PR, the [skip ci] command prevents running the job.

@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

/test ci/centos/k8s-e2e-external-storage/1.28

@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

/test ci/centos/k8s-e2e-external-storage/1.28

Yay, this now works!

@nixpanic nixpanic marked this pull request as ready for review August 7, 2024 08:08
@nixpanic nixpanic requested review from Madhu-1 and a team August 7, 2024 08:08
Madhu-1
Madhu-1 previously approved these changes Aug 7, 2024
View reviewed changes
@Madhu-1 Madhu-1 requested a review from a team August 7, 2024 08:11
Rakshith-R
Rakshith-R previously approved these changes Aug 7, 2024
View reviewed changes
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Aug 7, 2024

@Mergifyio queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 7, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The pull request #4751 has been manually updated.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

something is weird, all CI jobs seem to be missing in the status?

@nixpanic
Copy link
Member Author

nixpanic commented Aug 7, 2024

Ah, of course I mentioned [skip ci] in the commit message, and therefor GitHub does not want to run the jobs either: https://docs.github.com/en/actions/managing-workflow-runs-and-deployments/managing-workflow-runs/skipping-workflow-runs

@nixpanic
doc: add description of Mergify commands for merging a PR
f92edd6 
Signed-off-by: Niels de Vos <ndevos@ibm.com>
@mergify mergify bot dismissed stale reviews from Rakshith-R and Madhu-1 August 7, 2024 12:00

Pull request has been modified.

@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Aug 7, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Aug 7, 2024
@nixpanic nixpanic requested a review from a team August 8, 2024 07:41
@nixpanic
Copy link
Member Author

nixpanic commented Aug 8, 2024

@Mergifyio refresh

@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 8, 2024

refresh

✅ Pull request refreshed

@mergify mergify bot merged commit d06497c into ceph:devel Aug 9, 2024
41 checks passed
@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 9, 2024

The PR description contains the unsupported [skip ci] command, please update the description and mark the PR ready for review again.

nixpanic added a commit to nixpanic/ceph-csi that referenced this pull request Aug 9, 2024
@nixpanic
ci: prevent incorrect pattern matching for skipping ci command
67fe494 
Mergify matches `body-raw` as regular expression, but that makes it
difficult (if not impossible) to match the `[` in a string.

Fixes: ceph#4751
Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic nixpanic mentioned this pull request Aug 9, 2024
Merged
mergify bot pushed a commit that referenced this pull request Aug 9, 2024
@nixpanic @mergify
ci: prevent incorrect pattern matching for skipping ci command
2fd9257 
Mergify matches `body-raw` as regular expression, but that makes it
difficult (if not impossible) to match the `[` in a string.

Fixes: #4751
Signed-off-by: Niels de Vos <ndevos@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rakshith-R Rakshith-R Rakshith-R approved these changes

@Madhu-1 Madhu-1 Madhu-1 approved these changes

Assignees
No one assigned
Labels
ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures component/testing Additional test cases or CI work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nixpanic @Madhu-1 @ceph-csi-bot @Rakshith-R