[WIP] Attempt at feature-gating the default image choice

[jkyros]

• I was trying to find a good way to "soft" gate the default behavior of defaulting to the new format image ( per [WIP] Teach MCO to use the new-format image by default  #3258)

• It occurred to me that it needed to be gated both during bootstrap and during live operation and I was basically re-implementing feature gates poorly, so I figured maybe I should just try to use them.

• This is just trying to see what it feels like to have the controller bootstrap and "live" render controller react to feature gates.

• I thought about just pushing the featuregates into controllerconfig (they currently only make it into renderconfig right now), but:

  • some of our other controllers already look directly at the feature gates
  • we'd have to weirdly merge featuregates also into controllerconfig during bootstrap
  • so I shied away from doing that unless we want to decide something like "controllers will always read featuregates from controllerconfig".

• I think I'm probably"cheating" by "soft" using featuregates since I don't actually require a featureset be enabled

This does seem to work though -- if you set the featuregate at installtime (e.g. openshift-install create manifests and putting something like

kind: FeatureGate
metadata:
  name: cluster
spec:
  customNoUpgrade:
    enabled:
    - MCONewContainerImageFormat

in the manifests folder.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jkyros

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:

• OWNERS [jkyros]

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

[cgwalters]

Offhand I feel like preventing upgrades is unnecessary here and actually actively breaks testing out what we're trying to accomplish. We definitely want to be able to test maintaining an override for the OS while upgrading OCP.

[cgwalters]

We had a realtime chat on this and briefly I'd summarize my end point of view is that we should instead add a machineConfigOperatorFeatures: ["custom-os-override"] or so that's part of MachineConfig. A nice aspect about this is that it's more clearly per-pool; distinguishing between e.g. control plane versus workers or even a subset of workers.

[jlebon]

If the goal is to gate at the pool level, would it belong better as part of the MachineConfigPool CRD instead?

[openshift-ci]

@jkyros: The following tests 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/e2e-agnostic-upgrade	99f9788	link	true	/test e2e-agnostic-upgrade
ci/prow/bootstrap-unit	99f9788	link	false	/test bootstrap-unit
ci/prow/e2e-aws	99f9788	link	true	/test e2e-aws
ci/prow/verify	99f9788	link	true	/test verify
ci/prow/e2e-gcp-op	99f9788	link	true	/test e2e-gcp-op
ci/prow/okd-scos-images	99f9788	link	true	/test okd-scos-images

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/test-infra repository. I understand the commands that are listed here.

[openshift-merge-robot]

@jkyros: PR needs rebase.

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/test-infra repository.

[cgwalters]

I think at this point we aren't going to do this, so closing just to clear up the large MCO PR queue. (But like a lot of things I know there's been some back and forth, obviously please do reopen if I'm wrong or this changes)