-
Notifications
You must be signed in to change notification settings - Fork 187
container pushing updates #3096
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
container pushing updates #3096
Conversation
dustymabe
commented
Sep 13, 2022
|
There are a few things going on in this PR. First after this merges we'll be able to switch our FCOS container push to use |
| # Allow the builder user to run rootless podman | ||
| # Referenced at: https://github.com/containers/podman/issues/4056#issuecomment-1245715492 | ||
| # Lifted from: https://github.com/containers/podman/blob/6e382d9ec2e6eb79a72537544341e496368b6c63/contrib/podmanimage/stable/Containerfile#L25-L26 | ||
| echo -e "builder:1:999\nbuilder:1001:64535" > /etc/subuid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine, though ultimately I think what we want is to operate on oci directories for this - there's no reason to pull images into containers-storage: here. I am not aware of tooling for this that lives in the github.com/containers ecosystem though.
There's https://github.com/opencontainers/umoci which I think handles this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also worth noting that this is an overlapping change with #2985 which uses supermin to run podman instead there.
Ultimately what we want is https://kubernetes.io/docs/concepts/workloads/pods/user-namespaces/ stabilized - that unblocks clean nested containerization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also worth noting that this is an overlapping change with #2985 which uses supermin to run
podmaninstead there.
Yep. My suggestion in #3096 (comment) is that in the future (when we don't have clusters per architecture in RHCOS and just have multi-arch builders running FCOS like we do for the FCOS pipeline) we won't do that but rather run the builds through cosa remote-build-container which performs the builds via podman --remote. It's how we're building cosa and fcos-buildroot today.
7d83180 to
a68f72f
Compare
ravanelli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it lgtm, I don't know much about the `oci part to have an opinion though.
a68f72f to
939e169
Compare
These two utilities create old style oscontainer images (i.e. not runnable containers). They are on the way out in favor of the new OSTree Native Containers. This commit renames the files to add `deprecated-legacy-format` into the filename. It symlinks the old filename back to the new filename to keep backwards compatiblity. It's a cosmetic change, but hints at our direction.
This will allow us to build containers that we want to deliver in our uploaded build artifacts rather than pushed to a registry.
This takes a similar approach as openshift [1] where some labels with information about the git repo are added to the built container image to allow for rich information to be inpsected without having to download an image from a registry. [1] https://access.redhat.com/documentation/en-us/openshift_container_platform/4.8/html/cicd/builds#builds-output-image-labels_managing-build-output
Colin pointed me at [1], which are some standard labels for this kind of thing. Let's just use those instead. Fixes: coreos#3049 [1] https://github.com/opencontainers/image-spec/blob/main/annotations.md#pre-defined-annotation-keys
This passes through to `podman build` and allows for replacing the FROM line in the Containerfile/Dockerfile.
We had os.environ and also environ. Let's make it more consistent.
This file was originally a copy/paste of another file and this comment was mistakenly left over from that. Also fix a typo in the sentence above it.
This commit adds an --artifact option to `cosa push-container-manifest`
which allows for pushing artifacts that were built as part of a
pipeline build and are referenced in the `meta.json` file to a remote
registry.
It is multi-arch aware and defaults to pushing a manifest list with
all the requested architectures available for that build.
Example usage:
cosa push-container-manifest \
--repo quay.io/dustymabe/fedora-coreos --tag stable --artifact=ostree \
--metajsonname=base-oscontainer --build=latest --arch=x86_64 --arch=aarch64
We do *some* podman operations inside the COSA container. If running
locally as the `builder` user podman will barf when trying to run
newuidmap if we don't change up the subuid/subgid mappings.
With this change we'll be able to test in our local rootless podman
COSA container that `cosa push-container-manifest` works.
In order to figure out this worked (at least for what limited podman
manifest commands I'm running) I first followed the issue at [1]
and realized I had success with the `quay.io/podman/stable` image
and then looked inside the image to see what the mapping was.
I then lifted the mapping from there [2] and applied it here and
it works.
Note that inside the pipeline right now (in OpenShift) we still run
as a random user but that seems to still be working OK for us for
pushing the manifest because it can't find the random UID/GID in
/etc/{subuid,subgid} so it falls back to "rootless single mapping
into the namespace".
[1] containers/podman#4056 (comment)
[2] https://github.com/containers/podman/blob/6e382d9ec2e6eb79a72537544341e496368b6c63/contrib/podmanimage/stable/Containerfile#L25-L26
939e169 to
762f687
Compare