Accept new image format, consume in controllerconfig, but stop there #3286
Conversation
Part of openshift/enhancements#1032 We'll add the new-format image into the payload alongside the old one until we can complete the transition. (There may actually be a separate `rhel-coreos-extensions` image e.g. too, so this is just the start) Note this PR is just laying groundwork; the new format container will not be used by default.
The extensions for our layered/bootable `rhel-coreos` images are no longer going to be contained within the image themselves. They will be a separate container. See: openshift/os#763 This makes sure that, if present, the new extensions image gets pushed through to controllerconfig and is available when merging machineconfigs.
The MCO orchestrates the management/setup of a big pile of different containers today, from `machine-os-content` to `haproxy` and configuring cri-o to use the correct `pause`/`pod` container. These images get set up at both "bootstrap" time and cluster time. In bootstrap today we manually scrape each image out of the CVO in shell script, and then pass them as CLI arguments inside `bootkube.sh`. This means adding new images requires a tedious "ratcheting" process where the CLI argument is added to the MCO, then we patch the installer to pass it in. Instead, add a new CLI argument which accepts a serialized imagestream object, which is exactly what the CVO carries. Prep for adding a new `rhel-coreos` image into the payload, so I can avoid that ratcheting process.
So once upon a time, our image references in our manifests, the ones that look like: `registry.svc.ci.openshift.org/openshift:something` probably pointed at real images. But: - that registry has long since retired and - the openshift client (`oc`) rewrites these values as though they were templated when an openshift release gets packed by `oc adm release new` This was not immediately apparent to someone who was not familiar with the process, and an outside observer could be forgiven for thinking that these were real values. This tries to alleviate any confusion or belief that these values are 'real' by making them obviously fake, but also descriptive of their function.
The way things work with the openshift client, once we add an image to image-references, `oc` expects it to be in the release, and `oc adm release new` will fail if it's not present. Also, in order for the placeholder image location in our `machine-config-osimageurl` configmap to get rewritten with a real value (rather than the bogus 'template' value it has by default), it has to be referenced in image-references. The desired outcome was "if it's there use it, otherwise ignore it", but that's just not possible with regard to the payload the way things are set up. This groups the addition of the new format os container and extensions container together into sort of an "on switch" commit that will break the builds if merged before https://issues.redhat.com/browse/ART-3883, but will make them work with new images if merged afterwards.
openshift-ci
bot
added
the
approved
| @@ -23,6 +23,10 @@ spec: | |||
| from: | |||
| kind: DockerImage | |||
| name: registry.svc.ci.openshift.org/openshift:machine-os-content | |||
| - name: rhel-coreos | |||
There was a problem hiding this comment.
I believe this needs to be rhel-coreos-8 now.
There was a problem hiding this comment.
possibly bad form on my part -- I assume you're going commit-by-commit -- but I pulled that out when I added the extensions container, and then added them back in in the commit at the end (because in the old PR, that was what broke images when we didn't have the ART images, and I thought we might want to peel that one off)
| - name: keepalived-ipfailover | ||
| from: | ||
| kind: DockerImage | ||
| name: registry.svc.ci.openshift.org/openshift:keepalived-ipfailover | ||
| name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:keepalived-ipfailover |
There was a problem hiding this comment.
Definitely in favor of this explicit placeholder!
|
@jkyros: all tests passed! 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. |
|
Nice, it works! /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, jkyros 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 |
|
It looks like this adds an explicit dependency on the |
This reverts part of openshift#3286 While we did land `rhel-coreos-8` in both CI and ART/nightlies, the corresponding extensions container is currently only in CI, not in ART/nightly builds. We're working on it in https://issues.redhat.com/browse/COS-1646 (Which will also need to have code changes from ART similar to https://issues.redhat.com/browse/ART-3883)
So...we may need to add a build-time conditional to the MCO for this. FCOS would probably just be
My tentative plan here is to add So far we've avoided the operating system naming problem by calling everything related to that |
|
I see, if the plan is to have multiple different images available eventually, this makes sense to me. |
When the CPO is at N and the NodePool.spec.release at N-1 we fail to render ignition payload because openshift/machine-config-operator#3286 broke backward compatibility. This PRs lets it fall back to MCO previously supported flags.
When the CPO is at N and the NodePool.spec.release at N-1 we fail to render ignition payload because openshift/machine-config-operator#3286 broke backward compatibility. This PRs lets it fall back to MCO previously supported flags.
When the CPO is at N and the NodePool.spec.release at N-1 we fail to render ignition payload because openshift/machine-config-operator#3286 broke backward compatibility. This PRs lets it fall back to MCO previously supported flags.
When the CPO is at N and the NodePool.spec.release at N-1 we fail to render ignition payload because openshift/machine-config-operator#3286 broke backward compatibility. This PRs lets it fall back to MCO previously supported flags. This commit ping the image used in the upgrade test to a no broken version.
As discussed in #3258, this:
rhel-coreos-8andrhel-coreos-8-extensionsimages to the MCO'simage-referencesfile so after this they will show up in nightliesOnce this is in, and the MCO can accept the new argument, we will need to:
And then once that is in, we can go back to #3258 and decide if/when/how we want the MCO to use the new format image by default.