Add Change Management and Maintenance Schedules

[jupierce]

No description provided.

[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 jupierce. For more information see the Kubernetes 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:

• enhancements/update/OWNERS

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

[dhellmann]

I'm concerned about the complexity of the first 2 workflows. I think everything here will work as described, but having 2 separate operators in the system evaluating schedules and rules about what updates are allowed when seems more complicated than having 1 thing toggling the on/off flag for those components. Centralizing the schedule management means we only need 1 thing to track time, and everything else can just watch it's input API for changes, including to that on/off flag, to know what it needs to be doing at any given time. It also gives us the option of delivering that centralized thing separately (in an operator, ACM, OCM, etc.) or the owner of self-managed clusters providing their own implementation applying some rules we wouldn't want to have to support.

[dhellmann]

This feels like a bug in our existing upgrade status reporting.

[jupierce]

We're working to simplify the experience for operations teams on several fronts:

1. oc adm update status which will provide a single place to find detailed status information and SOPs.
2. Another enhancement I will be writing where node drain timeouts are an easily enabled part of a fully managed update.
3. This enhancement which:
4. Creates the foundation for control-plane and worker-node separation which will exploited by other oc commands to support diverse personas.
5. Makes this type of issue less disruptive. Even if a drain takes 12 hours, you don't have to sit there and watch it. When it finishes, if it is outside your maintenance schedule, your cluster will not be subject to any more disruption.

[dhellmann]

If those tools already exist, could we just take over maintenance of those tools instead of moving the implementation of the feature?

[jupierce]

I'm not an authority on its implementation, but I believe it has aspects spread across OCM, Hive, and MUO. I think the implementation would also differ for HCP.
Assuming we could take it over, it would not facilitate the separation of control-plane and worker-node updates since it only creates a window for an entire cluster update.

[jewzaam]

Some also in managed-cluster-config (MCC) which is plumbed through hive, and it's not a great experience to manage. We have to pick a set of verisons when a configuration can apply and then react when the cluster version is changed. We're in the process of reworking MCC so this is less painful but ultimately still is not a great experience to manage. Having the ability to just apply a change to a MachineConfig etc would be far better in my opinion.

[dhellmann]

This detailed workflow narrative is amazing!

[dhellmann]

We could add it there, though, right? Or as an OLM-installed operator?

[jupierce]

Yes, it could. The conclusion presented here was with guidance from Derek.

[dhellmann]

What is driving the request for this feature, managed services or users with self-managed clusters?

[jupierce]

It unfolded as follows (see motivation section for more depth on each point):

1. There is a negative perception of upgrades due to worker nodes being entangled with the end to end process. It also forces unnecessary maintenance on nodes and risks for customers.
2. We can eliminate those problems and empower users by separating the control-plane and worker-node updates.
3. @derekwaynecarr raised the concern that doing so would double operational burden. Also guidance that whatever improvements we make in standalone should be in HCP.
4. Maintenance windows were identified as a tool which could prevent new operational burden for customers who may not benefit from the separation. Maintenance windows are also a common configuration in other managed kubernetes solutions.
5. SRE, as part of all these conversations, indicates product level support for maintenance windows would help simplify their existing implementation.
6. Exploration of all other benefits of change management & scheduling which simplify operations.

So, item (1) was the genesis (particularly in standalone since SRE manages to insulate customers from gaps today), but we found additional value along the path.

[jewzaam]

To be clear on expectations, this includes the material change will not be applied if a node reboots for other reasons or a node is created (i.e. MHC replacement, scale up, new machineset / machine)?

[jupierce]

Good catch. I need to make this explicit -- there is no guarantee. The schedule assures that no material change is initiated, but the state of rebooted nodes / new nodes is dependent on the profile and state of rollout. For example, in standalone, if a new release payload is targeted by the CVO, but cluster change management is paused, the CVO will not roll out machine configs which ultimately inform new nodes joining the cluster. Thus, new nodes joining the cluster will receive the old configuration.
If, however, the MCP is halfway through updating old nodes when change management is paused, new nodes joining the cluster will receive the new configuration.

[jupierce]

Adding something like the following to the summary in the next push:

Change management enforcement _does not_ attempt to define or control the detailed state of the
system. It only pertains to whether controllers which support change management 
will attempt to initiate material change themselves. For example, if changes are paused in the middle
of a cluster update and a node is manually rebooted, change management does not define
whether the node will rejoin the cluster with the new or old version.

[dhellmann]

Can we make that more deterministic somehow? How hard would it be to apply a simple rule like "new nodes will always be deployed with the latest config" or "new nodes will always be deployed with a consistent configuration"?

[jupierce]

"new nodes will always be deployed with a consistent configuration"

That isn't hard. There will be a distinct moment where the MCS will start serving the new vs the old.

"new nodes will always be deployed with the latest config"

Since we need to implement this for multiple controllers, the safe bet is to leave that choice up to the implementation. Intuitively, any control loop can be paused, so, as written, the enhancement should be applicable across any controller. However, to provide this guarantee, we would need to say something like "no external sign of inconsistency can be visible until reconciliation is complete".

There is inherent friction with eventual consistency in that statement.

Consider the questions it raises in standalone:

• In a paused state with MachineConfig changes pending, there is a clear difference between "new" vs "old" config. Since the "new" is not incorporated into an MCP yet, new machines booted into the cluster must necessarily receive the old.
• Once the CVO unpauses and introduces a MachineConfig update, the MCP will be updated. If we pause at this point, what should a new machine receive? If you say "new", we've violated the proposed contract.
• Once the MCO has updated 75% of the machines, should it still be serving the old config? Is consistency more important than the inevitable prolonging of the update when those new nodes running old config need to be drained & rebooted? Do we start serving new config while they newish nodes are being rebooted (if we do, its possible that we will serve old config indefinitely if new machines constantly scale into and out of the cluster)?

There are detailed answers we could give here that would make the standalone update process subtly more predictable. But is CAPI going to implement this contract? Would ?

In short, I think all can reasonably insist upon is the ability to pause your progress towards consistency. If we try to define what the behavior of the system must be when we pause you, things get complex.

[dhellmann]

It sounds like the rule needs to be more complex than the simple examples I gave. It still seems like we want to be able to explain clearly to users what they are going to get, though. How about "the most recent config that was being applied to nodes before the MCO was paused"? That is at least predictable, even if it depends on the timing of the pause step. And to cope with the timing, we could say that new nodes don't get a config until at least 1 existing node from the pool is updated to use it.

[jupierce]

I can't find any documented contract for the behavior in the existing pausing primitives provided by MCP or CAPI, so I'll need to leave this for SMEs to weigh in on the what is possible here.

[jewzaam]

Some also in managed-cluster-config (MCC) which is plumbed through hive, and it's not a great experience to manage. We have to pick a set of verisons when a configuration can apply and then react when the cluster version is changed. We're in the process of reworking MCC so this is less painful but ultimately still is not a great experience to manage. Having the ability to just apply a change to a MachineConfig etc would be far better in my opinion.

[jewzaam]

How far into a maintenance schedule's window will an update event trigger? I.e. if something stops the upgrade of workers to start, maybe the operator doing this work is degraded, but it then recovers.

[jupierce]

How far into a maintenance schedule's window will an update event trigger?

Change can be initiated up to the last instant of the permissive window.

if something stops the upgrade of workers to start, maybe the operator doing this work is degraded, but it then recovers

I think of the implementation as pausing and resuming the reconciliation control loop(s). So relevant operators might go degraded, but only for reasons they go degraded prior to the introduction of change management.

[jewzaam]

Behavior for upgrades today is we will cancel if it takes too long to start. Would need to find out what "too long" actually is. We'll need to be careful about how this is communicated when we switch over to using this for managed. More thinking about the duration for control plane upgrades. I know we don't guarentee how long it will take, but perhaps something should consider how long those upgrades take and estimate if it would roughly fit into the "window" of the schedule.

[jupierce]

Control plane averages about an hour. Are you thinking that OCM might want to prevent users from scheduling less than say, 2 hours to prevent their expectations from being violated?

[jewzaam]

For control plane I wouldn't set an expectation on an end time, just allow scheduling the start. Having an end implies an ability to stop the upgrade. This is not the intention of this EP.

[jewzaam]

If we (SD) need to put something in place to manage this it's going to mean keeping some code somewhere. Today scheduling is via OCM. If this enhancement is accepted this becomes a platform capbility. We (SD) have not talked about the implications of such a feature on how we enable customer to manage upgrades. Can we see a world where SD gets out of the business of managing schedules completely at some point in the future? If so, having the right controls in place to ensure sane configurations will generally be a good idea.

That being said, we only care about the version of the cluster in a few cases.

1. if the cluster version is end of life (EOL)
2. if the cluster's control plane is running on RH SRE infra

In the #1 scenario, the cluster goes into Limited Support.
In the #2 scenario, it only applys for HCP toploogies where the pods of the control plane are running on RH infrastructure. SRE will force a control plane upgrade if the control plane version for signifigent bugs that need remediated, there is a vulnerability we need to remediate, or the control plane version is now EOL.

[jupierce]

Can we see a world where SD gets out of the business of managing schedules completely at some point in the future?

Yes. SD could let customers fully define their own maintenance schedules and simply override them when necessary (e.g. using disabledUntil). The reason SD might want to play a larger role in constraining the setting is to make sure, when necessary, that forced updates are performed within a customer's desired window. Your #2 scenario, for example: you have to do the update and ideally want users to be OK with when it happens. If you force them to choose a non-zero change management window each month, hopefully they are less surprised when forced changes happen during that window.

[jewzaam]

Why encode meaning into negative values vs having explicit state captured in another field? This can work of course but is mixing use: time until permitted change and status (paused).

[jupierce]

My motivation was to simplify the promql necessary for useful alerting by an ops team. A simple rule might be warn "Invalid update schedule" when seconds_until_update < 0 or seconds_until_update_permitted > 60 days vs enumerating booleans for why things might be busted. SD would certainly know best here, so I'm open to alternatives. I just want any rule to survive the introduction of new error conditions.

[jewzaam]

From the observability perspective it does reduce cardinality to use the value to encode the state. As noted it will work. It gets a bit brittle when you now have a mapping through various layers and have to know that encoded meaning in the metric value.

[jewzaam]

We should have skew protection (whatever it's called) disabled because we'll force a minor version upgrade of control planes in ROSA HCP when the control plane version is EOL. Validation of allowed skew is possible at other layers like OCM/CS, not sure what we permit at this time.

[jewzaam]

I don't expect SRE will override the schedule in this scenario. The change would be staged and customer notified of options. Customer can either wait for their next update window or the customer can permit the change to progress.

[jewzaam]

SRE have a couple of options and it depends on if it's HCP or not.

If HCP and the issue is signifigent enough, yes we would stage the update and force the change to land regardless of the customer maintenance schedule.

For Classic, where control plane is in customer's account, there is an important question to answer: does this impact SRE's ability to provide support and/or impact SLA with customer?

If yes, there is a support and/or SLA impcat, then SRE would do the same as with HCP and pushed the upgrade through.

If no, the change would be staged and customer schedule will pick it up on the next update. Customer will be notified and has an option to update earlier. Customer can defer the update as usual.

[wking]

MachineConfigPools consume inputs from many places, and ClusterVersion updates are only one aspect. https://amd64.ocp.releases.ci.openshift.org/ -> 4.16.0-ec.4 -> aws-ovn-serial -> Artifacts -> ... -> gather-extra artifacts:

$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-release-master-nightly-4.16-e2e-aws-ovn-serial/1767648317749071872/artifacts/e2e-aws-ovn-serial/gather-extra/artifacts/machineconfigs.json | jq -r '.items[].metadata | .creationTimestamp + " " + .name' | grep rendered-worker- | sort
2024-03-12T20:45:43Z rendered-worker-9724b3c3c79d2e58b093eb46d0c2c3ff
2024-03-12T20:51:58Z rendered-worker-33a6a11e659aca048b955f168eb770f3
2024-03-12T20:52:04Z rendered-worker-3e2eab9d46a1db0948347f34d268ea61
$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-release-master-nightly-4.16-e2e-aws-ovn-serial/1767648317749071872/artifacts/e2e-aws-ovn-serial/gather-extra/artifacts/machineconfigpools.json | jq -c '.items[] | select(.metadata.name == "worker").status.configuration.source[]'
{"apiVersion":"machineconfiguration.openshift.io/v1","kind":"MachineConfig","name":"00-worker"}
{"apiVersion":"machineconfiguration.openshift.io/v1","kind":"MachineConfig","name":"01-worker-container-runtime"}
{"apiVersion":"machineconfiguration.openshift.io/v1","kind":"MachineConfig","name":"01-worker-kubelet"}
{"apiVersion":"machineconfiguration.openshift.io/v1","kind":"MachineConfig","name":"97-worker-generated-kubelet"}
{"apiVersion":"machineconfiguration.openshift.io/v1","kind":"MachineConfig","name":"98-worker-generated-kubelet"}
{"apiVersion":"machineconfiguration.openshift.io/v1","kind":"MachineConfig","name":"99-worker-generated-registries"}
{"apiVersion":"machineconfiguration.openshift.io/v1","kind":"MachineConfig","name":"99-worker-ssh"}

Instead of having it implicitly assumed that ClusterVersion is going to be the controlling resource taking precedence over local MachineConfigPool configuration, maybe changeManagement could explicitly declare that with references to higher-precedence managers?

Or, because that would make the system even more complicated, how likely is it that folks are creating new MachineConfigPools without looping in cluster lifecycle administrators? If cluster lifecycle administrators cannot agree amongst themselves, that is not a problem we can solve with tooling. If they do agree, but want unified control over the policy, I'm not as down on UpdatePolicy CRD as the current alternatives-section is, and possibly there's a way to mitigate the concerns around that while still avoiding implicit or explicit hierarchy complications.

[jupierce]

There were two motivations to the current proposal:

1. It is intended to be implementable with existing CAPI primitives. CAPI exposes "paused" on the two main resources Cluster and MachineDeployment. My understanding is that pausing the Cluster pauses the MachineDeployment but not vice versa. This should allow the HyperShift operator to fulfill the simple changeManagement hierarchy by pausing and unpausing these resources. If the enhancement calls for independence of changeManagement stanzas, we would need to be confident CAPI would accept a significant paradigm shift. I would only feel compelled to do that if we thought there was a significant advantage.
2. The guiding principle of "simple things should be simple." Setting something globally, once, and not having to keep it in mind for an unrelated activity reduces the mental overhead associated with the product. A diligent enough team can always solve for with policy / clever automation / etc. But the more we ask this theoretical team to solve, the less pleasant the product is to use. "Cluster lifecycle administrator" is often going to be a single person trying to do lifecycle management along with everything else. Ideally, the enhancement will improve their QOL while also being flexible enough for plausible complicated use cases.

[jharrington22]

Should we re frame this section to combine more of what is in "Node Update Separation to Address Problems in User Perception" as the case here is more to give customers control on when to update worker nodes due to possible interruptions to their workloads.

[jharrington22]

Typically for managed OpenShift we will provide the customer the ability to schedule this.

[sdodson]

That action specific scheduling would go away once this is implemented or just get layered on top?

1. pid limit MC change goes at HH:MM mm/dd/yyyy
2. change window opens after above time

[jharrington22]

We would remove our implementation and replace it with a maintenance schedule.

[jharrington22]

For worker nodes this would be a valid strategy for a customer. At some point they will be notified that they must upgrade if control plane version skew is too far miss aligned and the running control plane version is going end of life.

[jharrington22]

This is interesting as SRE would have to ensure that the disabledUntil window is long enough to update all nodes. Typically we don't apply fleet wide changes like this immediately, we configure the rollout so they are applied after the next z-stream update.

[jupierce]

Right - this would be for emergency purposes only. machineconfiguration.openshift.io/bypass-change-management is another option if you didn't need to announce an end of maintenance guarantee to customers.

[JoelSpeed]

I think this is a contender for longest EP, congrats 🎉

I've left a bunch of comments through this, and, from an API perspective at least, have a number of questions about the UX and some comments about it being a separate, referred resource. It's only as I got to the end that I see this is an alternative already discussed. I've got more homework now on seeing what prior discussion there was about this, my current view though is that embedding it within the object is going to create some awkward duplication that could potentially be avoided. Feels like there's a line though on where to separate

This definitely needs review from the MCO folks, there's significant overlap with a project they are currently working on, we need to make sure to de-duplicate the efforts.

There's also some assumptions about how the MCO works that I believe are incorrect, and some of the design may need to be revised to accommodate this

CC @sinnykumari @yuqi-zhang I think you both ought to review this one

[JoelSpeed]

How does this work for MachineConfig in HCP, since they make sense to be cluster scoped, but not in the case of HCP? This feels the same

[jupierce]

MachineConfigs serialized as configmaps in the hosted cluster namespace: https://hypershift-docs.netlify.app/how-to/automated-machine-management/configure-machines/ .

[JoelSpeed]

Eww 😅

I was expecting there to be a XYZ and a ClusterXYZ where XYZ is for managing the cluster you're on and ClusterXYZ is for managing guest clusters and is namespaced, naming could be confusing though 🤔

[jewzaam]

Not seeing where a forced drain would be configured. In a managed context we need have this as an option, if only for our own clusters. We can decide what default value to use (today it is 1 hour I believe).

[jupierce]

Forced drain is a separate EP that I hope will also reduce MUO: #1616 .

[sdodson]

There are parts I need to re-read to ensure I understand the details, but given a lot of my comments here have been pending for a while I'm going to go ahead and post them then come back for a second pass.

[sdodson]

@sinnykumari @LalatenduMohanty @csrwng I think cover first pass reviewers
Probably want to run this by @rhamilto even if we potentially decide that we don't want a web console UX around this in cluster. To me I think it'd be good to at least expose node maintenance schedules.

[sdodson]

That action specific scheduling would go away once this is implemented or just get layered on top?

1. pid limit MC change goes at HH:MM mm/dd/yyyy
2. change window opens after above time

[sdodson]

Sometimes there can be control plane updates without associated reboots, it's rare given we update the kernel every other calendar week, but it's possible that a z-stream update could update only pod scoped resources. We'd still consider that to be a material change, correct?

[yuqi-zhang]

As an added note, in practice I think I've only ever seen 1 z-stream update not have any OS changes. Changes to openshift-clients/hyperkube/crio/etc. happen pretty frequently so even if there were no kernel changes something in the OS gets rebuilt.

This does imply that we cannot use "did a reboot happen" as whether an update completed or not, but I don't think anything monitors for that.

[sdodson]

Whenever we say telemetry (What gets sent back to Red Hat) we're constrained by cardinality limits which would likely mean that we get one metric from MCO indicating the number of pools which are not currently in an Updated state. However metrics within the cluster could be more fine grained as proposed elsewhere in this doc, per pool and I'm fine with them including time since last fully reconciled and estimated time until next reconciliation whenever there are updates pending and it's not actively rolling out those changes.

[sdodson]

I don't believe we have those metrics today. We'd be designing them as part of this enhancement.

[sdodson]

Yeah, this would just be the administrator annotates (labels?) each node with the new desiredConfig. MCO and MCD handle the rest from there. Administrator is responsible for annotating nodes in order they want and rate limiting that labeling however they want.

[yuqi-zhang]

Sorry for the late review. Just wanted to add some points/questions from the MCO perspective

[yuqi-zhang]

As an added note, in practice I think I've only ever seen 1 z-stream update not have any OS changes. Changes to openshift-clients/hyperkube/crio/etc. happen pretty frequently so even if there were no kernel changes something in the OS gets rebuilt.

This does imply that we cannot use "did a reboot happen" as whether an update completed or not, but I don't think anything monitors for that.

[yuqi-zhang]

I have a somewhat similar question to Joel here. As I understand it, worker (or other custom pools) can set their independent changeManagements, and the global (cluster) one is AND'ed into it?

What if I wanted to have different cadences for my control plane and workers (e.g. control planes on Mon-Fri but not weekends, and workers on weekends but not Mon-Fri)?

And since master pool is separate, this also goes back to my earlier question on whether that's independent (i.e. the control plane nodes themselves being upgraded is a separate profile than the control plane components rolling out)

[yuqi-zhang]

The description here more matches with what hosted control planes are doing (except that HCP would spin up an entirely new node)

[yuqi-zhang]

I would like to call out that:

1. HCP has both replace and inplace upgrade nodepools, and while that shouldn't matter for maint. schedule, they do upgrade differently, and
2. HCP differs from standalone in cert management, where the certs there will trigger creation of new nodes, and no rotation will happen if the upgrades are paused (due to a maint. schedule), so you run more risk of certs expiring down the line (although I don't know if they actually have certificate rotation handling for all certs today)

[yuqi-zhang]

We don't really have a mechanism for this today, since all configuration change calculations are done via rendered ones, and we just do a merge of all base configs.

I'm not sure I fully understand this section. Basically any pool-level configurations on the master pool are allowed the same way as worker, but they aren't honored if there's an incoming upgrade? But any user changes turn us back into worker calculation? (where the CVO level maint. schedule and the master pool are union'ed?)

[yuqi-zhang]

I don't think that's safe, since disruption is not just reboots, but also node drains, etc. all of which you can specify via the policy. Besides, the MCO isn't that granular that it can break down a subset of MC changes to happen.

I think node disruption policy should only define how a change is acted on, but otherwise is gated by the maint. schedule like everyone else.

[yuqi-zhang]

I'd also like to double check how we manage slow updates/half updated pools vs blocked upgrades/failing nodes, etc.

It reads to me that we'll have to reconsider how we do status calculation with the new window, since half-upgraded pools will become the norm, which means we'll have to figure out how/when/what we consider errors, and add extra pool status/operator status to reflect some of this

[wking]

nit: there's no openshift-machine-config namespace; all the MCO components are in openshift-machine-config-operator. E.g. in a 4.17.0-ec.1 CI run:

$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-release-master-nightly-4.17-e2e-aws-ovn-serial/1806673110347288576/artifacts/e2e-aws-ovn-serial/gather-extra/artifacts/namespaces.json | jq -r '.items[].metadata.name' | grep -1 machine-config
openshift-machine-api
openshift-machine-config-operator
openshift-marketplace

[jupierce]

Thanks @wking - update in next push.

[wking]

With disabledUntil as a "I want to touch this despite the lack of formal maintenance window" access hatch, I think it makes sense to have that one down on the covered resource in changeManagement. But I'm less clear on why pausedUntil's "despite the usual maintenance window, I have reasons to want to go slowly for a bit" would be down in changeManagement. Maybe pausedUntil belongs up on ChangeManagementPolicy? Or maybe exclude already covers that use case at the ChangeManagementPolicy level, and we don't need pausedUntil at all?

[jupierce]

I wanted pausedUntil to be a knob even without a ChangeManagementPolicy defined. It is in the resource itself because I believe:

1. It offers a simple way for users to control roll-outs without having to get into the details of creating ChangeManagementPolicy definitions. I want them to be able to easily toggle pausing a rollout of an MCP without the rabbit hole of understanding ChangeManagementPolicy - that rabbit hole is only worth it for someone who wants a real MaintenanceSchedule.
2. Moving pausedUntil into ChangeManagementPolicy makes the meaning even more challenging to convey / interpret. For example, how would you interpret "pausedUntil: true" for a Restrictive strategy vs a Permissive strategy? A paused restrictive strategy sounds like it would be evaluated as permissive.
3. It permits CLI tools to pause and unpause rollouts regardless of whether an administrator has configured a ChangeManagementPolicy. I think this is critical in order to implement verbs like oc adm update worker-nodes start|stop|pause and ensure they have MCP granularity.

[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

[openshift-bot]

Stale enhancement proposals rot after 7d of inactivity.

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

Mark the proposal as fresh by commenting /remove-lifecycle rotten.
Rotten proposals close after an additional 7d of inactivity.
Exclude this proposal from closing by commenting /lifecycle frozen.

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

/lifecycle rotten
/remove-lifecycle stale

[jupierce]

/remove-lifecycle rotten
/remove-lifecycle stale

[jupierce]

/remove-lifecycle stale

[JoelSpeed]

With this, were you thinking with regards to having special commands built into the CLI that would allow pausing/disabling etc?

[sdodson]

That's what I'd assume, oc adm commands that layer on top of the APIs.

[jupierce]

Yes, exactly. A series of subverbs under oc adm update have been proposed for OTA: https://docs.google.com/presentation/d/1BtVSGWaG6LD0qye9BHPU5ssnEYTGPOlhHeogFfECUnk/edit#slide=id.g25513f80e3d_0_572 .

[JoelSpeed]

Can you add an explanation of why this is namespaced, for a standalone cluster it doesn't really make sense to be namespaced does it? Depending on which API server this is on in HCP, it could also make sense to be cluster scoped, I'm guessing it's on the management side though and that's why we want it to be namespaced

[sdodson]

Also does RBAC get nasty here? The policy would have to be in the namespace of the controller that adheres to it or be granted permissions to read from other namespaces.

[jupierce]

Yes, they are on the management side for HCP and that drove the namespace decision. Adding a note to this effect in the next push.

[JoelSpeed]

Did we decide that each resource should be able to select just a single policy, or did we want to be able to select multiple policies?

[jupierce]

Described a bit later - manageable resources can refer to only one policy. Permitting multiple means returning to the non-deterministic calculation of overlapping intervals. Something I'm happy to avoid with a single policy.

[JoelSpeed]

When we think of resources being namespace, it is often because we want different actors to act upon them, and so namespacing them gives us that control and allows isolation between different actors. Since most (all?) resources in a standalone cluster that will have changeManagement stanzas are cluster scoped, it makes me think the actor on these resources should have cluster permissions and therefore this resource probably should also be cluster scoped?

[jupierce]

The actor would be namespaced for HCP. So unless we want to have a namespaced (HostedChangeManagementPolicy?) and non-namespaced version of the CRD, namespaced was my compromise.

[JoelSpeed]

A separate, namespace scoped version could be achievable, it may require some tooling improvements to make it easier to manage, but, nothing is impossible right?

[JoelSpeed]

I spoke with David about this recently, and I think, that you will need a different set of fields for the HCP use vs OCP use. And that actually, a ChangeManagementPolicy and HostedChangeManagementPolicy, while creating toil, would likely be the cleaner approach from an API perspective

[sdodson]

you will need a different set of fields for the HCP use vs OCP use

Anything more on how the fields would be different? This is just the policy definition right?

[JoelSpeed]

So typically this would be to do with references, if the resource references a secret, or a configmap, or some other object, typically a namespaced resource does not include a namespace field in the reference, as the referenced object must always be in the same namespace. For a cluster scoped, you have to specify the namespace.

In this API, it's different though. We don't have any namespaced references inside the API itself, so it would be the same spec and status needed, vis-a-vis some minor documentation differences.

The issue though is on the binding side. At the moment, the expectation is that the binding is by referencing the ChangeManagementPolicy from objects like MachineConfig, which is cluster scoped. So what do we do with that reference?

It would be the one that needed to be different, at which point, we are creating way too much complexity IMO.

Since MCPs don't exist in HyperShift, perhaps this isn't actually an issue though 🤔 The nodepool is namespaced and wouldn't therefore need a namespace field in the reference

[jupierce]

Thanks @JoelSpeed - I've separated HostedChangeManagementPolicy (namespaced and for NodePool/HostedCluster) and cluster scoped ChangeManagementPolicy (for anything internal to the cluster on which the resource is defined).

[JoelSpeed]

Assuming a new version comes along later, v3, will the MCO recognise that 10 are on v1, 10 are on v2, and update those on v1 first, or will it be random? Then you might end up with a mix of v1, v2 and v3?

What is our reporting like for understanding which version a Node is on? I could imagine a situation where we get a bug and the customers says yes, cluster is on version X.Y.Z, and we can see a bug there that was fixed in say X.Y.Z-1, but their nodes are mixed across several versions prior to that, how do we make it clear that the node that broke in this instance is on an unpatched release and therefore the customer needs to upgrade to fix the bug?

And that said, what if there's a change that affects the rendered config, that also relies on controllers being updated, or vice versa, how do we handle skew there 🤔

[JoelSpeed]

Does the API not already have ways to bypass the change management? Why not leverage those? Or are we now looking granularly at the MachineConfig level? I think we'll have to square that with the MCO team as I don't think that's a feature they have today

[jupierce]

There are several ways to bypass change management. This mechanism was a way to make "control-plane as a single material change" less of a snowflake (i.e. it happens as a single material change because of a standard annotation vs just being hard coded).
It also meant to give SRE/ops some guarantees for critical machine config updates without having to guess at a "disabledUntil" date. I will add a note to that effect to the enhancement.

[jupierce]

Rolling out critical machine config changes for worker nodes also is made easier with this annotation.
Instead of, for example, trying to predict a disabledUntil date, an SRE/operations team can use this
annotation to specify their goal with precision. Compare this with disabledUntil, which
(a) an operations team would generally overestimate in order to ensure that updates complete and
(b) may cause subsequent, non-critical, machine configuration changes to cause further workload disruption
(node reboots) that are unwarranted.

[iamkirkbater]

Is there a differentiation between standalone and HCP needed here? If I understand correctly, a control-plane upgrade on a HCP would not require rebooting nodes specifically, but the pods for that HCP. Or am I just splitting unnecessary hairs here?

[sdodson]

I'd consider control plane updates to be a material change whether or not it's standalone or HCP, not because pods or nodes are restarted, but because new code or config gets deployed and behavior will change. That's why we seek to be able to confine HostedControlPlane, ClusterVersion, MachineConfigPool, and NodePool updates to the permissive periods in a given schedule.

Probably splitting hairs too, but would a rolling reboot of nodes with no change be considered a material change? Maybe not for control plane nodes where things are well tuned for zero disruption, including CI tests that monitor for disruption, but user workloads are often less tolerant of rolling restarts. Either way, if initiated via oc adm reboot-machine-config-pool they'd both be confined to permissive windows because this actually applies a benign MachineConfig change and the MCO would observe any defined schedule.

[andreaskaris]

From the names disabledUntil and pausedUntil, it is not clear what each of them actually disables or pauses. The terms "disable" and "pause" are too similar and insinuate that disable is the stronger version of pause; however, for this proposal, they would be the exact opposite operation.

With the current naming scheme, I could easily see an admin who wants to set pausedUntil: true because they urgently want to stop a problematic update. Instead, they confuse the 2 terms, and set disabledUntil: true which effectively does the exact opposite of what the admin wants

[zanetworker]

@enxebre @csrwng @sjenning @celebdor Please review.

[openshift-ci]

@jupierce: 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.

[JoelSpeed]

Would daysOfWeek be clearer/more explicit?

[JoelSpeed]

Maybe datesOfMonth?

[JoelSpeed]

On another thread, mentioned this may be confusing, perhaps

day:
  on:
  - weekOfMonth: ...
    dayOfWeek: ...

[JoelSpeed]

The namespace would be the namespace of the hosted cluster right? Rather than being a concrete known namespace now, perhaps we want to change this to

Suggested change

	namespace: openshift-machine-config-operator
	namespace: <hosted-cluster-namespace>