Add Periodic MetalLB Bump workflow #341

liornoy · 2023-04-03T14:06:00Z

This commit adds a new workflow to periodically align
with the upstream MetalLB repository.

This is setup to run daily at 7AM and 7PM (GMT+2 clock),
run the make bump_metallb and if differences exist,
then open a PR with the bump,
and on the commit message added details of the old commit,
and the new commit, just so it will be easier
for developers to maintain and keep track of changes.

Created metallb_ref.txt as the single source of truth for
the metallb commit hash.

Also deleted the make test from the bump_metallb, because we want
the workflow not to fail and to open PR even if the unit tests are
failing, so we could catch and fix that.

Notice: Every time this workflow runs it fetches latest metallb
and force-push to already open PR under the "bumpmetallb" branch.

Signed-off-by: liornoy lnoy@redhat.com

fedepaol · 2023-04-05T10:00:08Z

Makefile

@@ -256,7 +256,6 @@ bump_metallb: ## Bumps metallb commit ID and creates manifests. It also validate
 	hack/bump_metallb.sh
 	$(MAKE) bin
 	$(MAKE) bundle-release
-	$(MAKE) test


why this was removed?

"Also deleted the make test from the bump_metallb, because we want the workflow not to fail and to open PR even if the unit tests are failing, so we could catch and fix that."

fedepaol · 2023-04-05T10:01:57Z

.github/workflows/bump_metallb.yml

+          commit-message: |
+            Bump MetalLB
+
+            This commit bumps MetalLB from `${{ steps.old-commit-info.outputs.commit_sha }}` to `${{ steps.new-commit-info.outputs.commit_sha }}`.


Is there a way to close any existing already opened pr?
Thinking about a scenario where we bump metallb multiple times but we wait to bump the operator.
Ideally, only the most recent one should be open

Yes, that could be nice. Need to figure out how to query the open PR and check if it should be kept or closed.

@fedepaol the workflow will periodically override old PRs with the latest metallb. so I think it could work for us.

mlguerrero12 · 2023-04-19T16:00:30Z

please remove the .idea/workspace.xml file. It'd be nice if we add it to .gitignore

liornoy · 2023-04-19T16:07:07Z

please remove the .idea/workspace.xml file. It'd be nice if we add it to .gitignore

sure will do.

liornoy · 2023-04-19T16:52:01Z

@fedepaol I pushed my working workflow to here, running twice a day.
regarding your comment about closing existing pr and having only the latest -
I found out that this workflow is going to force-push to an existing PR, so it will
update to the latest on its own.

BUT for this to work we must create a dedicated personal auth token for metallb
with the permission for "workflows" - and that is required because during the bump we
update the metallb_e2e.yml workflow and that requires a non-default token.
I named the secret "PAT" here, so you'll need to create that secret with this name holding the token
with the "workflows" permissions.

liornoy · 2023-04-23T11:28:24Z

Also, note that I count on us being able to checkout into the automated sync PR and resolve conflicts if need with force-push.

fedepaol · 2023-05-08T08:49:53Z

@fedepaol I pushed my working workflow to here, running twice a day. regarding your comment about closing existing pr and having only the latest - I found out that this workflow is going to force-push to an existing PR, so it will update to the latest on its own.

BUT for this to work we must create a dedicated personal auth token for metallb with the permission for "workflows" - and that is required because during the bump we update the metallb_e2e.yml workflow and that requires a non-default token. I named the secret "PAT" here, so you'll need to create that secret with this name holding the token with the "workflows" permissions.

Sorry for the delay. There's one thing that I am maybe missing. This won't bump anything, it will just file a pr. So, why the token is required? And if it is required because we change it on a local branch, can we change the workflow so it will read from a file (which we can treat as the source of truth) so we can avoid the extra permission?

liornoy · 2023-05-08T09:30:30Z

@fedepaol I pushed my working workflow to here, running twice a day. regarding your comment about closing existing pr and having only the latest - I found out that this workflow is going to force-push to an existing PR, so it will update to the latest on its own.
BUT for this to work we must create a dedicated personal auth token for metallb with the permission for "workflows" - and that is required because during the bump we update the metallb_e2e.yml workflow and that requires a non-default token. I named the secret "PAT" here, so you'll need to create that secret with this name holding the token with the "workflows" permissions.

Sorry for the delay. There's one thing that I am maybe missing. This won't bump anything, it will just file a pr. So, why the token is required? And if it is required because we change it on a local branch, can we change the workflow so it will read from a file (which we can treat as the source of truth) so we can avoid the extra permission?

it won't just file a pr. it will run the metallb bump script that includes the bin and bundle-release.
see example: https://github.com/liornoy/metallb-operator/pull/43/files.

Regarding the special token, we need that for allowing this workflow to change the metallb ref under
.github/workflows/metallb_e2e.yml which is a key part of this metallb bump procedure
I don't understand how reading a file will replace the extra permission.

liornoy · 2023-05-08T09:31:59Z

Adding an example of how an auto-gen bump MetalLB PR will look like: liornoy#43

fedepaol · 2023-05-08T09:36:15Z

Regarding the special token, we need that for allowing this workflow to change the metallb ref under .github/workflows/metallb_e2e.yml which is a key part of this metallb bump procedure I don't understand how reading a file will replace the extra permission.

The reason for changing the e2e workflow is because the commit id is embedded in the file. What I am suggesting here is, change the workflow so that it reads a file. In this way the pr will bump the file instead of the workflow and we'll avoid the extra permission.

fedepaol · 2023-05-08T09:37:32Z

it won't just file a pr. it will run the metallb bump script that includes the bin and bundle-release. see example: https://github.com/liornoy/metallb-operator/pull/43/files.

But it won't bump it directly on main right? It will file a pr with the bump so we can just accept it as in liornoy#43 ?

liornoy · 2023-05-08T09:38:10Z

Regarding the special token, we need that for allowing this workflow to change the metallb ref under .github/workflows/metallb_e2e.yml which is a key part of this metallb bump procedure I don't understand how reading a file will replace the extra permission.

The reason for changing the e2e workflow is because the commit id is embedded in the file. What I am suggesting here is, change the workflow so that it reads a file. In this way the pr will bump the file instead of the workflow and we'll avoid the extra permission.

oh I get it. will do.

liornoy · 2023-05-08T09:38:24Z

it won't just file a pr. it will run the metallb bump script that includes the bin and bundle-release. see example: https://github.com/liornoy/metallb-operator/pull/43/files.

But it won't bump it directly on main right? It will file a pr with the bump so we can just accept it as in liornoy#43 ?

yes. under "bumpmetallb" branch

liornoy · 2023-05-08T14:07:43Z

Regarding the special token, we need that for allowing this workflow to change the metallb ref under .github/workflows/metallb_e2e.yml which is a key part of this metallb bump procedure I don't understand how reading a file will replace the extra permission.

The reason for changing the e2e workflow is because the commit id is embedded in the file. What I am suggesting here is, change the workflow so that it reads a file. In this way the pr will bump the file instead of the workflow and we'll avoid the extra permission.

@fedepaol Done

liornoy · 2023-05-08T16:56:53Z

I just realised that I don't must use this github action to read the metallb ref file, but just use a "run" block with bash script.
I'll update it soon

This commit adds a new workflow to periodically align with the upstream MetalLB repository. This is setup to run daily at 7AM and 7PM (GMT+2 clock), run the make `bump_metallb` and if differences exist, then open a PR with the bump, and on the commit message added details of the old commit, and the new commit, just so it will be easier for developers to maintain and keep track of changes. Created metallb_ref.txt as single source of truth for the metallb commit hash. Also deleted the `make test` from the bump_metallb, because we want the workflow not to fail and to open PR even if the unit tests are failing, so we could catch and fix that. Notice: Every time this workflow runs it fetches latest metallb and force-push to already open pr under the "bumpmetallb" branch. Signed-off-by: liornoy <lnoy@redhat.com>

This commit adds .idea/ to the .gitignore in order to help developers using Goland. Signed-off-by: liornoy <lnoy@redhat.com>

fedepaol · 2023-05-09T10:30:26Z

.github/workflows/metallb_e2e.yml

      - name: Checkout MetalLB
        uses: actions/checkout@v2
        with:
          repository: metallb/metallb
          path: metallb
-          ref: 1424dbaef1313b8bb20df136e8eacd2dbacdae02
+          ref: "${{ steps.metallb_ref.outputs.content }}"


fedepaol · 2023-05-09T10:31:47Z

lgtm

liornoy · 2023-05-09T11:12:47Z

It failed because of the BGP large communities commit.
And the fact that it's not currently aligned to latest metallb

liornoy · 2023-05-09T11:23:19Z

@fedepaol do you want me to add another commit to bump metallb to be aligned?
or let's merge it like that and see at 19:00 how the PR looks like when there is a difference in the commit ref?

fedepaol · 2023-05-09T15:19:34Z

Merged. Let's see if it works

liornoy requested review from fedepaol, gclawes and oribon as code owners April 3, 2023 14:06

fedepaol reviewed Apr 5, 2023

View reviewed changes

liornoy force-pushed the add-bump-metallb-automation branch from ac0f8be to a986206 Compare April 16, 2023 14:41

liornoy force-pushed the add-bump-metallb-automation branch 2 times, most recently from 6357ccd to 887f44e Compare April 19, 2023 16:41

liornoy force-pushed the add-bump-metallb-automation branch 2 times, most recently from 7b05bfe to 5c45334 Compare May 8, 2023 14:05

liornoy force-pushed the add-bump-metallb-automation branch from 5c45334 to 3b6c179 Compare May 9, 2023 09:02

liornoy added 2 commits May 9, 2023 12:06

Ignore .idea files

66ade5e

This commit adds .idea/ to the .gitignore in order to help developers using Goland. Signed-off-by: liornoy <lnoy@redhat.com>

liornoy force-pushed the add-bump-metallb-automation branch from 3b6c179 to 66ade5e Compare May 9, 2023 09:06

fedepaol reviewed May 9, 2023

View reviewed changes

fedepaol merged commit 7b1bdfa into metallb:main May 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Periodic MetalLB Bump workflow #341

Add Periodic MetalLB Bump workflow #341

liornoy commented Apr 3, 2023 •

edited

Loading

fedepaol Apr 5, 2023

liornoy Apr 6, 2023

fedepaol Apr 5, 2023

liornoy Apr 6, 2023

liornoy Apr 23, 2023 •

edited

Loading

mlguerrero12 commented Apr 19, 2023

liornoy commented Apr 19, 2023

liornoy commented Apr 19, 2023

liornoy commented Apr 23, 2023

fedepaol commented May 8, 2023

liornoy commented May 8, 2023

liornoy commented May 8, 2023

fedepaol commented May 8, 2023

fedepaol commented May 8, 2023

liornoy commented May 8, 2023

liornoy commented May 8, 2023 •

edited

Loading

liornoy commented May 8, 2023

liornoy commented May 8, 2023

fedepaol May 9, 2023

fedepaol commented May 9, 2023

liornoy commented May 9, 2023 •

edited

Loading

liornoy commented May 9, 2023

fedepaol commented May 9, 2023

Add Periodic MetalLB Bump workflow #341

Add Periodic MetalLB Bump workflow #341

Conversation

liornoy commented Apr 3, 2023 • edited Loading

fedepaol Apr 5, 2023

Choose a reason for hiding this comment

liornoy Apr 6, 2023

Choose a reason for hiding this comment

fedepaol Apr 5, 2023

Choose a reason for hiding this comment

liornoy Apr 6, 2023

Choose a reason for hiding this comment

liornoy Apr 23, 2023 • edited Loading

Choose a reason for hiding this comment

mlguerrero12 commented Apr 19, 2023

liornoy commented Apr 19, 2023

liornoy commented Apr 19, 2023

liornoy commented Apr 23, 2023

fedepaol commented May 8, 2023

liornoy commented May 8, 2023

liornoy commented May 8, 2023

fedepaol commented May 8, 2023

fedepaol commented May 8, 2023

liornoy commented May 8, 2023

liornoy commented May 8, 2023 • edited Loading

liornoy commented May 8, 2023

liornoy commented May 8, 2023

fedepaol May 9, 2023

Choose a reason for hiding this comment

fedepaol commented May 9, 2023

liornoy commented May 9, 2023 • edited Loading

liornoy commented May 9, 2023

fedepaol commented May 9, 2023

liornoy commented Apr 3, 2023 •

edited

Loading

liornoy Apr 23, 2023 •

edited

Loading

liornoy commented May 8, 2023 •

edited

Loading

liornoy commented May 9, 2023 •

edited

Loading