OTA-1339: Update Status API and Controller

[petr-muller]

This enhancement proposes a new API to expose the status and health of the OpenShift cluster update process. The API will be implemented as a new CRD managed by a new controller owned by the OpenShift OTA team. The enhancement aims to deliver the API early, backed by a simple controller, to gather feedback from potential API clients (internal and external) before investing effort into building consensus on a complete, robust, and modular implementation.

The enhancement is based on the early Update Health API Controller Proposal and Update Health API Draft docs and feedback gathered from them.

The value of the feature when implemented was validated through a 4.16 client-based prototype, and the proposal was driven by a prototype implementation of the controller.

The enhancement is accompanied by the API PR openshift/api#2012 which implements the described concepts.

[openshift-ci-robot]

@petr-muller: This pull request references OTA-1339 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.18.0" version, but no target version was set.

In response to this:

WIP

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.

[openshift-ci-robot]

@petr-muller: This pull request references OTA-1339 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.18.0" version, but no target version was set.

In response to this:

WIP: still need to finish some sections and incorporate feedback from previous rounds

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.

[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

[petr-muller]

/remove-lifecycle stale

[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 assign pratikmahajan for approval. 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:

• 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

[openshift-ci-robot]

@petr-muller: This pull request references OTA-1339 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:

This enhancement proposes a new API to expose the status and health of the OpenShift cluster update process. The API will be implemented as a new CRD managed by a new controller owned by the OpenShift OTA team. The enhancement aims to deliver the API early, backed by a simple controller, to gather feedback from potential API clients (internal and external) before investing effort into building consensus on a complete, robust, and modular implementation.

The enhancement is based on the early Update Health API Controller Proposal and Update Health API Draft docs and feedback gathered from them.

The value of the feature when implemented was validated through a 4.16 client-based prototype, and the proposal was driven by a prototype implementation of the controller.

The enhancement is accompanied by the API PR openshift/api#2012 which implements the described concepts.

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.

[andreadecorte]

This section talks about an HostedClusterUpdateStatus. In this proposal, is this resource expected to contain also the NodePool status, or there will be N NodePoolUpdateStatus, one for each NodePool?
I think it would be better to be clear on this point.
Option 2 is more aligned with HyperShift topology IMO

[petr-muller]

The UpdateStatus is more like a container for insights, there's exactly one for a cluster, so in HCP it needs to be namespaced and needs to have a different name - HostedCluster... here refers more to a "hosted cluster" as an entity, not HostedCluster as a resource.

The actual status tied to a resource that represents the cluster would be exposed through a status insight, so you'd have a HostedClusterStatusInsight (=HCP analogue of the ClusterVersionStatusInsight on Standalone), and also a NodePoolStatusInsight per each NodePool (=HCP analogues of the MachineConfigPoolStatusInsight on Standalone).

[andreadecorte]

Just to note that so far in my understanding is that HyperShift tries to expose all the feedbacks for the consumer either in the HostedCluster or in the NodePool resources. So exposing the UpgradeStatus in a dedicated CR for the user to consume is somehow a design change. But I defer to the Hypershift team for comments on that.

[petr-muller]

Yeah, that's true. We have a similar tension in Standalone, where the information is supposed to be exposed through ClusterVersion, ClusterOperator and MachineConfigPool resource status, but the APIs are not expressive enough, typically there's too much aggregation resulting in single kube Condition that ends up too high-level, and the amount of interpretation a client needs to do to determine the update status is quite high.

We are being asked by the BU for the Status API to be present in both Standalone and HCP, the current proposal is my best effort to bring the same experience to both form factors.

[andreadecorte]

Thanks, I agree, there is a limit on the aggregation we can do, especially for the upgrade use case, so it makes sense to explore this path for me as an API consumer. It can definitely help reducing the logic client-side,

[JoelSpeed]

here the information is supposed to be exposed through ClusterVersion, ClusterOperator and MachineConfigPool resource status, but the APIs are not expressive enough

Is there merit in extending these objects to improve their status if this is an issue?

[petr-muller]

There is absolutely merit in improving resource statuses, but I view this as a synergy with the proposal rather than being sufficient for the problems we're solving. Several arguments:

1. We cannot easily add new status information for Nodes
2. In some cases (upgrade...) the status is not really fitting the spec->status reconciliation over one resource. One second after CVO acknowledges an update, all COs and all Nodes essentially become "outdated" (from the user PoV at least!) but nothing has an idea about that, except for CVO. But even CVO does not know everything that happens during the update (especially, it does not know anything about Nodes). So who maintains the "Outdated=True" thing where?
3. General fragmentation of the APIs is a concern. Good status will still only be useful if either the user knows about the resource to inspect and the fact that it will expose a status. This is natural for kube enthusiasts and OpenShift experts, but our users are often not that. So you can take advantage of the fact that all these are APIs, so you can build a $thing that inspects and interprets all of them, but now you have logic, which partially encodes knowledge of the OCP as a system. And we have at least three places that need this logic (admin console, CLI, console.redhat.com).

We also have a bit of a Conway's law problem here IMO. We have subsystems for which individual teams are responsible, and they expose their status in some way. But we do not have anyone responsible for the status of the whole thing, and that means we are expecting our users to understand our components (or at least understand well that OpenShift is a distributed system made of components) when they are actually approaching our product as a "whole thing" and want to interact with it that way.

[JoelSpeed]

We cannot easily add new status information for Nodes

We have just implemented MachineConfigNode, which will provide a lot of new status about upgrades of Nodes, is there data missing from that which you think we may want to request is added?

So who maintains the "Outdated=True" thing where?

Does the CVO not have any status today to show how many COs are reporting that they are on an older version?

But we do not have anyone responsible for the status of the whole thing

This raises an interesting question, do you think we need to impose standards on the individual teams to make sure they expose their status in a consistent manner so that we can reduce that cognitive load for end users?

[petr-muller]

We have just implemented MachineConfigNode, which will provide a lot of new status about upgrades of Nodes, is there data missing from that which you think we may want to request is added?

I'm not sure. But generally my problem is not missing data, and if "We cannot easily add new status information for Nodes" would be the issue then I'd agree MCN is good solution alone.

Does the CVO not have any status today to show how many COs are reporting that they are on an older version?

No. It has a Progressing condition which in a string form can say something like "Update in progress, 48% complete. Waiting on machine-config." but it actually does not monitor COs all the time. It applies payload manifests in a structured way, and when it applies a CO one it waits for CO version to be bumped before going further, but that's it. It does not care at all about COs already updated or the ones that will be updated in the future.

This raises an interesting question, do you think we need to impose standards on the individual teams to make sure they expose their status in a consistent manner so that we can reduce that cognitive load for end users?

That would be a great improvement, yes. I don't know if its achievable. Its definitely not achievable fast or easily. The ideal state would be something like a distributed API with an unified surface. But right now we seem to have trouble even enforcing a shared understanding (not to say compliant implementation) of Available and Degraded on COs. It's hard, and it is not just a technical problem.

[wking]

nit: "requires immediate attention" might be too strong. For example, the impact of a PDB-protected workload blocking a Node drain might just be "Node drain doesn't progress until the PDB issue resolves", which might just be "wait for the PDB-protected CI run to wrap up". Can we soften to something like "that warrants attention by the cluster administrator" so we have room for things like PDB-vs-drain that sit on the border between "admin might care" and "admin might not care, but informing them is still expected to be acceptable signal-to-noise"?

[petr-muller]

Sounds good, addressed in 353a4b5

[wking]

nit: might be worth inlining or linking your your:

Adding this layer can be considered in the future if the proposed model is problematic.

here, so folks reading through the doc in order understand that you're deferring this layer for now, and not designing anything that would make it hard to add this layer if future folks decide it would have sufficient value.

[petr-muller]

Sounds good, addressed in 353a4b5

[wking]

For the bar to a new image, Product builds and becoming part of an OpenShift release requires an enhancement (which we have here :), and the approval of image names for mirroring. The latter requires the OpenShift mirroring approvers.

[petr-muller]

Thanks for the links! I added the root link to the doc. There is another angle to the separate image / CVO: there's just a single release payload, no separate ones for TechPreview etc. Which means the image (only used by a small subset of clusters) would be necessary to e.g. mirror to disconnected environments. So I propose to continue shipping via CVO in TP and consider extracting to a dedicated image as a part of GA transition criteria.

Addressed in 353a4b5

[wking]

Might be worth talking about the removal of the tech-preview client-side logic here?

[petr-muller]

Good point, added in 353a4b5

[wking]

Probably also worth pointing out that all the resources we're currently planning on monitoring are old, quiet v1 types, and are already in tier 1 support. So the chance that a CRD or one of its monitored properties goes away is pretty small.

[petr-muller]

Added in 353a4b5

[enxebre]

This is made more clear in the "Workflow Description" section and below, but can we maybe have a paragraph up here expressing why existing clusterVersion resource status is not enough to solve this problem and how this augments it?

[enxebre]

Can we clarify how does this scale to 500 nodes? Does this has builtin message aggregation heuristics?

[petr-muller]

This is an excellent point. Nodes are problematic, scaling-wise. Are you asking about the presentation, or about the underlying data? I'll address both.

Presentation (easier): Even the current prototype already only shows lines for "interesting" nodes and aggregates the rest. Full output example is here, snipping the relevant part here:

WORKER POOL   ASSESSMENT   COMPLETION    STATUS
worker        Degraded     37% (22/59)   44 Available, 5 Progressing, 12 Draining, 7 Degraded

Worker Pool Nodes: worker
NAME                                      ASSESSMENT    PHASE      VERSION       EST   MESSAGE
build0-gstfj-ci-prowjobs-worker-b-9lztv   Degraded      Draining   4.16.0-ec.2   ?     failed to drain node: <node> after 1 hour. Please see machine-config-controller logs for more information
... only "interesting" lines shown...
build0-gstfj-ci-tests-worker-c-jq5rk      Unavailable   Updated    4.16.0-ec.3   -     Node is marked unschedulable
...
Omitted additional 49 Total, 21 Completed, 44 Available, 5 Progressing, 28 Outdated, 5 Draining, 0 Excluded, and 0 Degraded nodes.
Pass along --details=nodes to see all information.

Underlying data (harder): As proposed there's a single insight for each Node, which is probably problematic (I already run into this in openshift/api#2012 when I had to write API constraints and Nodes cause by an order of magnitude larger (hundreds) amount of insights to exist in the system, which can cause scaling issues. I will propose, specifically for nodes, some aggregation for their status insights, because in usual cases most nodes will not be in an interesting state (most nodes will either be pending update or completed one).

[JoelSpeed]

Is this a problem with trying to make this a singleton? Would it make more sense if this were broken down into smaller units, that could then be filtered based on fields within them, eg you could skip listing any where the assessment was "EverythingIsAwesome" for example?

[petr-muller]

It's simply too many entities once we start counting nodes, I agree that we'll need to aggregate somehow, not sure of exact method yet (will probably depend a bit on other maybe-break-this-up decisions)

[JoelSpeed]

How would you quantify what an acceptable number of entities is? Yes, we expect customers may have hundreds of nodes, but if the majority of those are fine, and we provide appropriate tooling for filtering to those which are not fine, do we really have an issue with the count?

Also bear in mind that with the current approach, as you have hundreds of nodes, won't each one be represented somehow in the current API, exploding it?

I could imagine a top level API that says, hey, these particular list of insights might need your attention, and you take that list and go oc get insight <bad one> to get the deeper detail

[petr-muller]

Hundreds of nodes causing hundreds of node insights to exist, exploding the API is exactly the problem I meant here, and I'll need to think about how to mitigate it - I'm thinking about aggregating nodes that are in a similar state into a single insights. Going from potentially hundreds of insights, each with metadata like conditions, to lower tens of insights, where usually two of them ("nodes that are happily waiting to be updated" and "nodes that are happily done updating") would have a list of hundreds of names (still hundreds, but now just hundreds of strings instead of hundreds of full structs) sounds acceptable to me.

[enxebre]

Can we clearly express in a sentence who is the consumer of this API in the hcp context? is it any managed services powered by hypershift.openshift.io? is it the cluster admin?

[enxebre]

fwiw if there was a use case the hcp could push the namespaced UpdateStatus into the guest cluster as a cluster scoped UpdateStatus. I'd expect though there's no need for that and this is consumed by managed services which them choose how to present to the service consumer

[petr-muller]

I'd expect though there's no need for that and this is consumed by managed services which them choose how to present to the service consumer

+1

[enxebre]

How much of the initial builtin logic can be reused in an hcp cluster? Whereas having a namespaced resource gives the right level of flexibility, I'm concern that hcp ends up fully disconnected and reusability is completely lost implementation wise. Is there a plan for reusability (impl wise) of capturing well known insights?

[petr-muller]

It is hard for me to quantify. API-wise, I expect the USC to be able to manage both the standalone and hcp UpdateStatus variants (not at the same time ofc), driven by a flag or something. There will be minor differences in the data structures involved (mainly to be able to refer to resources in two api surfaces) but most code paths should be very similar and there is no reason for HCP to be disconnected. It is simply a piece that receives data that follows certain a contract and processes that into a UpdateStatus resource.

For the checkers... for progress (status) insights, I assume we will need separate controllers but mostly because the update procedures simply are implemented differently between HCP and Standalone, and it is hard to escape that. So far we have not found both informers we have (control plane and node one, for standalone) to be that complex. Some parts are definitely reusable, like some parts of node assessments, maybe alert monitoring. At least for the initial round everything lives in a single binary (=single repo), so similarities are easy to spot.

What can we do to avoid disconnected HCP? HCP support is part of what BU asks so we're not planning to ignore that. Would committing to start building the HCP bits as soon as the foundations are in place be enough? Starting to cover HCP soon (while still in TechPreview) would reduce the space for divergence.

[enxebre]

any chance to capture a driving use case in this proposal illustrated by concrete insights which would not possible with todays UX?

[PratikMahajan]

does this operand come up only when the update is requested or does it keep running ?

[petr-muller]

As proposed it is running continuously. Can be discussed as a drawback, I guess. Running continuously can be seen as a waste, there are options to mitigate:

• We can manage its lifecycle (switch it on for update and off when it is finished). Starting is easy, stopping not so much. We need to handle stuff like paused MCOs that get unpaused eventually, but we could actually read the API itself to know that USC is no longer needed to run :D. This would be a good raison d'etre for a cluster operator.
• USC itself can moderate its activity outside of update. It has watches and API writes, but it can probably ignore most cluster activity when it knows there is no update going on, and just check for signs of one starting, while doing some minimal maintenance of UpdateStatus resource.

I'm not sure if we need to do this decision in the proposal, sounds like somehting ot evaluate with evidence, so I'd propose to discuss this as a drawback / open question, and put a decision as a new TP -> GA criterion?

[petr-muller]

6332863 addresses this, lmk if it sounds reasonable to you

[petr-muller]

30f68e2 changed content related to web console. Web Console has resource discovery so we do not have to ship a plugin to accomodate presence/absence of the API, it can handle it. Therefore we can drop this from drawbacks (and keep just a brief mention about clients having to gracefully degrade in API absence), and discuss the necessary changes directly in the proposal (new Update UX in Web Console section).

[JoelSpeed]

Might read better as something a little more direct, less implicit

Suggested change

	* As a cluster administrator, I want to be clearly told if there are issues I need to address during the update so that I can do so.
	* As a cluster administrator, I want to be clearly told if there are issues I need to address during the update so that I can resolve the issues.

[JoelSpeed]

Generally mixing multiple concepts within a single API is a recipe for trouble. Small, well defined APIs that do one job are preferred. Why is it imperative they they are a single API type?

[petr-muller]

By multiple concepts you mean "status" and "health" here? I am not that hard on having a single API type for these two. I can imagine having separate containers for status and insights.

[enxebre]

isn't health a subset of status? can we define both?

[petr-muller]

I will add definitions for shared language.

Over time I started using "progress" in some places I used "status" previously, because the latter is so generic (and gets mixed up with kube notion of status).

• status/progress is data that communicate details about ongoing process - what parts have started, what parts are done, what remains to be done, how long are things taking, how long do we expect them to take
• health is whether the things go well and details for when not

[JoelSpeed]

I would certainly like to explore what having separate APIs for these would look like because yes, you talk further in the API about the insights, and how they relate to the progress of objects, and then have separate statements roughly equating to "but when it's a health related observation, it looks different", which signals to me that we are probably trying to cram two different types of data into one API, and with that, will end up with an API that works, but is not optimal for either of the two types of data we are trying to represent

[JoelSpeed]

Is this a problem with trying to make this a singleton? Would it make more sense if this were broken down into smaller units, that could then be filtered based on fields within them, eg you could skip listing any where the assessment was "EverythingIsAwesome" for example?

[JoelSpeed]

Given that the new MachineConfigNode is still under TP, have we considered whether the MCN is the best place for detailed node health information and whether this API should aggregate from those, and not perhaps duplicate that information?

[petr-muller]

Yeah I keep MCN in mind and I expect using this as a source of information for nodes.

[JoelSpeed]

Rather than copying information from MCN, have you considered just linking to MCNs for the details of the errors?

[petr-muller]

Not specifically for MCNs, but for other status insights we did - and there is a same principle applied to all of them. I try to interpret the state of the original resource, in the context of an update, knowledge of the whole system, and maybe taking into account the state of other resources, so that the consumer of the API does not need to understand the whole system, just the one surface we provide. And that interpretation carries with it a reference to the original structure so that it can be inspected in detail, when necessary.

There's value between the extremes of "blindly copy data" and "only refer to another resource".

[JoelSpeed]

What is the workflow for opting out at upgrade time? I didn't think that was possible

[petr-muller]

I think the following is possible:

1. You are using vCurrent capability set on 4.X which equals v4.X one, or one of the v.(1..X) ones
2. You see that 4.X+1 has a new capability you do not want, included in v4.X+1 so you'd get it if you update
   3a. You switch to v4.X capability set and update. New capability is not enabled (none of them)
   3b. You may want some of the 4.X+1 capabilities but not all. You can switch to None and individually list enabled capabilities to ones in v4.X. Then you update and add desired new capabilities.

I may be wrong.

[JoelSpeed]

You see that 4.X+1 has a new capability you do not want

I suspect a lot of users don't check this

If that's the current way this works, then great, we should make sure that's documented. Can we confirm with others that they agree this is the pattern?

[petr-muller]

Yeah I'd expect most users to just follow vCurrent - you just onboard what we ship. I saw efforts in OCP in the past to make behaviors optional (like registry, build etc) so I wanted to respect that. Any specific "others" you want me to agree with the pattern?

[JoelSpeed]

As long as someone has tested, or an authority can verify the "get out of jail free" route, then I'm happy

[JoelSpeed]

It would be useful to provide in this section some examples of how the API might look when in use and explain what data is being stored within the API, unless that exists somewhere else?

In the current API design, it appears there's a repeating section almost that is used to represent different data across the API, but with a similar layout, explaining this repeating block and how it will be used would be helpful at least

[petr-muller]

I'll see if I can add some rendered API examples.

[JoelSpeed]

here the information is supposed to be exposed through ClusterVersion, ClusterOperator and MachineConfigPool resource status, but the APIs are not expressive enough

Is there merit in extending these objects to improve their status if this is an issue?

[JoelSpeed]

Tooling such as OC and the UI? Anything else?

If the idea is that tooling is going to use this, why does it need to be a single resource, and why can we not gather many resources to gather a cohesive view of the cluster?

[petr-muller]

More UIs - web console is one, ACM is another.

I am not entirely opposed to breaking the data structure down, as long as the "gather many resources" does not constitute too much logic necessary for clients to implement. The currents state allows the client to simply iterate the insights to build UXes like the prototype, without need to crosscheck resources against each other.

My preferred approach would be to try to live with the singleton for a while, and then decide what is the best way to break the thing down (it is easier to break things down than assemble them together from small parts).

[JoelSpeed]

Please make sure we have somewhere in this doc, a list of all the places we expect to consume this, I'm sure others will probably ask too how many places will consume this

[JoelSpeed]

Update Health Insights report a state or condition in the cluster that is abnormal or negative and either affects or is affected by the update.

I can find this information today by listing clusteroperators can't I? And filtering on those that are not available?

[enxebre]

As I understand it, the value proposition of the enhancement is exactly removing the need to manually do what you describe (among may other existing manual troubleshooting means) for any client tooling / Cluster Admin.

I'm not sure if a structured API / controller is the best answer to this problem. It feels we might be just moving the problem higher in the stack. And maybe next year we'll feel the need to abstract away updateStatus.

It would be good to articulate concrete upgrade failure scenarios and how they would be presented in this API, so we can have a general common idea of a criteria for success of this proposal and that can be used as a guardrail to help preventing the above from happening.

To solve the "mental model" miss match problem, it feels this could be better suited for a system that we can feed with all the input we already have in mature structured APIs and we let it reason holistically about it and present it in a friendly Cluster Admin Persona language. Codify this holistic interpretation of the cluster in a traditional controller without that system is hard.

[petr-muller]

The value proposition is the unified UX for various classes of problems, tied to an abstraction of the platform-level process realized by many small parts, where previously you needed to investigate the small parts directly (and know about their existence, and know how to troubleshoot them).

I am not absolutely sure the structured API (and its current form) is the best answer to the problem (and I agree about the yet-another-abstraction comment partially, from a purist PoV). But pragmatically, we have verified the following:

1. We know the experimental client-based functionality is useful even in its current form (a lot of positive feedback internally, we see QE using when reporting update-related problems, BU & TAMs say it has positive reception by customers). Hell, the first thing I do when I investigate a update-related must-gather is I run the thing myself against a statis-kas :D
2. The logic we had to do for (1) is complicated enough for me to think every client should not re-implement it, plus some of it feels a lot like encoded platform knowledge / assumptions which I feel belongs to the platform itself, not to the layer around it.
3. We also found some things hard to do from client side, which relies on a one-time shapshot of the cluster state, where a controller able to observe the state continuously would be able to provide some useful additional information (for a flapping thing, how many times did it flap? when was a problematic state first observed? that kind of thing).
4. I said the logic is complicated enough to be in the client, but it is not actually that complicated for a controller. I have already built a crude prototype of the controller once, the robust thing is a bit more complicated but not that much. It becomes complicated once we want it to become a distributed system of producers/informers, which is exactly the reason why this enhancement avoids proposing that, and describes a centralized component instead (but internally, it is still separated enough). To me this is a good tradeoff, where even if we learn the solution has some major flaws, we can decide to never promote it and learn more about what to build instead.

system that we can feed with all the input we already have in mature structured APIs and we let it reason holistically about it and present it in a friendly Cluster Admin Persona language

TBH this description feels a lot like the thing we're proposing. I often think of it as a "the one knowledgeable consumer/interpreter of fragmented OpenShift APIs". Just in-cluster.

[JoelSpeed]

If we didn't go down the route of creating a structured API for this, are there existing APIs that we could improve the observability of, or existing systems (grafana?) that we could improve to provide a better UX?

Obviously we already have the CLI tooling which handles this, we also want a dashboard (console? grafana?) and at the moment we would have to duplicate the logic. Where else do we think we might want this improved/unified UX?

It would probably also be worth highlighting somewhere what data we are aggregating, and what data we are supplying that is net new

[JoelSpeed]

receiving and processing insights from the other controllers in the USC

Is there a message bus involved? How does it receive insights from other places?

[petr-muller]

It is not proposed as a full distributed system yet, so I think of inter-controller (within a single process) communication as an implementation detail. I have it partially working and I owe even my team a high-level overview of the algorithm so I will write it and link it from here (but I'd like to avoid including it in the enhancement because I think it is a detail for now).

High-level: multiple informers send messages through a Go channel to a consumer. No bi-directional communication. Bit of protocol there to handle outdated state etc.

[JoelSpeed]

In the future, when this does become a distributed system, have you thought about what that message bus would look like? It feels like an important step to understand this if we are considering the current proposal a temporary route

[JoelSpeed]

Detail about this communication between controllers would be helpful, an intermediate resource?

[petr-muller]

Answered by #1701 (comment) I think (will provide a high-level description of the current implementation but I consider it to be an implementation detail that can change).

[JoelSpeed]

What would this API look like if the informers were a separate resource, that could be referenced, and then each actor could control one or more of these, and users could list and filter them as they need? It would greatly reduce the size of the output CR here right?

kind: UpdateInsight
spec:
  updatePool:
    type: Worker | ControlPlane
    worker:
      name: ... # we can have multiple worker pools?
  target:
    resource: Nodes | ClusterOperators | MachineConfigPools | ClusterVersions
    name: ....
status:
  conditions: []
  ... # I haven't worked out what else goes here

[petr-muller]

That's one way how we could break the thing down. It has a downside of insights for the same $thing being spread in different locations so any client that wants to report on the $thing needs to put it together.

I am thinking about breaking the big CRD down following the top-level status fields (controlPlane + []workerPool). Instead of one UpdateStatus{.status: {.controlPlane: {}, []workerPools{{workerPool: {}, ...}}}}, have:

ControlPlaneUpdateStatus {}
WorkerPoolUpdateStatus{name: pool-1}
WorkerPoolUpdateStatus{name: pool-2}
...

[JoelSpeed]

It is not possible to reduce the API verbosity without losing valuable information,

What information do you think you would lose? If the API were broken into multiple resources, would this still be a concern?

A top level could refer to individual insight objects that contain detailed (but well scoped) information about a particular issue

[petr-muller]

Basically about every resource involved in the update, we need to know:

1. Is it outdated, updating, or updated, ideally with transitions? This is currently realized with conditions, which are quite verbose, especially if you need to have one for 500 nodes each.
2. ...about anything problematic related to that resource.

Breaking it down to smaller items would work as long as we are still somewhat solving the API fragmentation problem and do not force clients to have complicated logic to put the information together. I think there are some promising directions in the comments here.

[JoelSpeed]

What compatibility do we promise for OC, can this eventually go away? Or because this is a capability, it will always be an issue?

[petr-muller]

Yeah, as long as it is a capability, clients will need to handle the absence of the API. We originally thought this would be an issue for web console but they told us it is fine because web console already does resource discovery and can have conditional behavior on resource presence / absence (see 30f68e2)

[JoelSpeed]

What happens with the health of the cluster when an upgrade isn't in progress? Is there not value in health reporting continuously?

[petr-muller]

I agree there is value, but I'm not sure we are the ones who want to solve that problem. There's a bit of Conway's Law here again I guess. We're an update-focused team talking to update-focused BU representative, who relays update-related problems to us from the field. So we think we are able to tackle the smaller problem because it deals with the domain we know well.

[JoelSpeed]

Can you provide a concrete example of how this will be easier with this proposal? A full workflow showing an example insight and how that points me to the correct fix would be extremely helpful for understanding the desire of this EP

[petr-muller]

Alberto asked for this so I'll come up with an example. For a single insight, the experience may not be necessarily so much better though, compared to when you are the OpenShift expert and know where to look (especially in the current state where we have not focused that much on the insight content, but rather on the structures around).

In the meantime, I have a collection of asciinema recordings that record the output of the prototype in a watch loop. I believe they illustrate that even simply aggregating and showing information in context has value, and that is waht the API enables building:

(I honestly do not remember which ones are really good, but the b02 ones are very realistic, and often show how poor some of our user-exposed copy is btw)

• https://asciinema.org/a/680985
• https://asciinema.org/a/687546
• https://asciinema.org/a/701696

There's a ton more on https://asciinema.org/~petr-muller

[JoelSpeed]

The actual business value of the more general system is not entirely clear and would need to be validated.

How much architectural discussion has there been internally around this?

[petr-muller]

There has been a lot of internal discussion, but the prevailing use case we want to solve was always upgrade-focused, right from the BU ask. Pretty much all discussions always involved asking whether there is an existing reporting system we could easily use to fulfill what is being asked, and the answer was always no (some of these are discussed in the alternatives).

In an ideal world someone would build a general cluster reporting facility that we could use (we have observability folks but they tend to focus on other cases, I guess because they talk to observability-minded customers through observability-minded PMs...).

[enxebre]

can we disambiguate "users" referring to specific Persona? e.g. Cluster Admin Persona

[sdodson]

@petr-muller Is this example up to date with everything in the openshift/api PR? If not, any chance this could be made a bit more complete with the full API represented?

[sdodson]

By that I mean, when I look at the API PR it seems like there's a lot more there than illustrated here, can this be filled in to illustrate a more complete UpdateStatus resource?

[petr-muller]

I'll come up with up to date and further populated example of the API

[petr-muller]

Example input + output + walkthrough added here:

https://github.com/openshift/enhancements/blob/7687d60346060818d7f4725ef0d4529ca197b26b/enhancements/update/update-status-api/update-status-api-example.md

[openshift-ci]

@petr-muller: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/markdownlint	82a0289	link	true	/test markdownlint

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.