NO-JIRA: Various patches to build extensions container using `podman build` in production by jlebon · Pull Request #1772 · openshift/os

jlebon · 2025-03-17T14:25:19Z

We're getting ready to build the extensions container in production as part of the layered node image work. To be able to build it using podman build directly and not via cosa build-extensions-container, a few adjustments are needed.

See individual commit messages.

jlebon · 2025-03-17T14:25:27Z

Requires: openshift/release#62894

jlebon · 2025-03-17T14:26:47Z

Should be merged together with coreos/coreos-assembler#4044.

ravanelli

LGTM

jbtrystram

/lgtm
/approve

Prashanth684 · 2025-03-17T16:52:08Z

-        --create-gpg-keys) create_gpg_keys; exit 0;;
        --) break;;
        *) echo "$0: invalid argument: $1" >&2; exit 1;;
    esac


so, do we still need the else block at all further down ,if we are only building with OPENSHIFT_CI=1 ? i.e when we are building with cosa? If we do, then the conditional below,

osname=$(source /usr/lib/os-release; if [ $ID == centos ]; then echo scos; fi)

also needs to accommodate the fact that ID can be scos.

Sorry, can you expand? Which else block exactly?

If we do, then the conditional below,

osname=$(source /usr/lib/os-release; if [ $ID == centos ]; then echo scos; fi)

also needs to accommodate the fact that ID can be scos.

Hmm, not sure I follow. In what scenario will the ID be scos? Currently that's only in the okd-c9s variant, which pretty much I think we should just nuke at this point.

Ok i think i understand now - i was talking about the block here, but i believe we need it for the tests? Now that the OKD images are layered and we don't need okd-c9s anymore, should this be changed to c9s as well?

Yeah, that block is still being used in the cosa path to build the base RHCOS images.

Now that the OKD images are layered and we don't need okd-c9s anymore, should this be changed to c9s as well?

I think it'll need to be reworked a bit yeah. Probably the cleanest actually is to just pass /usr/share/rpm-ostree/treefile.json since we're running from the node image which will inherit that from the base. And for the extensions manifest, probably we could auto-select between e.g. extensions-ocp.yaml and extensions-okd.yaml based on the node image we're building FROM?

that makes sense. I can take a look at this and try to do this.

dustymabe · 2025-03-17T17:27:03Z

 ARG VARIANT=""
 RUN if [ "${OPENSHIFT_CI}" != 0 ]; then ci/get-ocp-repo.sh --ocp-layer packages-openshift.yaml; fi
-RUN if [[ -n "${VARIANT}" ]]; then MANIFEST="manifest-${VARIANT}.yaml"; EXTENSIONS="extensions-${VARIANT}.yaml"; else MANIFEST="manifest.yaml"; EXTENSIONS="extensions.yaml"; fi && rpm-ostree compose extensions --rootfs=/ --output-dir=/usr/share/rpm-ostree/extensions/ ./"${MANIFEST}" ./"${EXTENSIONS}"
+RUN --mount=type=secret,id=yumrepos,target=/os/secret.repo if [[ -n "${VARIANT}" ]]; then MANIFEST="manifest-${VARIANT}.yaml"; EXTENSIONS="extensions-${VARIANT}.yaml"; else MANIFEST="manifest.yaml"; EXTENSIONS="extensions.yaml"; fi && rpm-ostree compose extensions --rootfs=/ --output-dir=/usr/share/rpm-ostree/extensions/ ./"${MANIFEST}" ./"${EXTENSIONS}"


Does this make the secret always required?

No, it's a no-op if the secret wasn't provided.

jlebon · 2025-03-17T22:09:50Z

/retest

jlebon · 2025-03-18T00:22:08Z

/retest

jlebon · 2025-03-18T16:50:15Z

/retest

jlebon · 2025-03-18T20:46:58Z

Latest push should fix the scos-9-build-test-* tests. The rhcos-9-build-test-* tests will fail until coreos/coreos-assembler#4044 merges.

It's not built anywhere but in that script currently. OKD doesn't care about this since they're targeting the layered model, and we already only build the c9s variant in the prod pipeline. So stop building this in CI and just instead build the c9s variant.

We want to be able to build this container directly via `podman build` in production, but the way this conditional is structured breaks things because we're always calling `get-ocp-repo.sh`, which is only useful when building in OpenShift CI. Really, the conditional there was not correct. We should check for `OPENSHIFT_CI` instead like in the main `Containerfile` for the node image, and set that same build arg in openshift/release. The `--create-gpg-keys` case was needed when building `okd-c9s` in CI, but we don't do that anymore, so drop it and drop the switch from the script since it no longer has a user.

I think that's implied but it fixes my editor's syntax highlighting.

Just like the main `Containerfile` for the layered node image, also support a `yumrepos` secret being passed in for injecting repo data. This will be used for building this container in production. (Again, like we do for the node image.)

Rather than repeating the `> /tmp/extensions.json`, just do it once by redirecting the overall output of the command list and pipeline. Although not strictly necessary by bash operator precedence rules, add parentheses for the inner `dnf repoquery | sed` pipeline because other- wise, one is left wondering what the operator precedence rules are.

Just like 6241505 ("Containerfile: add metadata in last layer of node image"), we want to make it easy for anyone but particularly ART to query metadata about the extensions container. Instead of generating `extensions.json` but assuming it's a bind-mount, directly put it in the final container image. In the cosa path, we'll keep propagating that info up to the build's `meta.json` to not regress, but now at least in the container-native flow, we also have that metadata accessible either by doing an `oc image extract` or a `podman run $img cat /path/to/extensions.json`.

Those aren't real packages so don't include it in the metadata to not add confusion.

This has been around for a while but isn't used anywhere AFAICT. Just nuke it.

It seems unnecessary to build it in both the `metal` and `qemu` path. Also this implicitly stops building it in the `c9s` tests, where it doesn't make sense.

jlebon · 2025-03-19T13:22:49Z

e2e-aws seems to be failing on infra issues I think.

/override ci/prow/e2e-aws

openshift-ci · 2025-03-19T13:23:17Z

@jlebon: Overrode contexts on behalf of jlebon: ci/prow/e2e-aws

Details

In response to this:

e2e-aws seems to be failing on infra issues I think.

/override ci/prow/e2e-aws

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-sigs/prow repository.

openshift-ci · 2025-03-19T13:23:20Z

@jlebon: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot · 2025-03-19T13:23:30Z

@jlebon: This pull request explicitly references no jira issue.

Details

In response to this:

We're getting ready to build the extensions container in production as part of the layered node image work. To be able to build it using podman build directly and not via cosa build-extensions-container, a few adjustments are needed.

See individual commit messages.

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 openshift-eng/jira-lifecycle-plugin repository.

jbtrystram · 2025-03-19T13:50:01Z

/lgtm

openshift-ci · 2025-03-19T14:00:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dustymabe, jbtrystram, jlebon, ravanelli

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dustymabe,jbtrystram,jlebon,ravanelli]

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

openshift-ci Bot requested review from cverna and marmijo March 17, 2025 14:28

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2025

ravanelli approved these changes Mar 17, 2025

View reviewed changes

jbtrystram approved these changes Mar 17, 2025

View reviewed changes

openshift-ci Bot assigned jbtrystram Mar 17, 2025

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2025

Prashanth684 reviewed Mar 17, 2025

View reviewed changes

dustymabe mentioned this pull request Mar 17, 2025

Various patches to build extensions container using podman build in production coreos/coreos-assembler#4044

Merged

dustymabe approved these changes Mar 17, 2025

View reviewed changes

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2025

jlebon added 8 commits March 18, 2025 19:58

extensions/Dockerfile: set default value for VARIANT

4bba6b7

I think that's implied but it fixes my editor's syntax highlighting.

extensions/Dockerfile: support yumrepos secret

4c29e4f

Just like the main `Containerfile` for the layered node image, also support a `yumrepos` secret being passed in for injecting repo data. This will be used for building this container in production. (Again, like we do for the node image.)

scripts/generate-metadata: skip gpg-pubkey rpmdb entries

eb0b37f

Those aren't real packages so don't include it in the metadata to not add confusion.

ci/prow-entrypoint: drop "build"/"init-and-build-default" verbs

4a9510d

This has been around for a while but isn't used anywhere AFAICT. Just nuke it.

jlebon force-pushed the pr/extensions-meta branch from 483278b to bf865b7 Compare March 18, 2025 23:58

ci/prow-entrypoint: build extensions in ocp-rhel-9.6 QEMU tests only

12e7354

It seems unnecessary to build it in both the `metal` and `qemu` path. Also this implicitly stops building it in the `c9s` tests, where it doesn't make sense.

jlebon force-pushed the pr/extensions-meta branch from bf865b7 to 12e7354 Compare March 19, 2025 02:16

jlebon changed the title ~~Various patches to build extensions container using podman build in production~~ NO-JIRA: Various patches to build extensions container using podman build in production Mar 19, 2025

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 19, 2025

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 19, 2025

openshift-merge-bot Bot merged commit f6eee15 into openshift:master Mar 19, 2025
14 checks passed

Conversation

jlebon commented Mar 17, 2025

Uh oh!

jlebon commented Mar 17, 2025

Uh oh!

jlebon commented Mar 17, 2025

Uh oh!

ravanelli left a comment

Choose a reason for hiding this comment

Uh oh!

jbtrystram left a comment

Choose a reason for hiding this comment

Uh oh!

Prashanth684 Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jlebon Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

Prashanth684 Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jlebon Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

Prashanth684 Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

dustymabe Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jlebon Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

jlebon commented Mar 17, 2025

Uh oh!

jlebon commented Mar 18, 2025

Uh oh!

jlebon commented Mar 18, 2025

Uh oh!

jlebon commented Mar 18, 2025

Uh oh!

jlebon commented Mar 19, 2025

Uh oh!

openshift-ci Bot commented Mar 19, 2025

Uh oh!

openshift-ci Bot commented Mar 19, 2025

Uh oh!

openshift-ci-robot commented Mar 19, 2025

Uh oh!

jbtrystram commented Mar 19, 2025

Uh oh!

openshift-ci Bot commented Mar 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants