enhancements/update/targeted-update-edge-blocking: Client-side platform parameter #737
Conversation
I keep repeating this in various internal discussions. Collect it all up with a bow so I can just drop links ;).
Generated with: $ (cd enhancements/update/overriding-blocked-edges; dot -Tsvg flow.dot >flow.svg) using: $ dot -V dot - graphviz version 2.30.1 (20170916.1124)
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…rm parameter I still prefer the flexibility of a PromQL approach [1]. But that is a bigger update-service shift, and I haven't been able to convince folks it's worth doing. This alternative proposal guesses that "platform" will help with targeting most of our target-able blockers, and be quicker to implement. [1]: openshift#426
wking
force-pushed
the
targeted-edge-blocking-by-platform
branch
from
714a3d9 to
8bd3491
Compare
There was a problem hiding this comment.
Most of my review comments in #426 applicable to this PR as well.
| Clusters whose platform is in the array will be blocked. | ||
| Clusters whose platform is known to not be in the array will not be blocked. | ||
|
|
||
| Clusters whose platform is unknown are also blocked. |
There was a problem hiding this comment.
We should not block unknown. Because it is unknown. Blocking clusters has side effect i.e. we need to provide path for them to upgrade. So we should be sure about which platform we want to block. In case of unknown , by definition is it not clear what platform the cluster is running on. So blocking it will create unnecessary churn IMO.
| Maintainers of other update service implementations may or may not be able to apply them to their own implementation. | ||
|
|
||
| The graph-builder's graph-data scraper should learn about [the new 1.1.0 schema](#enhanced-graph-data-schema-for-blocking-edges), and record any `clusters` properties in edge metadata. | ||
| The graph-builder should also be prepared for, and alert on, invalid graph-data configuration. |
There was a problem hiding this comment.
Can you explain The graph-builder should also be prepared for, and alert on, invalid graph-data configuration . I do not think I understand.
There was a problem hiding this comment.
If/when Cincinnati gets fed broken graph-data, it should complain about it in a way its admins will notice. Alerts seemed like a reasonable choice. That way they know they should acquire fixed graph-data.
| #### Clients not reporting data | ||
|
|
||
| Some Cincinnati clients, for example, older cluster-version operators and external web renderers, may not submit `platform` query parameters. | ||
| These clients will have conditional edges blocked, unless their update service happens to implement [cluster property lookup](#update-service-cluster-property-lookup). |
There was a problem hiding this comment.
Another reason not to block edges when platform is none.
|
Replaced by the v3 #821, where I'm now recommending cluster-side decisions, with the update service declaring conditional edges and telling the cluster-side tools how to decide if the cluster is vulnerable to the known bugs impacting its update decisions. |
I still prefer the flexibility of a PromQL approach (#426). But that is a bigger update-service shift, and I haven't been able to convince folks it's worth doing, and maybe they're right and it is overkill. This alternative proposal guesses that "platform" will help with targeting most of our target-able blockers, and be quicker to implement. Historically, it would have been helpful in scoping down:
Noneclusters.