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

Build push #97

Merged
merged 3 commits into from
Mar 24, 2022
Merged

Build push #97

merged 3 commits into from
Mar 24, 2022

Conversation

cevich
Copy link
Member

@cevich cevich commented Oct 4, 2021

These changes produce a new special-purpose VM image containing an embedded script (main.sh) designed to make use of the (included) build-push.sh script from c/automation. The intent is for the skopeo, buildah, and podman repositories to have a shared, uniform, and standard way for building then pushing their own multi-arch manifest lists for public consumption. To that end, a system-test is included here to confirm expected output, given mock inputs.

Note: Currently this task is performed using a difficult to read/maintain Github-actions workflow, making use of docker and buildx. Worse, due to design/implementation defects, this workflow is duplicated across all three containers-org repositories.

@cevich cevich force-pushed the build_push branch 3 times, most recently from 07e1f68 to 87bd0ba Compare October 5, 2021 15:49
@cevich cevich force-pushed the build_push branch 2 times, most recently from d313a02 to b36c528 Compare December 9, 2021 20:52
@github-actions
Copy link

github-actions bot commented Dec 9, 2021

Cirrus CI build successful. Image ID c5287601612521472 ready for use.

@cevich cevich force-pushed the build_push branch 3 times, most recently from 79348ea to 09e0dbd Compare December 10, 2021 17:03
@github-actions
Copy link

Cirrus CI build successful. Image ID c5155236223385600 ready for use.

@cevich cevich force-pushed the build_push branch 6 times, most recently from 437a769 to 13d9b49 Compare December 10, 2021 17:58
@github-actions
Copy link

Cirrus CI build successful. Image ID c5159739704213504 ready for use.

@cevich cevich force-pushed the build_push branch 4 times, most recently from cc252ee to 3806c13 Compare January 20, 2022 18:41
@github-actions
Copy link

Cirrus CI build successful. Image ID c5897222851133440 ready for use.

@github-actions
Copy link

Cirrus CI build successful. Image ID c4996708735123456 ready for use.

1 similar comment
@github-actions
Copy link

Cirrus CI build successful. Image ID c4996708735123456 ready for use.

@cevich cevich force-pushed the build_push branch 2 times, most recently from 607928e to d2d99a4 Compare January 21, 2022 19:54
@cevich cevich force-pushed the build_push branch 2 times, most recently from 53802ef to b7b5ab1 Compare February 2, 2022 17:47
@cevich
Copy link
Member Author

cevich commented Mar 9, 2022

Thanks, yeah no rush, it's aimed at replacing something already "working", so low-urgency. I believe you already saw/reviewed the underlying build-push.sh script (from c/automation repo). This PR is just about bundling that tool with an environment and execution recipe. As-is, it should produce the same output images as the github-action workflows...though I haven't gone as far as actually trying that yet.

Edit: intended use ref: containers/podman#13478

build-push/bin/main.sh Outdated Show resolved Hide resolved
lib.sh Show resolved Hide resolved
build-push/bin/main.sh Outdated Show resolved Hide resolved
build-push/bin/main.sh Outdated Show resolved Hide resolved
@edsantiago
Copy link
Member

Sorry for the delay - every time I start on this, other priorities come up.

And... sorry again, but I can't really review this. It's huge, and there's a lot of outside context I'm not privy to. I've done one full pass looking for basic inconsistencies, but I really can't review this in depth. I'd say just go for it, push it, and if something breaks it can be iteratively fixed.

@cevich
Copy link
Member Author

cevich commented Mar 22, 2022

And... sorry again, but I can't really review this. It's huge, and there's a lot of outside context I'm not privy to.

Yep I understand. Most of the outside context you've reviewed in the past, but it is complex overall. However, my goal here is to re-implement the (3x duplicated) github-action workflow in a way that's unit & integration tested, can be re-used across all three repos., and is mostly human readable/maintainable (given some amount of "study-time" of course).

Though I totally understand reviewing this likely also requires at least some understanding of the github-action workflow...which is a built-up mess, tweaked and extended based on lots of group discussions 😖 Perhaps it would be helpful to future maintainers, if instead of one big main.sh, I broke this up further into three scripts: One for handling the 'stable' image, and two more for dealing with 'testing' and 'upstream' images?

Some Context: These images are ultimately downloaded thousands of times per month, so if some change is needed in the build process, I want this overall build setup to be maintainable by people other than me. Maybe that's asking too much though, I can't tell. If you have readability suggestions, I'm definitely all ears 👂

@cevich
Copy link
Member Author

cevich commented Mar 22, 2022

Converting this back to draft-status so the DO NOT MERGE commit doesn't accidentally get merged in.

@github-actions
Copy link

Cirrus CI build successful. Image ID c5529980355477504 ready for use.

README.md Show resolved Hide resolved
@cevich
Copy link
Member Author

cevich commented Mar 24, 2022

Force-push: removed temporary DO NOT MERGE commit.

@cevich cevich marked this pull request as ready for review March 24, 2022 16:05
@github-actions
Copy link

Cirrus CI build successful. Image ID c5785324482723840 ready for use.

@edsantiago
Copy link
Member

LGTM but the github diff page is showing a bunch of ugly dark yellow warning messages

@cevich
Copy link
Member Author

cevich commented Mar 24, 2022

a bunch of ugly dark yellow warning messages

Ya, ignore those, they're false-positive YAML problems. Anyway, thanks for looking, I'll fix up the docs and get this in.

@github-actions
Copy link

Cirrus CI build successful. Image ID c6261670816251904 ready for use.

@cevich cevich merged commit 1501326 into containers:main Mar 24, 2022
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.

2 participants