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

DO NOT MERGE: buildah vendor treadmill #13808

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

edsantiago
Copy link
Member

Followup to Cabal discussion 2022-04-07, re: letting us vendor in buildah on a moment's notice.

Instead of cron'ing this, let's see if this works: keep this PR open perpetually, with daily updates to get latest podman and buildah.

See individual commit messages for details.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 7, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 7, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 7, 2022
@edsantiago edsantiago added do-not-merge/blocked-paths Indicates that a PR should not merge because it touches files in blocked paths. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. labels Apr 7, 2022
@edsantiago
Copy link
Member Author

The first commit is the important one. This one must be hand-crafted by a human, there is no possible way to automate it. This is the one that, in case of emergency vendor, can be grabbed into a real PR.

The second commit is a throwaway, ignore it. That's just automated fetch-the-latest-buildah.

@openshift-ci openshift-ci bot removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Apr 7, 2022
@edsantiago
Copy link
Member Author

bud test failed, remote only, because the new --identity-label=false option is not getting passed through. Probably an easy fix, but not this late in my day.

This is still a proof-of-concept PR, and my feeling so far is positive: it has quickly identified multiple problems in the buildah merge that are easy to solve iteratively. A nightly cron job, or a non-gating step in buildah CI, would be utterly useless because neither one would have the crucial first-commit. Without that, there's no way to make it past the first or second or third failure.

@rhatdan
Copy link
Member

rhatdan commented Apr 8, 2022

I just pushed a fix for this.

@edsantiago edsantiago force-pushed the vendor_buildah_latest branch 2 times, most recently from 4c3987c to a58b300 Compare April 8, 2022 17:43
@Luap99
Copy link
Member

Luap99 commented Apr 11, 2022

How is this PR regularly updated?

@edsantiago
Copy link
Member Author

By a human, presumably me. There is actually no other way: this is not possible to solve via cron or PRs.

I am working on a script to automate as much of it as possible.

@edsantiago edsantiago changed the title [WIP] constant blend of latest-podman + latest-buildah DO NOT MERGE: buildah vendor treadmill Apr 12, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 12, 2022
@edsantiago edsantiago marked this pull request as draft April 13, 2022 11:51
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 13, 2022
@edsantiago
Copy link
Member Author

Followup, because I don't want anyone to think this is abandonware: it's not. I'm running my script at least twice a day, one of those times in the evening. One of the nice features I added is:

+---> Podman has bumped, but Buildah is unchanged. There's probably not much point to testing this.

So I'm choosing to save CI cycles by not re-pushing today. Should anyone need to re-vendor buildah tomorrow or this weekend (when I'm unavailable), just cherry-pick the first commit from this PR. It's still valid.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2022
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2022
@edsantiago
Copy link
Member Author

Yippee, script has caught a bug! containers/buildah#3919 breaks when vendoring into podman:

+---> Running 'make' to confirm that podman builds cleanly...
CGO_ENABLED=1 GOOS=linux GOARCH=amd64 go build \
        -mod=vendor  \
        -ldflags '-X github.com/containers/podman/v4/libpod/define.gitCommit=3bf05d3113df29addd392032797f4fdf5dae56b7 -X github.com/containers/podman/v4/libpod/define.buildInfo=1650539506 -X github.com/containers/podman/v4/libpod/config._installPrefix=/usr/local -X github.com/containers/podman/v4/libpod/config._etcDir=/usr/local/etc -X github.com/containers/common/pkg/config.additionalHelperBinariesDir= ' \
        -tags "   selinux systemd  exclude_graphdriver_devicemapper seccomp" \
        -o bin/podman ./cmd/podman
# github.com/containers/podman/v4/libpod
libpod/events.go:18:3: cannot use r.config.Engine.EventsLogFileMaxSize (type "github.com/containers/common/pkg/config".eventsLogMaxSize) as type uint64 in field value
make: *** [Makefile:321: bin/podman] Error 2
buildah-vendor-treadmill-x: 'make' failed with new buildah. Cannot continue.

I could just force-push right now and let y'all see the breakage in CI, or I could just report it like I am right here. Or I could file an issue in containers/common since that's where the offending code seems to be?

Anyhow, @rhatdan et al, PTAL because this is not going to vendor into podman.

@edsantiago
Copy link
Member Author

What the heck, I pushed anyway in case someone wants to pull & play.

Copy link

We were not able to find or create Copr project packit/containers-podman-13808 specified in the config with the following error:

Packit received HTTP 500 Internal Server Error from Copr Service. Check the Copr status page: https://copr.fedorainfracloud.org/status/stats/, or ask for help in Fedora Build System matrix channel: https://matrix.to/#/#buildsys:fedoraproject.org.

Unless the HTTP status code above is >= 500, please check your configuration for:

  1. typos in owner and project name (groups need to be prefixed with @)
  2. whether the project name doesn't contain not allowed characters (only letters, digits, underscores, dashes and dots must be used)
  3. whether the project itself exists (Packit creates projects only in its own namespace)
  4. whether Packit is allowed to build in your Copr project
  5. whether your Copr project/group is not private

Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

Luap99 pushed a commit to Luap99/libpod that referenced this pull request Oct 2, 2024
This commit was automatically cherry-picked
by buildah-vendor-treadmill v0.3
from the buildah vendor treadmill PR, containers#13808

  * Fix conflict caused by Ed's local-registry PR in buildah
  * Wire in "new" --retry and --retry-delay, these existed for longer
    but where non functional.

Signed-off-by: Ed Santiago <[email protected]>
Signed-off-by: Paul Holzinger <[email protected]>
@edsantiago edsantiago force-pushed the vendor_buildah_latest branch 5 times, most recently from cedb9d9 to 43af95e Compare October 23, 2024 21:42
@edsantiago edsantiago force-pushed the vendor_buildah_latest branch 2 times, most recently from 6011906 to 471b6c1 Compare November 11, 2024 13:59
@edsantiago edsantiago force-pushed the vendor_buildah_latest branch 2 times, most recently from 4d35d1a to c02ccd4 Compare November 22, 2024 03:02
@edsantiago
Copy link
Member Author

Gah. I completely forgot about remote.

One possible solution will be to use a custom socket for each process, maybe derived from $BATS_SUITE_TEST_NUMBER or maybe pseudorandom. That is going to be trickier than anything I can do in one day.

Good news is, though, THIS EFFORT IS WORTH IT! (parallelizing). Wow, 15 minutes for bud tests.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2024
@edsantiago
Copy link
Member Author

*sniffle*. Look at this. Passing bud tests, all in less than sixteen minutes. (For comparison: bud tests are by far our biggest slowdown, taking ~35 minutes on a CI run).

My changes here are absolutely not ready for prime time, but I think they're in good enough shape that someone else should be able to carry them through without much fuss.

This is a JUNK COMMIT from buildah-vendor-treadmill v0.3.

DO NOT MERGE! This is just a way to keep the buildah-podman
vendoring in sync. Refer to:

   https://github.com/containers/podman/wiki/Buildah-Vendor-Treadmill

Signed-off-by: Ed Santiago <[email protected]>
@Luap99
Copy link
Member

Luap99 commented Nov 22, 2024

I was counting on the parallel work to get this time down.

My changes here are absolutely not ready for prime time, but I think they're in good enough shape that someone else should be able to carry them through without much fuss.

Just to be clear we need to get the parallel test going in buildah first? Once that works we can enable it here for bud tests?!

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2024
@edsantiago
Copy link
Member Author

Just to be clear we need to get the parallel test going in buildah first? Once that works we can enable it here for bud tests?!

I believe so, yes. For context, that work is in containers/buildah#5552 but there's a weird flake that is blocking it.

I am very curious to find out if we hit that flake here in podman testing. I'll keep re-pushing this PR today.

On second thought, it might be worth thinking about pushing this into podman even before the buildah-parallel flake is fixed. Maybe by skipping the bud: build push with --force-compression test (in apply-podman-deltas): the value of test speedup is, IMNCIO (In My Now Completely Irrelevant Opinion) possibly greater than the value of a completely pointless test under podman. "Pointless" because, as the reader may or may not recall, what bud-in-podman tests do is:

    function run_buildah() {
        if arg is "build" or "bud" or "build-using-dockerfile"; then
            use podman build instead
        else
            invoke buildah directly
        fi
    }

Maybe I should just submit a PR and force-merge it. That way all the nice kind wonderful touching things y'all have said about me will poof, vanish into thin air and be replaced by yells of anger....

As you run --sync, please update this commit message with your
actual changes.

Changes since 2024-11-14:

  * EXPERIMENTAL: try running in parallel on fastvm
  * line-offset-only changes to helpers.bash diffs, due to
    new _prefetch()

  * SUPER-EXPERIMENTAL AND POSSIBLY BROKEN: add parallelization
    mechanism to podman-remote tests.

Signed-off-by: Ed Santiago <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bloat_approved Approve a PR in which binary file size grows by over 50k do-not-merge/blocked-paths Indicates that a PR should not merge because it touches files in blocked paths. do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants