USHIFT-6080: Enable custom feature gates on MicroShift

[copejon]

No description provided.

[openshift-ci-robot]

@copejon: This pull request references USHIFT-6080 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 spike to target the "4.21.0" version, but no target version was set.

In response to this:

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.

[copejon]

/jira ushift-6177

[pmtk]

Couple initial comments, I might add more once this enhancement gets traction

[pmtk]

From I've seen before the leave, MicroShift does not inherit any settings: whatever is enabled in openshift isn't enabled in microshift. It inherits the FGs, but they're all disabled, I think.
That's why I had to enable these two to fix rebase: openshift/microshift@31598b2

ref: https://github.com/openshift/api/blob/master/features.md

[copejon]

I see now that the rebase script also handles disabling some of the featuregates that are pulled in with openshift manifests. Good catch!

[pmtk]

This means we'll enable everything that's enabled on openshift, right?

[copejon]

Typo, it should say "MicroShift"

[pmtk]

When I dealt with FG couple months ago I was under impression that all kube components receive info from the api server about the enabled feature gates, so there was no need to deal with each component config individually, but please keep me honest.
You could add another FG to https://github.com/openshift/microshift/blob/main/pkg/controllers/kube-apiserver.go#L220 and see in the logs if it's propagated.

[copejon]

Oh I see now. I'd read that each component has a feature-gates CLI flag in the format of ${FEATURE_GATE}=${BOOL}. I see now the kube-apiserver is different in that the flag format is ${KUBE_COMPONENT||"KUBE"}:${FEATURE_GATE}=${BOOL}.

Something else confuses me: the rebase script explicitly sets several featuregates in the kubelet config directly: https://github.com/openshift/microshift/blob/72976404859c8509cc0aa34c1b38255d3a03976e/scripts/auto-rebase/rebase.sh#L689-L692

yq -i '.featureGates.APIPriorityAndFairness = true' "${REPOROOT}/assets/core/kubelet.yaml"
yq -i '.featureGates.PodSecurity = true' "${REPOROOT}/assets/core/kubelet.yaml"
yq -i '.featureGates.DownwardAPIHugePages = true' "${REPOROOT}/assets/core/kubelet.yaml"

But the kubelet doesn't seem to accept them:

kubelet W0929 14:26:08.754914 1210239 feature_gate.go:328] unrecognized feature gate: APIPriorityAndFairness
kubelet W0929 14:26:08.754919 1210239 feature_gate.go:328] unrecognized feature gate: DownwardAPIHugePages
kubelet W0929 14:26:08.754923 1210239 feature_gate.go:328] unrecognized feature gate: PodSecurity

[copejon]

Less of a concern for now but worth noting:
It looks like the default CSI driver also hardcodes a featuregate: https://github.com/openshift/microshift/blob/a88f42833d122e0bd3c47e2eb43e25f6c5024f8e/assets/optional/topolvm/03-topolvm.yaml#L999-L1005

[pmtk]

https://github.com/openshift/api/blob/master/config/v1/types_feature.go#L71-L73
It looks like you cannot go back from TP/DP/Custom FGs once enabled.
Undoing FGs on microshift and upgrading would be a deviation

[pmtk]

How do the operators filter FeatureGates? Do they use openshift config API or they ask API Server?

What if one of the operators wants to query that info but is unable due to differences - the operator on microshift would miss the functionality, right?

[copejon]

How do the operators filter FeatureGates? Do they use openshift config API or they ask API Server?

Operators watch the FeatureGate API object named cluster.

For instance, the machine-config-operator watches FG cluster, filters kubelet-specific values, and handles r/w to kubelet configs and kubelet service restarts.

What if one of the operators wants to query that info but is unable due to differences - the operator on microshift would miss the functionality, right?

Do you mean differences in how FeatureGates would be published on OpenShift vs MicroShift (config API vs file)?

[pmtk]

Do you mean differences in how FeatureGates would be published on OpenShift vs MicroShift (config API vs file)?

Exactly.
But I think if the need arises, we could just mirror file config -> FG CR

[pmtk]

We will implement this in MicroShift during config parsing, right?

[copejon]

Correct. I think that because these are new fields in the ushift config, ushift should handle some basic validation.

[pmtk]

Do you think we should persist history (or something similar) of enabled featuregates? Right now we have no mechanisms that would prevent upgrading after user would revert the FG settings.

We could utilize some file hidden in /var/lib/microshift (security through obscurity) and upon start up log the information, so when we get sosreport, we get some info

[copejon]

I agree with you. We need this file to both prevent cluster upgrades and to help in handling users' reverting feature customizations. More on this in my later comment

[pmtk]

Unless we add something like I described here: https://github.com/openshift/enhancements/pull/1851/files#r2410448992

[pmtk]

permanent, one-way operation that permanently disables upgrade capability

How do you ensure this?

[copejon]

On OpenShift, reverting a *NoUpgrade featureSet, which includes the custom list of feature gates, is blocked by API schema validation (here)

MicroShift's config isn't defined by a schema, so I need to come up with a proactive approach to config updates - a simple lock file, like you suggested above, would be enough I think.

On OpenShift, cluster upgradability is determined by the CVO. The CVO checks all Operator API objs for .status.conditions[].type: "Upgradeable", and if any are false, the CVO marks the cluster as a whole as un-upgradable (handled by this func here)

Again, I think this is probably solved by writing a lock file into /var/lib/microshift.

I'll amend the proposal to reflect this.

[pmtk]

Users must remove custom feature gate configurations before attempting minor version changes.

I wonder if there's something that should not be disabled after enabling, maybe something that changes data in a way that cannot be returned from, idk

[copejon]

This statement just needs to be removed entirely I think.

Whether or not an alpha/beta feature is irreversible is something that would have to be kept track of by the controller that implements the feature, if at all.

[pmtk]

This doesn't match https://github.com/openshift/enhancements/pull/1851/files#r2410457599

[pmtk]

Does it? I'm not sure if user edited file in /etc is subject to rollbacks.
I know there's some weird 3 way merge going on, but not sure if the user edited the file...

[copejon]

Oh, you're absolutely right. This statement needs to be removed.

[copejon]

Thanks for the thorough review @pmtk

I'll fix the discrepancies and amend the validation, fg reversion, and upgrade statements to reflect my responses.

[copejon]

Correct. I think that because these are new fields in the ushift config, ushift should handle some basic validation.

[copejon]

How do the operators filter FeatureGates? Do they use openshift config API or they ask API Server?

Operators watch the FeatureGate API object named cluster.

For instance, the machine-config-operator watches FG cluster, filters kubelet-specific values, and handles r/w to kubelet configs and kubelet service restarts.

What if one of the operators wants to query that info but is unable due to differences - the operator on microshift would miss the functionality, right?

Do you mean differences in how FeatureGates would be published on OpenShift vs MicroShift (config API vs file)?

[copejon]

I agree with you. We need this file to both prevent cluster upgrades and to help in handling users' reverting feature customizations. More on this in my later comment

[copejon]

On OpenShift, reverting a *NoUpgrade featureSet, which includes the custom list of feature gates, is blocked by API schema validation (here)

MicroShift's config isn't defined by a schema, so I need to come up with a proactive approach to config updates - a simple lock file, like you suggested above, would be enough I think.

On OpenShift, cluster upgradability is determined by the CVO. The CVO checks all Operator API objs for .status.conditions[].type: "Upgradeable", and if any are false, the CVO marks the cluster as a whole as un-upgradable (handled by this func here)

Again, I think this is probably solved by writing a lock file into /var/lib/microshift.

I'll amend the proposal to reflect this.

[copejon]

This statement just needs to be removed entirely I think.

Whether or not an alpha/beta feature is irreversible is something that would have to be kept track of by the controller that implements the feature, if at all.

[copejon]

Oh, you're absolutely right. This statement needs to be removed.

[ShaunaDiaz]

Do we expect that a dev to test using feature gates and then build the final deployment system separately? Should we advise that as a best practice, rather than dev trying to use this feature and then remove it?

[copejon]

Yes to both questions. Enabling custom feature gates is not a supported configuration. It is strictly for users who wish to test alpha/beta kube features.

[pmtk]

Do we expect that a dev to test using feature gates and then build the final deployment system separately?

I think it's general expectation, at least with bootc. Or at least for me.
You play with dev machine, you figure out how to do stuff, but for the production you want to automate as much as possible - like automatic updates to new bootc image, onboarding (maybe FIDO), etc. I wouldn't expect a fleet of 1000 devices to be set up one by one.

[ShaunaDiaz]

So can we use config snippets with this feature, or no?

[copejon]

Yes, that would be ideal

[ShaunaDiaz]

Ah, so perhaps we should add this to update troubleshooting? What error might we see if we fail an update because of the sentinel file?

[copejon]

Bootc users should refer to the journal to find the rpm install error.

Rpm users would see the error in stdout during rpm installation

@pacevedom @pmtk wdyt about that?

[pmtk]

Bootc users should refer to the journal to find the rpm install error.

Not sure if this is possible - AFAIK bootc is not reinstalling system or upgrading it on a package level, I think it's more of a snapshot of new root fs which you switch to on boot.
I think this would have to be failure at the beginning of the microshift startup. Something similar to "version based upgrade blocking" we haven't even used once.

This topic is mentioned in couple places, but I won't create extra comments

[copejon]

I see, good point. Handling it this way would also create a congruent behavior to what you propose when users change feature gates.

[ShaunaDiaz]

We currently advise users to open a support case before running a cleanup-data --all. Is there an easier way to accomplish removing the feature manually without having to do this?

[pmtk]

I believe that's the goal: you don't want users to revert the FG because upgrades are not supported. It could be that FG makes some changes to the cluster that might render it unusable upon disabling FG.
We want to avoid situation like: enables FG, runs, disables FG, upgrades, re-enables FG.

[copejon]

To add: They are not guaranteed to be backwards compatible.

FeatureGates represent mostly alpha and beta features, meaning they are in active development and could differ significantly between minor or patch versions. Users may open support tickets to address this but more than likely the engineering team can't do much about it.

[ShaunaDiaz]

In production environments and development environments both? Should we give some guidance about the differences in using feature gates per use case?

[copejon]

I'll reword this to be more explicit. FeatureGates should never be used in production environments. They exist to allow developers and users to experiment with k8s alpha/beta features before they become part of k8s by default.

[copejon]

Strike that. I had a crossed wire when writing this - stand alone cluster is a specific kind of OpenShift deployment and not MicroShift. Removed

[ShaunaDiaz]

Will we still provide users with an example config in docs that is tested?

[copejon]

Yes, ty. Will add

[ShaunaDiaz]

So if I understand, we'll support using this feature, but not necessarily whatever is enabled with it? How do we define the support scope so it's clear to everyone?

[copejon]

So if I understand, we'll support using this feature, but not necessarily whatever is enabled with it?

Correct. The microshift feature is that users will be able to configure feature-gates. Not the feature-gates themselves.

How do we define the support scope so it's clear to everyone?

I need to add more information on this. This OpenShift article provides the best detail I've found so far on support. However it only distinguishes between DevPreview and TechPreview (sets of a feature gates), and not individual feature gates.

[ShaunaDiaz]

Sounds like we need to add a Known Issue to the docs?

[copejon]

I think we can wait on this for now. We have a ticket to fix this issue. It could end up being a doc fix, but that's not known right now.

[ShaunaDiaz]

Is this still true if we can't downgrade, or is it a non-issue with this feature?

[copejon]

It would be a non-issue for downgrades also. It just mean that the target version of microshift will still be able to read the older version of the config.

[ShaunaDiaz]

After we do this sequence, how do we test to make sure it worked as intended?

[copejon]

I think it would be enough to checkout component logs or config files to verify there are no custom feature gates. microshift-cleanup-data --all and deleting .featureGates from the config is effectively wiping the slate clean.

[pacevedom]

A full cleanup wipes user data too from the cluster, as it removes all directories (including etcd).
Would it be enough to restart MicroShift instead? That should bring the control plane back a stable state, regardless of the workloads (as we do not have operators). The kubelet writes its configuration every time MicroShift is restarted and all components get the flags in similar fashion (code or config file, but before starting). If user's application breaks because it relies on specific FeatureGates then that is on them?

[copejon]

Because featureGates are either alpha or beta stages, its not a guarantee that disabling a feature ensures the changes that feature made to the cluster are also cleaned up. This could lead to errors that aren't trivial to diagnose.

[pmtk]

Or maybe it should fail to start if this was changed? I know it's "NoUpgrade", not "NoStart", but this would be explicit. In memory config overwrite seems a bit hidden

[copejon]

I mostly agree with you. The one sticking point for me is that OCP does almost the same thing as proposed here by silently reverting user changes to the FeatureGates API object. But maybe we can consider MicroShift enough of a different beast to justify not starting. Or perhaps some very loud logging would be sufficient?

[pmtk]

Not sure if using cleanup is strictly needed here. You can play with FGs on dev machine, so correcting an invalid value should be okay?

[copejon]

It's hard to say but I think we don't need to err this far on the side of caution. Will remove the cleanup.

[pmtk]

Should we also log the setting? So if we receive sosreport we'll see in the history that FG was enabled? You can, after all, remove the sentinel file and bypass the upgrade prevention mechanism.

[copejon]

Agreed! Will add

[copejon]

Great feedback! Comments in-line

[copejon]

I mostly agree with you. The one sticking point for me is that OCP does almost the same thing as proposed here by silently reverting user changes to the FeatureGates API object. But maybe we can consider MicroShift enough of a different beast to justify not starting. Or perhaps some very loud logging would be sufficient?

[copejon]

I see, good point. Handling it this way would also create a congruent behavior to what you propose when users change feature gates.

[copejon]

Agreed! Will add

[copejon]

Yes, ty! Will fix

[copejon]

To add: They are not guaranteed to be backwards compatible.

FeatureGates represent mostly alpha and beta features, meaning they are in active development and could differ significantly between minor or patch versions. Users may open support tickets to address this but more than likely the engineering team can't do much about it.

[copejon]

I'll reword this to be more explicit. FeatureGates should never be used in production environments. They exist to allow developers and users to experiment with k8s alpha/beta features before they become part of k8s by default.

[copejon]

No, and also I don't think its data we would want to maintain in the long term. Because of the experimental nature of FGs, it's more up to the user to analyze the benefits for their particular environment. Proposed feature is itself not at all resource intensive, while the featureGates a user may set may alter resource overhead.

[copejon]

It's hard to say but I think we don't need to err this far on the side of caution. Will remove the cleanup.

[copejon]

Yes, ty. Will add

[copejon]

So if I understand, we'll support using this feature, but not necessarily whatever is enabled with it?

Correct. The microshift feature is that users will be able to configure feature-gates. Not the feature-gates themselves.

How do we define the support scope so it's clear to everyone?

I need to add more information on this. This OpenShift article provides the best detail I've found so far on support. However it only distinguishes between DevPreview and TechPreview (sets of a feature gates), and not individual feature gates.

[pmtk]

Can we allow for changing to another not upgradable FG set? Like freely change between TechPreviewNoUpgrade, DevPreviewNoUpgrade, and customNoUpgrade but prevent from resetting them to upgradable setting? Or this is also prohibited by OpenShift?

[copejon]

We could, but we'd be diverging from OpenShift's featureset rules, which I don't think we'd want.

[pmtk]

No more Q from me

[pacevedom]

Same as https://github.com/openshift/enhancements/pull/1851/files#r2440358965

[pacevedom]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pacevedom

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 [pacevedom]

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

[openshift-ci]

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