Inject CoreOS stream metadata as configmap via CVO manifest #4760

cgwalters · 2021-03-16T17:35:13Z

Split out from #4582

This copies the bits from https://github.com/cgwalters/rhel-coreos-bootimages
which builds a ConfigMap out of the stream metadata and injects
it into the cluster.

We have an installer image in the release image today; this adds
the "is an operator" label, even though it's not really an
operator. We just want the CVO to inject the manifest.

Among other important semantics, this will ensure that in-place
cluster upgrades that have new pinned CoreOS stream data will
have this configmap updated.

openshift-ci-robot · 2021-03-16T17:35:26Z

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

cgwalters · 2021-03-16T17:38:41Z

OK so I believe that basically what this commit needs to do is what's in https://github.com/cgwalters/rhel-coreos-bootimages - basically. This would be the first time that the installer image in the release image is installing manifests. Does that make sense to everyone?

staebler · 2021-03-16T18:26:22Z

OK so I believe that basically what this commit needs to do is what's in https://github.com/cgwalters/rhel-coreos-bootimages - basically. This would be the first time that the installer image in the release image is installing manifests. Does that make sense to everyone?

My understanding is that we need the ConfigMap added under the manifests directory in the installer image that is included in the release.

The installer image may also need the io.openshift.release.operator=true label for CVO to consider its manifests.

staebler · 2021-03-16T18:27:21Z

Per discussion, the real fix here will probably require adding minimal CVO bits to the installer image.

Did you need the temporary fix in order to unblock something? Or can we do the real fix from the start?

cgwalters · 2021-03-16T19:12:05Z

Did you need the temporary fix in order to unblock something? Or can we do the real fix from the start?

I think this latest code will work - it was pretty easy to copy over the approach from https://github.com/cgwalters/rhel-coreos-bootimages .

If it does work we'll see it in the CI artifacts.

cgwalters · 2021-03-16T21:52:44Z

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_installer/4760/pull-ci-openshift-installer-master-e2e-aws/1371905428375474176/artifacts/e2e-aws/gather-extra/artifacts/oc_cmds/configmaps has
openshift-machine-config-operator coreos-bootimages 2 64m

🎉

staebler

Looks great. I just have a couple questions for things that are not clear to me, since operator manifests are a new experience for me.

staebler · 2021-03-17T00:37:57Z

manifests/coreos-bootimages.json

Would you happen to have a reference for what these annotations signify?

See
https://github.com/openshift/enhancements/blob/master/enhancements/update/cluster-profiles.md

Admittedly I cargo culted these from the MCO; we want this data in all of them I think. It's not directly relevant for e.g. single-node-developer but it also doesn't do anything and it's tiny.

staebler · 2021-03-17T00:38:29Z

manifests/coreos-bootimages.json

Is this something that is automatically updated by the CVO or by the release tooling?

It's replaced by oc adm release new yep.

staebler · 2021-03-17T00:41:57Z

hack/build-coreos-manifest.go

Is there some reason why you don't want to have the type here be corev1.ConfigMap? If it were, then we would get type safety later when filling out the data and not have to do any casting.

In the original repo I wasn't vendoring anything, it was basically just a git repo with a JSON file, so adding all the kube stuff to vendor/ felt like overkill.

But here, we're already vendoring, so indeed - fixed!

cgwalters · 2021-03-19T16:35:24Z

Any further thoughts on this?

This validates openshift/installer#4760 until we have an in-cluster consumer.

cgwalters · 2021-03-19T21:02:58Z

openshift/origin#25993 is a quick sanity test for this that can merge after this does.

This validates openshift/installer#4760 until we have an in-cluster consumer.

cgwalters · 2021-03-19T22:21:53Z

/test e2e-gcp
/test e2e-azure

staebler · 2021-03-20T01:23:34Z

Any further thoughts on this?

Sorry, I lost track of this. I'll take another look at this PR and the dependent PR early next week. From the last time I looked at it, it looked generally good, save for maybe a few nits.

Split out from openshift#4582 This copies the bits from https://github.com/cgwalters/rhel-coreos-bootimages which builds a ConfigMap out of the stream metadata and injects it into the cluster. We have an `installer` image in the release image today; this adds the "is an operator" label, even though it's not really an operator. We just want the CVO to inject the manifest. Among other important semantics, this will ensure that in-place cluster upgrades that have new pinned CoreOS stream data will have this configmap updated.

cgwalters · 2021-03-25T13:28:45Z

Rebased 🏄

staebler

Small nit that we can leave.
/lgtm

staebler · 2021-03-25T14:01:19Z

images/installer/Dockerfile.ci

 WORKDIR /go/src/github.com/openshift/installer
 COPY . .
 RUN hack/build.sh
+RUN go run -mod=vendor hack/build-coreos-manifest.go


Is -mod=vendor necessary here since we are using go 1.15?

staebler · 2021-03-25T14:03:15Z

/retest

openshift-ci-robot · 2021-03-25T14:03:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: staebler

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~~ [staebler]

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

cgwalters · 2021-03-25T19:41:23Z

/test e2e-aws-upgrade

openshift-bot · 2021-03-25T19:59:45Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-03-25T22:35:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-03-25T23:40:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-03-26T01:11:20Z

@cgwalters: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-azure	dd1326baddbfbe2aabaaf2b7b7a5e77e0c5d6aa9	link	`/test e2e-azure`
ci/prow/e2e-aws-workers-rhel7	`260b21c`	link	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-crc	`260b21c`	link	`/test e2e-crc`

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

openshift-bot · 2021-03-26T01:37:13Z

/retest

Please review the full test history for this PR and help us cut down flakes.

This validates openshift/installer#4760 until we have an in-cluster consumer.

Before this commit, CoreOS metadata was hard-coded in this Git repo and required manual maintenance to map OCP release versions to compatible AMIs. Now, the metadata is discovered automatically as part of the general release image metadata lookup, as the CoreOS metadata is part of the payload as of openshift/installer#4760. After this commit, HyperShift will only be compatible with OCP release images which embed the CoreOS metadata.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 16, 2021

openshift-ci-robot requested review from iamemilio and jhixson74 March 16, 2021 17:35

cgwalters mentioned this pull request Mar 16, 2021

Add and use stream metadata for RHCOS, add openshift-install coreos print-stream-json #4582

Merged

cgwalters force-pushed the stream-metadata-configmap branch from c32758d to bc01a06 Compare March 16, 2021 19:10

cgwalters marked this pull request as ready for review March 16, 2021 19:11

cgwalters changed the title ~~WIP: Inject CoreOS stream metadata as configmap~~ Inject CoreOS stream metadata as configmap via CVO manifest Mar 16, 2021

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 16, 2021

cgwalters force-pushed the stream-metadata-configmap branch 2 times, most recently from 4224981 to d8659d4 Compare March 16, 2021 19:24

cgwalters mentioned this pull request Mar 16, 2021

coreos-bootimage-streams: Standardized CoreOS bootimage metadata openshift/enhancements#679

Merged

staebler reviewed Mar 17, 2021

View reviewed changes

cgwalters force-pushed the stream-metadata-configmap branch from d8659d4 to dd1326b Compare March 17, 2021 13:46

cgwalters added a commit to cgwalters/origin that referenced this pull request Mar 19, 2021

tests/extended: Add a basic sanity test for CoreOS bootimages

a36be93

This validates openshift/installer#4760 until we have an in-cluster consumer.

cgwalters mentioned this pull request Mar 19, 2021

tests/extended: Add a basic sanity test for CoreOS bootimages openshift/origin#25993

Merged

cgwalters added a commit to cgwalters/origin that referenced this pull request Mar 19, 2021

tests/extended: Add a basic sanity test for CoreOS bootimages

3c87f33

This validates openshift/installer#4760 until we have an in-cluster consumer.

Prashanth684 mentioned this pull request Mar 22, 2021

data/bootstrap: Check if release image architecture matches host architecture #4592

Merged

cgwalters force-pushed the stream-metadata-configmap branch from dd1326b to 260b21c Compare March 25, 2021 13:28

staebler approved these changes Mar 25, 2021

View reviewed changes

openshift-ci-robot assigned staebler Mar 25, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2021

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 25, 2021

openshift-merge-robot merged commit 516bf4e into openshift:master Mar 26, 2021

cgwalters added a commit to cgwalters/origin that referenced this pull request Mar 26, 2021

tests/extended: Add a basic sanity test for CoreOS bootimages

43a626a

This validates openshift/installer#4760 until we have an in-cluster consumer.

cgwalters added a commit to cgwalters/origin that referenced this pull request Mar 26, 2021

tests/extended: Add a basic sanity test for CoreOS bootimages

5b62c32

This validates openshift/installer#4760 until we have an in-cluster consumer.

wking mentioned this pull request Apr 2, 2021

coreos-bootimages releaseVersion not substituted #4797

Closed

ironcladlou mentioned this pull request May 4, 2021

Dynamically discover release image OS metadata openshift/hypershift#205

Merged

cgwalters mentioned this pull request Mar 8, 2022

Fix create nodepool azure command openshift/hypershift#1118

Merged

4 tasks

djoshy mentioned this pull request Oct 31, 2023

Managing boot images via the MCO openshift/enhancements#1496

Merged

djoshy mentioned this pull request May 13, 2025

MCO-1504: Update bootimage management enhancement openshift/enhancements#1761

Merged

Inject CoreOS stream metadata as configmap via CVO manifest #4760

Inject CoreOS stream metadata as configmap via CVO manifest #4760

Conversation

cgwalters commented Mar 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 16, 2021

Uh oh!

cgwalters commented Mar 16, 2021

Uh oh!

staebler commented Mar 16, 2021

Uh oh!

staebler commented Mar 16, 2021

Uh oh!

cgwalters commented Mar 16, 2021

Uh oh!

cgwalters commented Mar 16, 2021

Uh oh!

staebler left a comment

Choose a reason for hiding this comment

Uh oh!

staebler Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

cgwalters Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

staebler Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

cgwalters Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

staebler Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

cgwalters Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Mar 19, 2021

Uh oh!

cgwalters commented Mar 19, 2021

Uh oh!

cgwalters commented Mar 19, 2021

Uh oh!

staebler commented Mar 20, 2021

Uh oh!

cgwalters commented Mar 25, 2021

Uh oh!

staebler left a comment

Choose a reason for hiding this comment

Uh oh!

staebler Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

staebler commented Mar 25, 2021

Uh oh!

openshift-ci-robot commented Mar 25, 2021

Uh oh!

cgwalters commented Mar 25, 2021

Uh oh!

openshift-bot commented Mar 25, 2021

Uh oh!

openshift-bot commented Mar 25, 2021

Uh oh!

openshift-bot commented Mar 25, 2021

Uh oh!

openshift-ci bot commented Mar 26, 2021

Uh oh!

openshift-bot commented Mar 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cgwalters commented Mar 16, 2021 •

edited

Loading