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

[rhcos-4.11-new] Backport container-related patches #3170

Merged
merged 37 commits into from
Nov 9, 2022

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Nov 7, 2022

This backports cosa push-container-manifest, cosa copy-container, and cosa buildextend-legacy-oscontainer.

@openshift-ci
Copy link

openshift-ci bot commented Nov 7, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jlebon
Copy link
Member Author

jlebon commented Nov 7, 2022

Still testing this locally.

ravanelli and others added 27 commits November 7, 2022 17:36
- Add common functions used in the utils library

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit a15d3a0)
- Add manifest create and push functions in order to create a multi arch
manifest list

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit 952c98c)
Replace run_cmd with a more featureful version taken from our
coreos-ostree-importer [1]. Also rename it to runcmd and convert
all uses to pass in a list versus a string for the command to
decrease the chances of shell injection.

[1] https://github.com/coreos/fedora-coreos-releng-automation/blob/e85d22672ef8acb79ba22a4b11d0f529585d74b7/coreos-ostree-importer/coreos_ostree_importer.py#L232-L245

(cherry picked from commit 2a1121d)
Nothing is using this yet so we might as well make some easy
improvements. First off let's name the file more specifically
and name the functions more specifically. Also let's add some
extra functions to help things work better.

(cherry picked from commit 5385e90)
This will create and push a container manifest (i.e. one that contains
multiple images from different architectures) to a remote registry.

(cherry picked from commit 8e5ad58)

jlebon: Had to manually add `push-container-manifest` to the entrypoint
        since we had already moved to the golang rewrite in the original
        commit.
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.

(cherry picked from commit 44361f6)

jlebon: I re-created this commit manually because a pure cherry-pick
        didn't quite work since we didn't backport the `runcmd` stuff.
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.

(cherry picked from commit 8dfc459)
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

(cherry picked from commit a8259fe)
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

(cherry picked from commit 5ffbf12)
…dated

I needed to include the `buildmeta.write` inside the for loop so that
all meta.json files for each arch would get updated with the reference
to the manifest list.

Fixes a8259fe

(cherry picked from commit 4020e84)
This introduces a new command to create a oci-archive of the
legacy oscontainer that will be pushed with
`cosa push-container-manifest` by the pipeline.

(cherry picked from commit 1f4cf7f)

jlebon: Had to resolve conflicts in schema change. Also manually changed
        new code to use `runcmd` from `cosalib.utils` instead of
        `cosalib.cmdlib`.
In the RHCOS pipeline, we want to be able to tag the same image using
both the stream name and the build ID.

(cherry picked from commit d335641)
The `oscontainer` and `base-oscontainer` keys should follow the same
schema. Currently, the former has a `digest` field, while the other one
does not. Tweak `cosa push-container-manifest` and `cosa push-container`
so that they follows the new schema. (Though note the latter command
will be deleted soon).

To keep previous 4.12 `meta.json` files valid, this loosens the `image`
schema definition so that `digest` is now optional. Once we branch for
4.12, we will undo this change so that it becomes required again.

Fixes coreos#3122

(cherry picked from commit 738cb39)

jlebon: I skipped backporting the changes to the schema since we don't
        need to be as flexible for 4.11. I also skipped the similar
        change to `cmd-push-container` since we're not planning to use
        that at all in the new pipeline.
As discussed in coreos#3122,
ART needs the arch-specific digest in the `meta.json` rather than the
manifest list digest. FCOS isn't planning to use the digest so doesn't
care about which one gets chosen.

Update `cosa push-container-manifest` to use the arch-specific digest.

(cherry picked from commit c7b5757)
This command supports copying images between registries, optionally
converting a manifest list into multiple images tagged by architecture.

This will be used by the pipeline to copy RHCOS images from their
canonical Quay.io location to registry.ci.openshift.org.

(cherry picked from commit c28bad0)

jlebon: Had to add command to bash entrypoint instead of go. Had to
        modify new code to use `runcmd` from `cosalib.utils.`
…ntainer`

This way, it's consistent with all the other artifacts we build in the
pipeline which makes it easier to add support for. Don't retain the old
name since it's not used anywhere else yet.

(cherry picked from commit 03dd8da)

jlebon: Had to add command to bash entrypoint instead of go.
This makes it clearer that it's the legacy oscontainer and not the
OSTree-native one.

(cherry picked from commit 6aafc08)
This command no longer pushes to a registry, so we don't need to save
the container digest.

(cherry picked from commit 77bb489)
This script no longer pushes. Instead teach a new `--ociarchive` which
takes a path to the output OCI archive to write.

(cherry picked from commit d9e68ef)
We're seeing the classic ENOMEM error from `cosa
buildextend-legacy-oscontainer` when trying to write back the legacy
oscontainer over 9p. Instead, have the supermin VM output the final OCI
archive over virtio-serial.

(cherry picked from commit 13a6ee4)
We want `buildah` to use the default temporary directory instead of one
that lives on the 9p mount. Do this by stopping to pass `workdir` (not
to be confused with the cosa workdir; this is the directory which
previously contained the temporary containers-storage and tmp dir).

This whole thing is really hard to follow. But the nice thing is that we
should be able to get rid of it soon.

(cherry picked from commit 1f05e1d)
That helps other readers understand why this file is a near duplicate of
oscontainer-deprecated-legacy-format.py.

(cherry picked from commit fca3c87)
It's not really useful anymore without passing that.

(cherry picked from commit 9b7c86d)
This isn't used anymore.

(cherry picked from commit 3d89e1a)
jlebon and others added 3 commits November 7, 2022 17:37
We need to support arch peeling even for multi-arch source containers
pushed using v2s2. Thankfully, the schema is identical to the OCI schema
for our purposes which makes supporting it painless.

Related: coreos#2726
(cherry picked from commit cd32708)
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
- If no metajsonname exists in the meta.json then we get a KeyError
  so we need to fix that by using `.get()`.
- If no metajsonname entry exists then we need to upload. Right now
  this won't set upload = True and we end up not uploading anything.

(cherry picked from commit 9e68cf2)
@jlebon jlebon force-pushed the pr/backport-container-stuff branch from ace038a to 3cfe5f0 Compare November 7, 2022 22:39
travier and others added 2 commits November 8, 2022 12:25
Copy repo config files from config dir & yumrepos git repo.

Fixes: coreos#3074
Fixes: coreos#3036
Fixes: coreos#3045
(cherry picked from commit 5c708b4)
As part of https://issues.redhat.com/browse/COS-1646 podman is
going to be running inside the supermin vm, we run out of disk
quickly with 5G when loading the rhcos image in the FROM line.
Podman also needs /sys/fs/cgroup properly mounted as cgroup2.

(cherry picked from commit de1856c)
@jlebon jlebon marked this pull request as ready for review November 8, 2022 17:49
@jlebon
Copy link
Member Author

jlebon commented Nov 8, 2022

OK, I can buildextend-legacy-oscontainer locally now! I'm waiting for #3174 to merge before backporting that too.

The old pipeline doesn't use this code, and the new pipeline doesn't
want to override the artifact name.

(cherry picked from commit e480fba)
Prep for next patch.

(cherry picked from commit e604d00)
For consistency with all other buildextend commands.

(cherry picked from commit 85fda1e)
All `meta.json` updates must use the `GenericBuildMeta` API to make sure
that concurrent writes don't race.

(cherry picked from commit d6a0f4a)

jlebon: Had to resolve conflicts around runcmd.
This command doesn't do that anymore.

(cherry picked from commit c795904)
@jlebon
Copy link
Member Author

jlebon commented Nov 8, 2022

Now with #3174 backported!

@openshift-ci
Copy link

openshift-ci bot commented Nov 8, 2022

@jlebon: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/rhcos 42def12 link true /test rhcos

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@dustymabe
Copy link
Member

testing this now in a 4.11 build in the new rhcos pipeline!

@dustymabe
Copy link
Member

the 4.11 build failed to push the oscontainer with:

jsonschema.exceptions.ValidationError: Additional properties are not allowed ('base-oscontainer' was unexpected)

@dustymabe
Copy link
Member

the 4.11 build failed to push the oscontainer with:

jsonschema.exceptions.ValidationError: Additional properties are not allowed ('base-oscontainer' was unexpected)

Ignore this. The pipeline code shouldn't be trying to push that container for 4.11. We'll make a PR there.

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

@dustymabe
Copy link
Member

ci/prow/rhcos is going to fail because of warning: Could not find remote branch release-4.11-new to clone.

Merging...

@dustymabe dustymabe merged commit 7b35980 into coreos:rhcos-4.11-new Nov 9, 2022
@jlebon jlebon deleted the pr/backport-container-stuff branch April 22, 2023 23:34
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.

None yet

5 participants