controller: Take first non-empty osImageURL #279
Conversation
Today each MC will contain both an Ignition fragment and an `osImageURL`. Define "merging" as using the first non-empty `osImageURL` so we don't have to be very picky about ordering. This is a smaller version of openshift#228 Prep for: openshift#273
openshift-ci-robot
added
size/M
|
/lgtm Looking at #228 (comment), I'm wondering if it'd make sense to extend it like so: take first non-empty osImageURL, but then also allow overrides from later non-empty osImageURLs and emitting a warning? OTOH, if we go the configmap route in #273, then we'd be able to override there. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test e2e-aws |
|
/test e2e-aws |
2 similar comments
|
/test e2e-aws |
|
/test e2e-aws |
|
I said it back in #228, but I'll say it again: We're going to need to allow the OS to be overridden, but we should make a stink when it happens. Users will inevitably find this and blow their foot off six months down the road when their cluster updates but the OS doesn't. What is the best way to surface that information? Can we propagate it up via the ClusterOperator CR? |
|
Part of the issue is we can't speak sanely about users "overriding" anything until we have the canonical CVO-driven bits from #273 |
|
The way I see it is is this is a short term aid just to have slightly less awkward semantics, though in the end it's really optional. Following on from #273 we could easily change things so that the rendered MC always has the osImageURL from the CVO driven configmap, i.e. ordering of the MC's doesn't matter, and further |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Today each MC will contain both an Ignition fragment and an
osImageURL. Define "merging" as using the firstnon-empty
osImageURLso we don't have to be very picky aboutordering.
This is a smaller version of
#228
Prep for: #273