AGENT-1250: Simplified cluster operations for disconnected environments

[andfasano]

/draft

This patch contains the enhancement proposal for the simplified cluster operations in the disconnected environments.

[openshift-ci-robot]

@andfasano: This pull request references AGENT-1250 which is a valid jira issue.

In response to this:

/draft

This patch contains the enhancement proposal for the simplified cluster operations in the disconnected environments.

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.

[andfasano]

/assign @zaneb

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from zaneb. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[zaneb]

We'll need to figure out how to map these names to the temporary registry.

It's also essential for security reasons that releases here are referenced only by digest and never by tag,

[zaneb]

I'd say all 3 of the alternatives from the discussion doc are worth documenting as rejected alternatives:

• the initial version of this proposal, before we realised that we would have to expose the local registries via api-int.
• the version using PinnedImageSet
• the idea of writing a registry that exposes the containers-storage as a registry

[jlebon]

the idea of writing a registry that exposes the containers-storage as a registry

See also https://github.com/cgwalters/cstor-dist/.

[zaneb]

Yeah, I have chatted to Colin about that. Unfortunately there are some fundamental limitations to what is standardised in the OCI Distribution protocol that we concluded made this approach too high-risk for our purposes.

[JoelSpeed]

Where do the control plane nodes source the images from?

[andfasano]

Once copied, from the disk. Did you maybe mean the exact location on disk?

[JoelSpeed]

I think I meant, how does the control plane know where to copy the images from, I didn't see anywhere in the API where you configure "ISO is loaded to this mount point"

[sdodson]

This wasn't clear to me either.

[zaneb]

A temporary registry will be started when the ISO is mounted (a udev rule matching on a disk label triggers a systemd service that starts it). The MCD will know how to get the image from that registry - it could be e.g. a static pod with a Service that gives it a well-known URL accessible within the cluster.

[andfasano]

(sorry, the file rename due the markdownlint failures scrambled a little bit the latest comments, I will keep addressing the unresolved one in the next commits - hopefully no more file renaming 😥 )

[danielerez]

What is the suggested approach to extract the relevant 'release-image' from the ISO?
Or maybe since the ISO is constructed for a specific release image anyway, we can keep it transparent for users? E.g. apply a MachineConfig that starts the upgrade (and detects the right version internally)

[andfasano]

The assumption is that the user is going to previously download a specific OCP ISO version, so it should be a known info upfront, explicitly specified by the user

[sdodson]

This sounds completely disconnected from the graph, does that matter? Do you care that you can easily go from 4.20.100 to 4.21.0 and fall back in time by two years?

If we had OSUS here this would just work normally, and you can use symbolic names. But we don't have a registry so we don't have OSUS. We could at least verify that the current version is listed in the target release's list of previous versions and that would prevent the upgrading into an older version issue where we would've never tested such an upgrade and it will almost certainly have some regression if you upgrade to a considerably older version. Perhaps put the IRI resource into a failed state if the current version is neither the version of the IRI or in the list of previous versions?

oc adm release info 4.19.8 -o json | jq '.metadata.previous'
[
  "4.18.19",
  "4.18.20",
  "4.18.21",
  "4.18.22",
  "4.19.0",
  "4.19.1",
  "4.19.2",
  "4.19.3",
  "4.19.4",
  "4.19.5",
  "4.19.6",
  "4.19.7"
]

[zaneb]

I like that idea, although ideally we would apply that at the time of upgrading rather than at the time of copying the image into the local registry. Is this something we should build into CVO in the absence of OSUS?

[sdodson]

I've lost track of the specific validations the CVO does but I believe the way it's intended to work today is if you provide a symbolic version it has to be on the graph. As soon as you provide a specific pull spec (--allow-explicit-upgrade) the CVO assumes you know what you're doing, not that that's ever been documented explicitly. There's a lot of blurred lines between CLI side guards and what the API accepts.

@wking can you check me here? what protections are in place here and what do you think would be best way to keep them from making a mistake by potentially selecting the wrong mirrored image when expecting to upgrade? Maybe it's time to move all the client side protections into the ClusterVersion CR and CVO via a new interface?

[zaneb]

It would be great if we had symbolic versions. What I think we'd need to do that is a mapping of tags to sha digests that is cryptographically signed. If that existed it would be fairly easy to build into the ISO, but the infrastructure to set it up if it doesn't would be potentially daunting. Do you know of any existing artifacts that could act as building blocks here? A brief reading of sigstore docs suggests that it won't help.

[palonsoro]

FWIW: Some time ago, before OSUS and oc-mirror were around, when performing OCP4 upgrades on disconnected environments, they had to be done with --allow-explicit-upgrade and that did have the risks you are pointing out here of not properly obeying the graph. In that era, the message used to be that you had to be careful to follow the graph on your own or you risked to not be properly supported.

However, I'd prefer to have safeguard mechanisms like the ones you are considering. The older approach of "follow the graph" on your own is too risky.

[zaneb]

Arguably there's a high probability that most disconnected upgrades still follow this path and don't use OSUS, and that we should therefore punt this to a future enhancement rather than block on it now. (I agree that I'd prefer to have it, all things being equal.)

[danielerez]

Currently we simply use 'podman load' on the registry tar file.
Are you referring to the same flow?

[andfasano]

Not exactly. By placing directly the registry image in the (podman) additional image store there will be no need to previously load it (and being read-only, it could not be pruned)

[danielerez]

Hmm, so how is it simpler than 'podman load'? I mean, we would still need some service/script to place the image in the store, right?

[andfasano]

The assumption was to have it already available at build time, but I think you're right (for the live phase) as currently it doesn't seem to be (easily) possible to extend the ISO content, in such case we'll have to fallback to the podman load approach.
For what regards the installation phase instead (so after the rebootd), the Assisted Installer could directly copy it to the target node (as mentioned here for the backend data). If so I will rephrase this part to make it clearer

[djoshy]

It might be helpful to have a singular controller for orchestrating the copy process across the MCDs running on the control plane nodes. This makes it easier to do aggregate several node states into overall conditions and to report error modes at the operator level.

[zaneb]

This design was inspired by PinnedImageSet in the hope that much of that can be... if not reused, at least used as a template.
IIUC PinnedImageSet does have its own controller running inside the MCO. I don't think the language here was intended to exclude that, so it could probably be clarified slightly.

[djoshy]

Ah, I see. Agreed that we should clarify if PIS is going to be reused here.

They're specified at the MachineConfigPool level and the node level images are represented in the status of the MachineConfigNode objects. We should be able to leverage that. We'd need some sort of translation between InternalReleaseImage <=> PinnedImageSets; maybe at the control-plane level?

I'm also not sure if PIS currently works in bootstrap/firstboot mode, so that might be something I can try getting clarification from the team on(cc @yuqi-zhang if you have context here). The flow above hints that IRI manifest is added at the end of the installation, so perhaps the MCO translation piece is not relevant at bootstrapping time?

[yuqi-zhang]

Hmm, I guess I'm not too clear on this either. It reads to me that the install flow is completely handled by the agent installer (i.e. it will "set up" the registry and image content from the install iso on all the control plane nodes), and upgrade wise, that responsibility falls onto the MCD via specifications in IRI? Does IRI come into the install process at all through the worker nodes?

Re: upgrade, my read of the process outlined below is that:

1. the user plugs in a usb disk ("attach the extended ISO") to the first control plane node
2. the user writes IRI for that specific node, so that the MCD copies it
3. the user unplugs it, plugs it into the second control plane node
4. write IRI to the second node
   repeat until done for control plane, then trigger the upgrade for the whole cluster, where the 3 control plane node reads off itself and then have the workers read off of the control planes?

If that's the case, I think regardless we'd need some daemon process to handle the actual "copy". We probably also need some controller handling the central state though, to ensure the user has successfully completed all copies and write some status before they can upgrade.

[zaneb]

@djoshy it's not literally a PinnedImageSet, but I am saying that the architecture of PIS and any lessons learned is readily adaptable to the architecture of this. (i.e. it's a thing that has to run on every node, report back when it is done, and roll up status so we can see when the whole cluster is up to date.) Hopefully that can speed up the design because it is not uncharted territory.

It reads to me that the install flow is completely handled by the agent installer (i.e. it will "set up" the registry and image content from the install iso on all the control plane nodes)

This is how the Dev Preview prototype works now. The plan was indeed for this continue because that means we can make sure that all of the nodes have their images cached on disk before proceeding with the installation (although we don't yet do that).

It would certainly be advantageous if we could rely on the IRI and MCO even at install time to create the local registry, as it would avoid duplicating the implementation, and eliminate the risk of ending up with both files on disk due to the ones created by the installation not being managed by the MCO. We could monitor the MCN status and not let the install continue until it is complete on the first 2 control plane nodes, but we would have no way of verifying the third node (the one running assisted-service) after it reboots, although theoretically since we'd have two working copies in the cluster there'd be nothing to stop it from eventually converging. This would require us to run an accessible registry on the bootstrap node, and pull data over the network even though it was previously available locally (though this is the case for worker nodes and during upgrades anyway). Perhaps a middle ground is to pre-populate just the images, and let MCO set up the registry service. @andfasano WDYT?

Does IRI come into the install process at all through the worker nodes?

We're proposing that worker nodes will not have a local registry, and that the control plane will operate as an HA registry for all nodes.

the user unplugs it, plugs it into the second control plane node

No, the idea is that when it is plugged in to the first control plane node, we'll start a temporary registry that is accessible from all control plane nodes. So they'll all ~simultaneously pull the image from the node with the ISO attached.

I think regardless we'd need some daemon process to handle the actual "copy". We probably also need some controller handling the central state though, to ensure the user has successfully completed all copies and write some status before they can upgrade.

Agree.

[zaneb]

It would certainly be advantageous if we could rely on the IRI and MCO even at install time to create the local registry

On reflection, this wouldn't work for single-node 🙁

[andfasano]

Perhaps a middle ground is to pre-populate just the images, and let MCO set up the registry service. @andfasano WDYT?

If this means letting AS copying the registry data before the first reboot then yes, I think it'd be more convenient to let MCO setup the registry as soon as possible. The registry must be available at least before machine-config-daemon-pull.service and resolv-prepender

[palonsoro]

I'd have some questions and I also found a typo.

[yuqi-zhang]

Hmm, I guess I'm not too clear on this either. It reads to me that the install flow is completely handled by the agent installer (i.e. it will "set up" the registry and image content from the install iso on all the control plane nodes), and upgrade wise, that responsibility falls onto the MCD via specifications in IRI? Does IRI come into the install process at all through the worker nodes?

Re: upgrade, my read of the process outlined below is that:

1. the user plugs in a usb disk ("attach the extended ISO") to the first control plane node
2. the user writes IRI for that specific node, so that the MCD copies it
3. the user unplugs it, plugs it into the second control plane node
4. write IRI to the second node
   repeat until done for control plane, then trigger the upgrade for the whole cluster, where the 3 control plane node reads off itself and then have the workers read off of the control planes?

If that's the case, I think regardless we'd need some daemon process to handle the actual "copy". We probably also need some controller handling the central state though, to ensure the user has successfully completed all copies and write some status before they can upgrade.

[yuqi-zhang]

Thinking about what we should report here, I wonder if each individual status entry for each payload should have subfields around progressing/success/error/etc, instead of overall conditions, so it's easier to see which payloads are there, which are being pulled, etc.

Although since a user is unlikely to specify more than 2 payloads here at a time, perhaps we just report the most recent error? Would the MCD would be constantly reconciling all releases it expects to be on disk? Or just the latest added/removed one?

[palonsoro]

Would the MCD would be constantly reconciling all releases it expects to be on disk? Or just the latest added/removed one?

It should reconcile them all. Otherwise, mid-upgrade scenarios could potentially break if the images from the older release are not being properly synchronized for some reason but still needed because their component is not yet fully upgraded.

[yuqi-zhang]

In that case I think the status should have sub sections per release on the status. i.e. it would be nice to see how many nodes have successfully pulled each release, what errors there are, etc.

[palonsoro]

I fully agree @yuqi-zhang . If a problem of any kind arises, we would indeed need that degree of detail. Otherwise, we would only know that that "something failed somewhere".

[JoelSpeed]

So given this direction, what subfields will the release status have?

[andfasano]

In general I do see the the action of adding / removing a specific release as an atomic operation, with its (global) status reported here. For the detailed error, could be the previously proposed approach be reused?

[JoelSpeed]

Is it possible that a user is both adding and removing a release simultaneously? If so, and there are errors both in adding and removing those release images, how would that be reported at the top level?

I think adding some more status in this list with the status (perhaps even tallying which release(s) is(are) currently in use by CVO) of each image makes sense so that we can work out problems with the individual releases

These could just be a set of conditions for each release perhaps, InUse, Ready, etc

[sdodson]

This sounds completely disconnected from the graph, does that matter? Do you care that you can easily go from 4.20.100 to 4.21.0 and fall back in time by two years?

If we had OSUS here this would just work normally, and you can use symbolic names. But we don't have a registry so we don't have OSUS. We could at least verify that the current version is listed in the target release's list of previous versions and that would prevent the upgrading into an older version issue where we would've never tested such an upgrade and it will almost certainly have some regression if you upgrade to a considerably older version. Perhaps put the IRI resource into a failed state if the current version is neither the version of the IRI or in the list of previous versions?

oc adm release info 4.19.8 -o json | jq '.metadata.previous'
[
  "4.18.19",
  "4.18.20",
  "4.18.21",
  "4.18.22",
  "4.19.0",
  "4.19.1",
  "4.19.2",
  "4.19.3",
  "4.19.4",
  "4.19.5",
  "4.19.6",
  "4.19.7"
]

[sdodson]

Skopeo is pointed directly at the host service or does it also get an IMDS entry and skopeo thinks it's talking to quay.io? If it had an IMDS entry could the IRI controller be used, unaltered, in a connected environment because it would eventually synchronize from the origin?

[zaneb]

There's something to be said for that, and also something to be said for not doing that because users who are connected but bandwidth-constrained will hate us if we download data over the network when they already supplied it locally 😄

Either way, the answer needs to be made explicit in this proposal.

[sdodson]

They'd have supplied it locally via an IMDS pointed at their local registry, right? I assume there's a way to say never even attempt to reach the origin and fail instead?

[zaneb]

Sadly I don't think there is.

[JoelSpeed]

So given this direction, what subfields will the release status have?

[JoelSpeed]

How would a user configure the appropriate pull secretes that will be allowed to pull from this registry?

[andfasano]

Do we really need a user to do that? Auto-generating the credentials (during the registry setup) and eventually saving them into an openshift-config/iri-registry CM could be enough? I do not expect anyhow the user to have any specific need to access it (from outside the cluster), though.

[zaneb]

We'll probably need to document how the user can force the credentials to be rotated.
I imagine that at installation time we'll generate credentials and save them in a Secret, as well as add the auth data to the cluster's pull secret.
If the user wants to rotate they'll need to delete the Secret (MCO will automatically replace?) and manually update the cluster pull secret.

[JoelSpeed]

This sounds like an important workflow to document in the EP

[zaneb]

This is ambiguous.

We will certainly need an IDMS because the containers inside the release payload refer to quay images by digest, and we will need to get them from the mirror.

It doesn't follow from this that the nominal pullspec (the one the release image is referred to as in CVO) is using api-int, so the word 'so' is out of place here. It could be any of:

• quay.io with tag + ITMS (possibly has to be this to use the upgrade graph checking in CVO?)
• quay.io with digest (covered by existing IDMS)
• api-int with tag
• api-int with digest

We need to be explicit about which.

[zaneb]

We can look at ClusterVersion's status.desired.image to get the target version.
If an upgrade is in progress... presumably we have to look at status.history and include every release back to the last one with status=Completed? (Not looking forward to seeing the VAP code for this 😬)

Also how can we tell if the release is using the internal registry? Presumably we need to use api-int as the nominal pullspec to tell them apart from ones pulled directly from the Internet.

[zaneb]

'stop' here is systemctl stop + systemctl disable? Or is it 'roll out new MachineConfig without the service'?
If the former, how will we get it to stick without the drift detection ree-enabling it?

[andfasano]

From an implementation point of view, it will be handled by the IRI controller by deleting the local registry MachineConfig resource

[zaneb]

During an upgrade, MCO also needs to replace the registry container image with the one from the new release image.

[zaneb]

Suggest registering as part of this PR 🙂

[joelanford]

Add details of the OLM aspect of the installation here?

[andfasano]

Do you maybe mean reporting the list of what was exactly included in the current release bundle? If so I will sooner update the proposal with more details on this topic (see also this comment)

[andfasano]

cc @danmanor for additional comments

[pablintino]

Adding some minor comments. Tomorrow I'll give this a second read, but I think it's OK from a MCO point of view.

[pablintino]

nit/question: I'm a bit surprised of seeing conditions outside the status.conditions section. Despite it makes sense I'd think it's worth to double check if we have already other examples like this one. If this usage of conditions is fine, should we report a global condition indicating if the IRI, from a node perspective, is ready/not to the global ones?

[andfasano]

I went for the conditions as for the most flexible way to describe the story of each individual release bundle on a given node, useful especially for troubleshooting in case of issue. The alternative could be to switch to a more rigid model with a predefined set of fields. Initially I checked the PinnedImageSet example but it was slightly different.
Anyhow, by using the conditions approach, I wouldn't see the need of additional fields to report the global condition, as the IRI controller could look for a specific condition reason

[yuqi-zhang]

Perhaps to help differentiate we can tweak the naming

[yuqi-zhang]

Some more questions inline

[yuqi-zhang]

Perhaps to help differentiate we can tweak the naming

[JoelSpeed]

Maybe we want to think about the naming of these two fields more. Available to me is suggesting this is something I could use, and I'm not sure exactly what the second releases field is for.

If I've read correctly, availableReleases is the currently mounted release, and releases is the list of stored releases in the local registry, right?

[andfasano]

Yes. In particular status.availableReleases indicates the currently mounted and not yet installed bundles, and it's meant to provide an auto-discovery facility to the end user.

[JoelSpeed]

We may want to think of a name the better distinguishes this from releases that are already installed. mountedReleases?

Available to me is too close to "I can use this for my cluster now", just thinking about how we use the term available in other contexts

[andfasano]

maybe a more explicit installableReleases?

[pablintino]

Just my 2 cents. If we grab the idea from the CVO ClusterVersion CR, that already have the concept of candidates, maybe availableReleases would be more aligned. IMHO mountedReleases exposes the mount implementation detail of how the feature internally works.

[yuqi-zhang]

As I understand it, one of the ambiguity with the word "available" comes from the uncertainty whether it's "available for the cluster use" or "available to be pulled in, such that the cluster can use it"

So the argument for dis-ambiguity would be mountedRelease --- locallyAvailableRelease, although I feel like that wordplay might make it more confusing than it helps, since mounted could also be understood as "locally mounted" or similar

Alternatively, like Pablo mentions, the CVO considers the "available to be upgraded to, but not yet on the cluster" as available, so maybe following that pattern would be simpler (and we can clarify via godocs). Then we'd get availableRelease --- activeRelease/localRelease or something like that.

(I guess what I'm saying is I feel like neither field should just be releases to help differentiate, but not a strong opinion other than that)

[andfasano]

I'd tend to agree with @pablintino, as mountedReleases seems too technical also for me.

[openshift-ci]

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

[danielerez]

Is it just a blank image with the bundle.json file? What's the process of signing this container image?

[andfasano]

Currently yes, just the bundle.json file. I'm not aware about the specific details for the signing process, but I do expect we'll have to address it while building the OVE ISO in Konflux (I'd expect something similar to CVO, cc @bfournie in case he could have more details about it)

[danielerez]

So the benefit of this approach is basically the ability to precisly monitor the installation, right?
If so, I just wonder about the priority of such an effort. I mean, it sounds like the right direction, but it's a pretty major change. Do you think it's a mandatory change that should be handled first? Comparing to the current behavior, do we get any advantages other than allowing to fail more gracefully?
Just want to understand whether it's merely a nice-to-have initiative :/

[andfasano]

IMO it's not a nice-to-have feature but a critical step to make the installation workflow more robust and properly monitored.
Copying 40Gb+ of data takes a good amount of time on a baremetal instance (up to 1hr as per the current testing), and several things can go wrong during the process, with the potential risk of leaving the data uncompleted or corrupted (as already experienced in the 4.19 DP release) - and thus the user should get properly informed that everything went fine (or if some problem happened, to avoid having later issues during the joining process, usually very painful to troubleshoot).
During the installation phase the Assisted Installer is the current eligible actor, on each node, responsible to carry on all the required task (such as writing the RHCOS image), and monitoring it's already supported via communication with the Assisted Service, which could be used by the user (via openshift-install wait-for command, or via the local Assisted UI) for that. And in this way we could also ensure that, prior of rebooting a node to let it join the control plane, all the required data (for making it working) will be effectively available.

[openshift-bot]

Inactive enhancement proposals go stale after 28d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Mark the proposal as fresh by commenting /remove-lifecycle stale.
Stale proposals rot after an additional 7d of inactivity and eventually close.
Exclude this proposal from closing by commenting /lifecycle frozen.

If this proposal is safe to close now please do so with /close.

/lifecycle stale

[andfasano]

/remove-lifecycle stale