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

[CI:BUILD] Contrib: Add containerfile to create podman-remote binary image #15813

Conversation

praveenkumar
Copy link
Contributor

Try to partial address #14664

- Add container file to build remote binary for each platform
- Add README.md file around same

Longer term plan is to have it attach either to quay and add github trigger to generate the image for each 4.x tag/branch. Only issue is as of now quay github trigger can only generate amd64 images not the arm64 one so may be we need to attach it to current cirrus CI to generate image for different arch and push to quay, same way we are doing for quay.io/containers/podman.

Another gap is created binaries for mac/windows are not signed one and we need to check if user try to copy those binary from the container image does it showing the signing warning.

@praveenkumar praveenkumar force-pushed the podman-remote-container-file branch from 657931f to 23a9733 Compare September 15, 2022 08:45
Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

LGTM other than two nits above.


- For Linux binary
```bash
$ podman cp $(podman create --name remote-temp quay.io/praveenkumar/podman-remote-artifacts:latest):/podman-remote-static . && podman rm remote-temp
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$ podman cp $(podman create --name remote-temp quay.io/praveenkumar/podman-remote-artifacts:latest):/podman-remote-static . && podman rm remote-temp
$ podman cp $(podman create --name remote-temp quay.io/containers/podman-remote-artifacts:latest):/podman-remote-static . && podman rm remote-temp

Copy link
Collaborator

@flouthoc flouthoc Sep 15, 2022

Choose a reason for hiding this comment

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

Points to personal repo, are we expecting to create similar repo/tag in containers

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think similar comment goes for darwin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flouthoc yes I forgot to remove my personal repo :) . I think we should have similar repo/tag in containers org for consumption.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 15, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, praveenkumar

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 Sep 15, 2022
@rhatdan
Copy link
Member

rhatdan commented Sep 15, 2022

@mheon @cevich @vrothberg PTAL


This directory contains the containerfile for creating a container image which consist podman-remote binary
for each platform (win/linux/mac). Plan is to hook it with quay github build trigger to generate this image
for each PR merged (in any 4.x branch or tags).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for each PR merged (in any 4.x branch or tags).
for each PR merged (for any branch or tag).

@praveenkumar praveenkumar force-pushed the podman-remote-container-file branch from 23a9733 to 6c162a3 Compare September 15, 2022 11:00
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

What is the benefit over curl-ing the binaries from the release pages? I assume for testing purposes but I'd appreciate making the intention explicit in the README.

I'd like some more maintainers to have a look.

for each platform (win/linux/mac). Plan is to hook it with quay github build trigger to generate this image
for each PR merged (in any 4.x branch or tags).

User can copy those binary on specific platform using following
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
User can copy those binary on specific platform using following
Users can copy the binaries onto the specific platforms using following instructions

@cevich cevich changed the title Contrib: Add containerfile to create podman-remote binary image [CI:BUILD] Contrib: Add containerfile to create podman-remote binary image Sep 15, 2022
@cevich
Copy link
Member

cevich commented Sep 15, 2022

Longer term plan is to have it attach either to quay and add github trigger to generate the image for each 4.x tag/branch.

Yeah, can't use quay. We have two Cirrus-CI tasks setup to build multi-arch image manifests, image_build and test_image_build. The later is intended for PRs (like this one) so you can test out new/changed builds. It's only available for manual-trigger, and only if you put [CI:BUILD] in the PR description (which I just did).

@cevich
Copy link
Member

cevich commented Sep 15, 2022

So on your next force-push, you'll see the test_image_build manual-trigger jobs pop up. I'll most likely need to help add the new one, since the build script is sensitive to the directory names.. I'll also need to setup the new repo in quay for it to push to.

@cevich
Copy link
Member

cevich commented Sep 15, 2022

Q: @rhatdan @TomSweeneyRedHat or anyone else: do we want to provide the stable, testing, and upstream flavors of the podman-remote image as well as a secondary push to quay.io/containers/podman:{stable, testing, upstream} and quay.io/podman-remote (new repo)? Since that logic is already cooked into the build script, it would be good to iron this all out up-front if possible.

@@ -0,0 +1,10 @@
FROM registry.access.redhat.com/ubi8/go-toolset:latest AS builder
Copy link
Member

Choose a reason for hiding this comment

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

All of our other container images are based on registry.fedoraproject.org/fedora:latest, for consistency it would be good to use that here too. An alternative could be registry.fedoraproject.org/fedora-minimal:latest. In either case, check out the contrib/podmanimage/*/Containerfiles to see how we install podman. It's MUCH faster to install than to compile during image-build time (due to needing emulation).

Copy link
Member

Choose a reason for hiding this comment

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

errr...nevermind 😞 that won't work for Mac or Windows will it? My concern is the multi-arch compiles are extremely slow under emulation. Hmmmm. Perhaps the thing to do is just grab the binaries directly from the CI system's artifacts task. They're not signed, but will be posted continuously for main, and recent release-branches (where we added ARM64 builds).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cevich Does it also published for specific tags like rc one? One of the use case which I am trying to solve with it, if we do any release on github (with specific branch) be it alpha, beta, rc we should have release binaries for those bits (longer term) either through the container image or from the CI artifacts. If I see #15202 (which is the PR for rc3) then looks like we just use the rc3 commit to do the tagging and there is no CI artifact for it.

Copy link
Member

Choose a reason for hiding this comment

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

Mostly, I don't think tags to, but for sure on most recent branches (and going forward) the CI system continuously publishes artifacts at a consistent URL. The URL will be of the form specified here: https://cirrus-ci.org/guide/writing-tasks/#latest-build-artifacts

COPY . .
RUN make podman-remote-static
RUN GOOS=windows make podman-remote
RUN GOOS=darwin make podman-remote
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of: Instead of compiling, curl the latest binaries from CI system using the branch-name specified as a build-arg?

Copy link
Member

Choose a reason for hiding this comment

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

I like that idea, then we know everyone is using the exact same binary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cevich if we are adding this to CI for each PR then first we have to wait for other CI run to be succeed (to generate required images) as part of some step builds?

Copy link
Member

Choose a reason for hiding this comment

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

Not the way the builds run today. They're optional on [CI:BUILD] PRs (to support testing/development). Otherwise they only run via a daily cron-job on main.

@praveenkumar praveenkumar force-pushed the podman-remote-container-file branch from 6c162a3 to 8c627df Compare September 26, 2022 05:04
@rhatdan
Copy link
Member

rhatdan commented Sep 26, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2022
@openshift-merge-robot openshift-merge-robot merged commit cf6136f into containers:main Sep 26, 2022
Overview
--------

This directory contains the containerfile for creating a container image which consist podman-remote binary
Copy link
Member

Choose a reason for hiding this comment

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

Nit and way past, but just in case containerfile -> Containerfile

This directory contains the containerfile for creating a container image which consist podman-remote binary
for each platform (win/linux/mac).

Users can copy those binaries onto the specific platforms using following instructions
Copy link
Member

Choose a reason for hiding this comment

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

following -> the following

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants