COS-3013: overlay node image before bootstrapping if necessary #8742
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
/test e2e-aws-ovn |
|
/retest |
Sweet! 🎉 Looks like e2e-aws-ovn-heterogeneous failed on a different issue. |
|
/retest |
|
/test e2e-agent-compact-ipv4 |
|
/test ? |
|
@zaneb: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
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. |
|
/test e2e-metal-assisted |
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/etc/systemd/system/node-image-pull.service
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
data/data/bootstrap/files/etc/systemd/system/node-image-pivot.target
Outdated
Show resolved
Hide resolved
|
Just surfacing some insights from internal discussions here. The AI test here is not using the ISO from The ABI tests here are not using the ISO from the |
It's odd to me that |
It's tightly coupled in ART builds because machine-os-images depends on the installer image. But upstream CI doesn't have any notion of dependencies between container images. The CI payload just contains the latest master build of every container image. So machine-os-images only gets rebuilt when a PR merges to it, regardless of any changes to the installer repo (let alone unmerged PRs in the installer repo). Arguably we chose the wrong ISO here, and in the event of a conflict between what rhcos.json says and what's in the payload, we should go with downloading the former so that we can CI changes like this. The assumption is that the payload is canonical, but that's not really true in CI, and CI is theoretically the only place they can differ. (I wonder if this would break disconnected CI tests though - @bfournie do you know? It'd also mean we'd be executing different code paths in CI than users are actually getting in reality.) |
This job is not required and expected to fail until #8698 merges. |
jlebon
force-pushed
the
pr/no-more-oc
branch
from
9f5f3ee to
d6b8f87
Compare
openshift-merge-robot
added
the
needs-rebase
|
OK, updated! Didn't rebase to make the diff easier, but also because I don't expect this to be merged before the associated enhancement is merged. But changes now include:
This works with Assisted Installer as well but requires a patch there. I'll open that one shortly. |
|
/test e2e-aws-ovn |
jlebon
force-pushed
the
pr/no-more-oc
branch
from
d6b8f87 to
fedeaad
Compare
openshift-merge-robot
removed
the
needs-rebase
|
OK, ended up doing a full rebase so try to get CI going. /test e2e-aws-ovn |
data/data/bootstrap/files/usr/local/bin/node-image-pull.sh.template
Outdated
Show resolved
Hide resolved
As per openshift/enhancements#1637, we're trying to get rid of all OpenShift-versioned components from the bootimages. This means that there will no longer be oc, kubelet, or crio binaries for example, which bootstrapping obviously relies on. To adapt to this, the OpenShift installer now ships a new `node-image-overlay.service` in its bootstrap Ignition config. This service takes care of pulling down the node image and overlaying it, effectively updating the system to the node image version. Here, we accordingly also adapt assisted-installer so that we run `node-image-overlay.service` before starting e.g. `kubelet.service` and `bootkube.service`. See also: openshift/installer#8742
As per openshift/enhancements#1637, we're trying to get rid of all OpenShift-versioned components from the bootimages. This means that there will no longer be oc, kubelet, or crio binaries for example, which bootstrapping obviously relies on. To adapt to this, the OpenShift installer now ships a new `node-image-overlay.service` in its bootstrap Ignition config. This service takes care of pulling down the node image and overlaying it, effectively updating the system to the node image version. Here, we accordingly also adapt assisted-installer so that we run `node-image-overlay.service` before starting e.g. `kubelet.service` and `bootkube.service`. See also: openshift/installer#8742
jlebon
force-pushed
the
pr/no-more-oc
branch
from
fedeaad to
81af230
Compare
|
Just to export some conversations we had with @mhrivnak about this. To summarize a few points:
|
|
I'll just add that bootstrap issues can be difficult to troubleshoot even today. With this new complexity, we should ensure that there's a clear and documented way to reproduce a customer's bootstrap environment in a lab. That should include:
|
I mentioned this in #8742 (comment), but the bootimage is specified by the payload as today. Of course, in some scenarios it's users that control the bootimage. The booted RHCOS version is visible in the journal logs but it's definitely not obvious. I've added a more direct journal log entry with that info now.
Thanks, this is good feedback. Added more logging in general from the new scripts. Let me know if that matches what you were thinking! openshift/release#60534 is enabling unconditional collection of bootstrap logs, which should make all this easier to see in CI too but we'll of course only see it as part of the PR that changes the bootimages. Edit: OK, just to give a better idea, here's a truncated bootstrap log to see what it looks like currently: bootstrap.log |
|
It's common for assisted-installer to boot a live ISO on systems before knowing which openshift version will get installed on them. That running live ISO will then be used for bootstrap-in-place once an installation is initiated. @eranco74 @carbonin do we need anything else to accommodate that, or are we all set? Will we grab these scripts and unit files out of the bootstrap ignition? @jlebon the main point otherwise is that we'll need flexibility to do the overlay on a variety of different versions of the coreos image. |
Sorry yeah, you're right. I actually knew this (I even referenced the related late-binding problem in the commit message), but forgot it when writing my previous comment. That's actually a perfect example of "there's no longer two kubelet and cri-o versions to think about, only one". I do have concerns though with not putting bounds on that skew to a range we can test and document. So I'd still argue for some version check as discussed in this ticket.
Yes. This is done by openshift/assisted-installer#899. |
|
|
||
| UNIT_DIR="${1:-/tmp}" | ||
|
|
||
| if ! rpm -q openshift-clients &>/dev/null; then |
There was a problem hiding this comment.
No, we don't install anything additional on the rootfs
|
@jlebon ah yes, sorry I forgot you did that for us! Thanks for that. The logging looks good to me. I think it'll help. |
|
/test e2e-agent-compact-ipv4 |
|
I've tested locally via dev-scripts using also openshift/assisted-installer#899 (and the related live boot ISO), and it went fine. |
|
/retest |
jlebon
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@jlebon: This pull request references COS-3013 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set. In response to this:
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. |
As per openshift/enhancements#1637, we're trying to get rid of all OpenShift-versioned components from the bootimages. This means that there will no longer be `oc`, `kubelet`, or `crio` binaries for example, which bootstrapping obviously relies on. Instead, now we change things up so that early on when booting the bootstrap node, we pull down the node image, unencapsulate it (this just means convert it back to an OSTree commit), then mount over its `/usr`, and import new `/etc` content. This is done by isolating to a different systemd target to only bring up the minimum number of services to do the pivot and then carry on with bootstrapping. This does not incur additional reboots and should be compatible with AI/ABI/SNO. But it is of course, a huge conceptual shift in how bootstrapping works. With this, we would now always be sure that we're using the same binaries as the target version as part of bootstrapping, which should alleviate some issues such as AI late-binding (see e.g. https://issues.redhat.com/browse/MGMT-16705). The big exception of course being the kernel. Relatedly, note we do persist `/usr/lib/modules` from the booted system so that loading kernel modules still works. To be conservative, the new logic only kicks in when using bootimages which do not have `oc`. This will allow us to ratchet this in more easily. Down the line, we should be able to replace some of this with `bootc apply-live` once that's available (and also works in a live environment). (See containers/bootc#76.) For full context, see the linked enhancement and discussions there.
golint was complaining about:
```
pkg/asset/ignition/bootstrap/common.go:406:2: ifElseChain: rewrite if-else to switch statement (gocritic)
if parentDir == "bin" || parentDir == "dispatcher.d" || parentDir == "system-generators" {
^
```
This is obsoleted now by `node-image-overlay.target` and its associated units which do a similar thing but applies to both OKD and OCP.
|
Updated this now! I've confirmed that containers/common#1758 is in RHEL 9.6, so we can drop the workaround for it. Also switched from Tested both a standard flow and an ABI flow with this. |
As per openshift/enhancements#1637, we're trying to get rid of all OpenShift-versioned components from the bootimages. This means that there will no longer be oc, kubelet, or crio binaries for example, which bootstrapping obviously relies on. To adapt to this, the OpenShift installer now ships a new `node-image-overlay.service` in its bootstrap Ignition config. This service takes care of pulling down the node image and overlaying it, effectively updating the system to the node image version. Here, we accordingly also adapt assisted-installer so that we run `node-image-overlay.service` before starting e.g. `kubelet.service` and `bootkube.service`. See also: openshift/installer#8742
|
/lgtm |
|
@jlebon: The following tests failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
openshift-merge-bot
bot
merged commit cdd054f
into
openshift:main
|
[ART PR BUILD NOTIFIER] Distgit: ose-installer-altinfra |
|
[ART PR BUILD NOTIFIER] Distgit: ose-installer-terraform-providers |
|
[ART PR BUILD NOTIFIER] Distgit: ose-baremetal-installer |
|
[ART PR BUILD NOTIFIER] Distgit: ose-installer-artifacts |
As per openshift/enhancements#1637, we're trying to get rid of all OpenShift-versioned components from the bootimages. This means that there will no longer be oc, kubelet, or crio binaries for example, which bootstrapping obviously relies on. To adapt to this, the OpenShift installer now ships a new `node-image-overlay.service` in its bootstrap Ignition config. This service takes care of pulling down the node image and overlaying it, effectively updating the system to the node image version. Here, we accordingly also adapt assisted-installer so that we run `node-image-overlay.service` before starting e.g. `kubelet.service` and `bootkube.service`. See also: openshift/installer#8742
* ops: add new FileExists method Prep for next patch. Also use that in one spot where we were manually calling `stat`. * overlay node image before bootstrapping if necessary As per openshift/enhancements#1637, we're trying to get rid of all OpenShift-versioned components from the bootimages. This means that there will no longer be oc, kubelet, or crio binaries for example, which bootstrapping obviously relies on. To adapt to this, the OpenShift installer now ships a new `node-image-overlay.service` in its bootstrap Ignition config. This service takes care of pulling down the node image and overlaying it, effectively updating the system to the node image version. Here, we accordingly also adapt assisted-installer so that we run `node-image-overlay.service` before starting e.g. `kubelet.service` and `bootkube.service`. See also: openshift/installer#8742
As per openshift/enhancements#1637, we're trying to get rid of all OpenShift-versioned components from the bootimages.
This means that there will no longer be
oc,kubelet, orcriobinaries for example, which bootstrapping obviously relies on.Instead, now we change things up so that early on when booting the bootstrap node, we pull down the node image, unencapsulate it (this just means convert it back to an OSTree commit), then mount over its
/usr, and import new/etccontent.This is done by isolating to a different systemd target to only bring up the minimum number of services to do the pivot and then carry on with bootstrapping.
This does not incur additional reboots and should be compatible with AI/ABI/SNO. But it is of course, a huge conceptual shift in how bootstrapping works. With this, we would now always be sure that we're using the same binaries as the target version as part of bootstrapping, which should alleviate some issues such as AI late-binding (see e.g. https://issues.redhat.com/browse/MGMT-16705).
The big exception of course being the kernel. Relatedly, note we do persist
/usr/lib/modulesfrom the booted system so that loading kernel modules still works.To be conservative, the new logic only kicks in when using bootimages which do not have
oc. This will allow us to ratchet this in more easily.Down the line, we should be able to replace some of this with
bootc apply-liveonce that's available (and also works in a live environment). (See containers/bootc#76.)For full context, see the linked enhancement and discussions there.