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.

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

[1/2] OLM workflow: add automatic OLM bundle submission. #460

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

fmuyassarov
Copy link
Collaborator

@fmuyassarov fmuyassarov force-pushed the automate-olm-releasing branch 3 times, most recently from 73504ed to 8d428bb Compare January 7, 2025 09:11
@fmuyassarov fmuyassarov marked this pull request as ready for review January 7, 2025 09:12
@fmuyassarov fmuyassarov requested a review from klihub January 7, 2025 13:04
@klihub klihub requested review from kad and marquiz January 10, 2025 07:51
@klihub
Copy link
Collaborator

klihub commented Jan 10, 2025

The other remaining thing is what we discussed offline today: adding a secondary trigger (maybe a push to a branch or a new tag matching a pattern) which would trigger the workload but with an eventual PR filed against a test repository (which probably should be the clone of the real community operators repo under the bot's account).

@fmuyassarov fmuyassarov force-pushed the automate-olm-releasing branch from 8d428bb to c11fb4f Compare January 13, 2025 10:22
@fmuyassarov fmuyassarov marked this pull request as draft January 13, 2025 10:22
@fmuyassarov fmuyassarov force-pushed the automate-olm-releasing branch from c11fb4f to aa9226e Compare January 14, 2025 09:02
@fmuyassarov
Copy link
Collaborator Author

The other remaining thing is what we discussed offline today: adding a secondary trigger (maybe a push to a branch or a new tag matching a pattern) which would trigger the workload but with an eventual PR filed against a test repository (which probably should be the clone of the real community operators repo under the bot's account).

It won't be that easy to test even with extra conditional trigger. Because, I don't have permissions to push anything that could result in the workflow to start. However, I have tested the same workflow file with only one difference being the source repo set as fmuyassarov/nri-plugins (my fork) instead of containers/nri-plugins. And the result you can see here: k8s-operatorhub/community-operators#5518.

@fmuyassarov fmuyassarov marked this pull request as ready for review January 14, 2025 09:05
@fmuyassarov fmuyassarov force-pushed the automate-olm-releasing branch 2 times, most recently from acb63df to d45d62b Compare January 14, 2025 19:13
@fmuyassarov fmuyassarov requested a review from klihub January 15, 2025 07:28
@fmuyassarov fmuyassarov force-pushed the automate-olm-releasing branch from d45d62b to 8388c4e Compare January 15, 2025 08:52
@fmuyassarov fmuyassarov requested a review from klihub January 15, 2025 09:02
@kad
Copy link
Collaborator

kad commented Jan 15, 2025

Generally looks ok, just need a cleanup of all debugging bits, and hardcoded versions to make it mergable.
Second thing to invistigate: don't use primary gpg private key, it should be enough to store in secrets just a signing sub-key.

@fmuyassarov
Copy link
Collaborator Author

Generally looks ok, just need a cleanup of all debugging bits, and hardcoded versions to make it mergable.

Those are added intentionally for the testing purpose. The idea is that we merge it as it is now and create an issue to test the workflow and if it behaves as it should there will be a follow up PR to clean up testing bits.

@fmuyassarov
Copy link
Collaborator Author

Generally looks ok, just need a cleanup of all debugging bits, and hardcoded versions to make it mergable.

Those are added intentionally for the testing purpose. The idea is that we merge it as it is now and create an issue to test the workflow and if it behaves as it should there will be a follow up PR to clean up testing bits.

@kad that's what we discussed with @klihub offline.

@fmuyassarov fmuyassarov force-pushed the automate-olm-releasing branch from 8388c4e to 8969a2a Compare January 16, 2025 11:06
@fmuyassarov fmuyassarov requested a review from klihub January 16, 2025 11:13
@klihub klihub requested a review from askervin January 17, 2025 06:57
@askervin
Copy link
Collaborator

I think this is great work. Thank you @fmuyassarov for automating these pull requests!

I don't want to request big changes before merging the first version. Having it tested without hardcoded versions would be enough for me.

Two questions, mainly on future steps:

Testability: Would it be convenient to enable testing this workflow so that it would recognize when it is running on a nri-plugins repo fork (instead of owned by containers), and in that case it would create the pull request to the community-operators repo fork of the same owner instead of upstream? And perhaps this workflow could be triggered by a pull_request_comment like "/test-operator-release" or something (that would obviously never create pull request to the upstream repo).

Re-releasing: do we ever want to create a pull request that changes an existing release? If not, maybe we should then explicitly fail on the "Copy the bundle..." step if the version directory already exists.

Copy link
Collaborator

@klihub klihub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fmuyassarov @askervin My suggestion would be to merge this and test-drive it, so Feruz can get rid of it.

Then we can beat it into final shape with one or more new PRs as necessary. WDYT ?

@askervin
Copy link
Collaborator

Tried to play with this a bit, and it's quite broken.

@klihub, this looks necessary: afd0c37

Maybe we should coordinate merging so that it'll get in very soon after this one.

@klihub
Copy link
Collaborator

klihub commented Jan 21, 2025

Tried to play with this a bit, and it's quite broken.

@klihub, this looks necessary: afd0c37

Maybe we should coordinate merging so that it'll get in very soon after this one.

@askervin I have updated that slightly, klihub@5b80b67, and have it on this devel/olm-bundle-submission branch. If a test trigger is used, it now files the PR against the fork of the user who opened (or reopened) the test trigger issue. Also, if filing the PR succeeded, it closes the test trigger issue in the end.

If we decide to merge this, I can open immediately a new PR with those additional commits. An alternative would be to replace this with a new PR with the additional commits included and merge that one in a single go.

@klihub klihub changed the title workflow: add a workflow to automate OLM bundle submission [1/2] OLM workflow: add automatic OLM bundle submission. Jan 21, 2025
Copy link
Collaborator

@askervin askervin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be merged when we are ready to merge #464 right after it.

@fmuyassarov fmuyassarov force-pushed the automate-olm-releasing branch from 8969a2a to cd645bc Compare January 21, 2025 12:24
@klihub klihub merged commit 295e82e into containers:main Jan 22, 2025
8 checks passed
@fmuyassarov fmuyassarov deleted the automate-olm-releasing branch January 22, 2025 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants