.github: add workflow to open downstream PR #305

flouthoc · 2025-09-02T20:39:30Z

No description provided.

mtrmac

I don’t understand; what is the goal of this?

With the go mod edit -replaced, I read this to mean that none of these PRs should ever be merged. So why are they PRs?

Guessing, is this intended to test container-libs in context of Buildah, but in a way that does not block merging any changes in container-libs?

.github/workflows/opendownstream-pr.yml

mtrmac · 2025-09-03T14:41:54Z

.github/workflows/opendownstream-pr.yml

+          if [ "${{ env.PR_ACTION }}" = "created" ]; then
+            COMMENT_BODY="✅ A new PR has been created in buildah to vendor these changes: **${{ env.TARGET_REPO_PR_URL }}**"
+          else
+            COMMENT_BODY="✅ The existing PR in buildah has been updated with these changes: **${{ env.TARGET_REPO_PR_URL }}**"


Is this going to add the same comment on every force-push? That might be rather annoying. Can this edit a pre-existing comment, or use some other way to convey that state? (a test status???)

Actually I don't think we need to notify as a comment at all on every force push, I can remove this part. The objective of workflow should be only to update/rebase downstream PR.

I am in favor of removing this part if it creates too much noise.

yes there is no need to comment again so I would remove it.

flouthoc · 2025-09-03T16:55:54Z

@mtrmac This PR is not ready for review yet. I will undraft it when it's ready.

Luap99 · 2025-09-04T12:39:49Z

I don’t understand; what is the goal of this?

The goal is to open a PR in buildah/podman automatically to run there tests there to see if that passesCI there. This is done as it seems like the simplest solution compared to trying to run the buildah/podman tests here (like done with skopeo) since that would require a ton of cirrus.yml duplication. The goal is simply to catch regressions before me merge the PRs here.

Now it makes sense to define some sort of criteria when to open a PR over there, first of all it should only happen when .go files are changed but I guess for many trivial changes we might still want to save the time?

mtrmac · 2025-09-04T14:55:23Z

The goal is to open a PR in buildah/podman automatically to run there tests there to see if that passesCI there. This is done as it seems like the simplest solution compared to trying to run the buildah/podman tests here (like done with skopeo) since that would require a ton of cirrus.yml duplication.

Ah, OK. If the depending projects are fine with the ongoing flow of PRs, filing PRs certainly works and it is easier to set up.

flouthoc · 2025-09-10T15:06:05Z

@Luap99 PTAL

mtrmac · 2025-09-10T17:15:28Z

.github/workflows/opendownstream-pr.yml

+            GOWORK=off go mod vendor
+            GOWORK=off go mod verify


(Doing this only once might be faster.)

I tried this but go mod -replace doesn't allows me do multiple replace directive and then run go mod vendor.

replace; tidy; replace; tidy; replace; tidy; vendor; verify works for me.

Works thanks !

mtrmac · 2025-09-10T17:19:02Z

.github/workflows/opendownstream-pr.yml

+name: 'Open downstream PRs'
+
+on:
+  pull_request_target


https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target contains a fairly strong warning.

This might well be OK, just a note to be careful. I didn’t now study that warning in detail.

we need this to access the secrets, but yes permissions wise we should configure them to the manual, since the workflow post a comment to a PR we need pull-request: write but the rest can be read I think.

Luap99 · 2025-09-11T13:16:15Z

.github/workflows/opendownstream-pr.yml

+name: 'Open downstream PRs'
+
+on:
+  pull_request_target


we need this to access the secrets, but yes permissions wise we should configure them to the manual, since the workflow post a comment to a PR we need pull-request: write but the rest can be read I think.

.github/workflows/opendownstream-pr.yml

Luap99 · 2025-09-11T13:23:55Z

.github/workflows/opendownstream-pr.yml

+        with:
+          repository: 'containers/buildah' # The target repository
+          path: 'buildah'                 # Checkout into a sub-directory
+          token: ${{ secrets.VENDOR_TOKEN_PODMANBOT }}


we should not need the podmabot token for the checkout do we?

We are also pushing to branch later, we need it for that.

Luap99 · 2025-09-11T13:24:55Z

.github/workflows/opendownstream-pr.yml

+          git fetch upstream
+          git rebase upstream/main


we already cloned the repo before so this just fetched the full history which seems unnecessary?

We are cloning podmanbot/buildah we need to rebase against main because we will open PR there.

but you don't have to clone podmanbot/buildah at all, you can clone https://github.com/containers/buildah.git just fine and then add podmanbot/buildah as push target without having to pull that one.

I guess the repos are not that big that it matters that much but as far as checkout goes it should be enough to just clone with --depth=1 to not load the full history create a commit and push that one.
Note sure if the gh cli will have problems there then but worth to test IMO.

I set fetch depth=1 .

Well you still fetch the entire update repo here, anyway I guess performance wise it doesn't matter much so I don't care to much about it.

Luap99 · 2025-09-11T13:32:18Z

.github/workflows/opendownstream-pr.yml

+          git commit -m "feat: Vendor changes from podmanbot/container-libs#${{ github.event.pull_request.number }}"
+
+          # Force push to update the branch if the action re-runs on 'synchronize'
+          git push origin $BRANCH_NAME --force


isn't origin containers/buildah here and the bot of course should have no direct push perms, it should push to its own fork and create the PR from there.

yes this push is going to podmanbot/buildah

The origin is podmanbot/buildah afaik

@jankaluza Yes, Correct.

(when I added the comment the origin was not podmanbot)

Anyhow logically it is really pointless to clone the outdated podmanbot repo. We just need to add this as remote to push to, we would ways want to clone only main of containers/buildah.

Cloning the full history just makes things slower like I mentioned here: #305 (comment)

Anyhow I am fine to merge it like this. We should first see if this works like that and is helpful before over optimizing I guess.

.github/workflows/opendownstream-pr.yml

Luap99 · 2025-09-11T13:34:10Z

.github/workflows/opendownstream-pr.yml

+          if [ "${{ env.PR_ACTION }}" = "created" ]; then
+            COMMENT_BODY="✅ A new PR has been created in buildah to vendor these changes: **${{ env.TARGET_REPO_PR_URL }}**"
+          else
+            COMMENT_BODY="✅ The existing PR in buildah has been updated with these changes: **${{ env.TARGET_REPO_PR_URL }}**"


yes there is no need to comment again so I would remove it.

.github/workflows/opendownstream-pr.yml

Luap99 · 2025-09-11T17:25:41Z

.github/workflows/opendownstream-pr.yml

+          update_module() {
+            local module=$1
+            echo "Updating module: $module"
+            go mod edit -replace ${module}=github.com/containers/container-libs/${module#go.podman.io/}@${COMMIT_SHA}


I am somewhat sure that this will not work. The commit will not be resolvable on this repo, it would actually need to point to the right fork AFAICT.

GitHub is a bit notorious about resolving commit references within the context of a different fork of the same repo.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally actually suggests that this is an intentional feature, for PRs:

Once a pull request is opened, GitHub stores all of the changes remotely. In other words, commits in a pull request are available in a repository even before the pull request is merged.

Well I can take the commit sha from this PR but it will not work

$ go mod edit -replace go.podman.io/common=github.com/containers/container-libs/common@c0d0f80f0eb2cf8e799580b58f0f90108aa407b3 && make vendor go mod tidy go: errors parsing go.mod: go.mod:196: replace github.com/containers/container-libs/common: version "c0d0f80f0eb2cf8e799580b58f0f90108aa407b3" invalid: unknown revision c0d0f80f0eb2cf8e799580b58f0f90108aa407b3 make: *** [Makefile:348: vendor] Error 1

I Also tried different from of the pull/305/head syntax without luck.

Keep in mind go pull the repo in the normal git and then the commit must be in the repo. And I am very sure these PR commits are never part of the repo if you do the normal git clone

Good to know, I guess Go’s pseudo-version mechanism is where that fails.

Okay i changed it to go mod edit -replace ${module}=github.com/${{ github.event.pull_request.head.repo.full_name }}/${module#go.podman.io/}@${COMMIT_SHA} and tested it here: flouthoc#1

Luap99

LGTM I guess. I am far from confident with the github action syntax and PR creation/bot flow?

Do you need me to add the VENDOR_TOKEN_PODMANBOT here to the repo?

flouthoc · 2025-09-12T15:53:10Z

LGTM I guess. I am far from confident with the github action syntax and PR creation/bot flow?

Do you need me to add the VENDOR_TOKEN_PODMANBOT here to the repo?

It is already added by @mheon afaik

flouthoc · 2025-09-12T17:53:45Z

@mtrmac @mheon @jankaluza @ashley-cui PTAL

jankaluza · 2025-09-15T14:32:42Z

LGTM.

I think this is sane way how to do that.

It would be super cool in the future to add a job which waits for the CI pipeline in the buildah PR to succeed/fail, so this could be directly visible in the container-libs as another CI test. If I understand it right, currently, the maintainer or author of PR has to open the buildah PR to verify whether things are green there.

There are some Github Actions doing something very similar, for example https://github.com/Codex-/await-remote-run?tab=readme-ov-file.

ashley-cui · 2025-09-15T14:38:56Z

.github/workflows/opendownstream-pr.yml

+      - name: 'Check for Go file changes'
+        id: check_go_changes
+        run: |
+          # Get the list of changed files in the PR
+          CHANGED_FILES=$(gh pr view ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --json files --jq '.files[].path')
+
+          # Filter for relevant Go files that should trigger downstream sync
+          # Include: .go files (excluding test files and vendor directory)
+          # Exclude: *_test.go files, vendor/ directory files
+          RELEVANT_GO_FILES=$(echo "$CHANGED_FILES" | grep -E '\.go$' | grep -v '_test\.go$' | grep -v '^vendor/' || echo "")
+
+          if [ -n "$RELEVANT_GO_FILES" ]; then
+            echo "Relevant Go files changed (excluding tests and vendor):"
+            echo "should_run=true" >> $GITHUB_OUTPUT
+            echo "go_changes=true" >> $GITHUB_ENV
+          else
+            echo "No relevant Go files were changed in this PR."
+            echo "Only test files, vendor files, or non-Go files were modified - skipping downstream sync."
+            echo "should_run=false" >> $GITHUB_OUTPUT
+            echo "go_changes=false" >> $GITHUB_ENV
+          fi
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}


I think we can just use paths

on: pull_request_target: paths: - '**.go' - '!vendor/**' - '!**.test.go'

This way, the GH action won't even be triggered if a go file isn't changed.

Luap99 · 2025-09-15T15:06:47Z

It would be super cool in the future to add a job which waits for the CI pipeline in the buildah PR to succeed/fail, so this could be directly visible in the container-libs as another CI test. If I understand it right, currently, the maintainer or author of PR has to open the buildah PR to verify whether things are green there.

The problem is these jobs "waste" a lot of runner times just sitting idle, our org only has access to the free runners which are limited to 20 at the same time or so IRC. With the size of this org we have seen several situations in the past where jobs were delayed for a very long time as to many repos used them.

jankaluza · 2025-09-15T16:42:15Z

It would be super cool in the future to add a job which waits for the CI pipeline in the buildah PR to succeed/fail, so this could be directly visible in the container-libs as another CI test. If I understand it right, currently, the maintainer or author of PR has to open the buildah PR to verify whether things are green there.

The problem is these jobs "waste" a lot of runner times just sitting idle, our org only has access to the free runners which are limited to 20 at the same time or so IRC. With the size of this org we have seen several situations in the past where jobs were delayed for a very long time as to many repos used them.

Thanks for the context. I didn't know that.

.github/workflows/opendownstream-pr.yml

Luap99 · 2025-09-15T16:56:10Z

.github/workflows/opendownstream-pr.yml

+      - '**/*.go'
+      - '!vendor/**'
+      - '!**.test.go'
+      - '!**/*.test.go'


This has to match '!**/*_test.go' for the go test file convention.

Signed-off-by: flouthoc <[email protected]>

flouthoc · 2025-09-15T17:11:54Z

@Luap99 @ashley-cui PTAL again.

ashley-cui · 2025-09-15T17:26:56Z

LGTM

Luap99

LGTM

flouthoc force-pushed the downstream-PR branch 5 times, most recently from 6f01976 to 2a95105 Compare September 3, 2025 00:07

mtrmac reviewed Sep 3, 2025

View reviewed changes

flouthoc marked this pull request as draft September 3, 2025 16:54

flouthoc force-pushed the downstream-PR branch from 2a95105 to bb97ae3 Compare September 10, 2025 14:57

flouthoc marked this pull request as ready for review September 10, 2025 14:57

flouthoc force-pushed the downstream-PR branch from bb97ae3 to 0be193c Compare September 10, 2025 17:13

mtrmac reviewed Sep 10, 2025

View reviewed changes

Luap99 reviewed Sep 11, 2025

View reviewed changes

flouthoc force-pushed the downstream-PR branch from 0be193c to c0d0f80 Compare September 11, 2025 16:22

flouthoc requested a review from Luap99 September 11, 2025 16:24

Luap99 reviewed Sep 11, 2025

View reviewed changes

flouthoc force-pushed the downstream-PR branch from c0d0f80 to c9fe53e Compare September 11, 2025 19:01

flouthoc requested a review from Luap99 September 11, 2025 19:04

Luap99 approved these changes Sep 12, 2025

View reviewed changes

flouthoc requested a review from mtrmac September 12, 2025 17:53

ashley-cui reviewed Sep 15, 2025

View reviewed changes

flouthoc force-pushed the downstream-PR branch from c9fe53e to 05b1d9e Compare September 15, 2025 16:36

flouthoc requested a review from ashley-cui September 15, 2025 16:37

flouthoc requested review from Luap99 and jankaluza September 15, 2025 16:37

Luap99 reviewed Sep 15, 2025

View reviewed changes

.github: add workflow to open downstream PR

251740f

Signed-off-by: flouthoc <[email protected]>

flouthoc force-pushed the downstream-PR branch from 05b1d9e to 251740f Compare September 15, 2025 17:08

flouthoc requested a review from Luap99 September 15, 2025 17:08

Luap99 approved these changes Sep 15, 2025

View reviewed changes

Luap99 merged commit a4dd8cd into containers:main Sep 15, 2025
16 checks passed

mtrmac mentioned this pull request Oct 1, 2025

opendownstream-pr workflow should close buildah PRs once the PR here merges #361

Open

.github: add workflow to open downstream PR #305

.github: add workflow to open downstream PR #305

Uh oh!

Conversation

flouthoc commented Sep 2, 2025

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flouthoc commented Sep 3, 2025

Uh oh!

Luap99 commented Sep 4, 2025

Uh oh!

mtrmac commented Sep 4, 2025

Uh oh!

flouthoc commented Sep 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jankaluza Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jankaluza Sep 15, 2025 •

edited

Loading

flouthoc commented Sep 12, 2025 •

edited

Loading