-
Notifications
You must be signed in to change notification settings - Fork 533
describe how the CVO can support TechPreviewNoUpgrade #957
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
openshift-merge-robot
merged 1 commit into
openshift:master
from
deads2k:cvo-techpreview
Nov 18, 2021
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| --- | ||
| title: CVO TechPreview Manifests | ||
| authors: | ||
| - "@deads2k" | ||
| reviewers: | ||
| - "@wking" | ||
| approvers: | ||
| - "@sdodson" | ||
| - "@wking" | ||
| api-approvers: # in case of new or modified APIs or API extensions (CRDs, aggregated apiservers, webhooks, finalizers) | ||
| - @sttts | ||
| creation-date: 2021-11-15 | ||
| last-updated: 2021-11-15 | ||
| tracking-link: # link to the tracking ticket (for example: Jira Feature or Epic ticket) that corresponds to this enhancement | ||
| - slack thread somewhere | ||
| see-also: | ||
| replaces: | ||
| superseded-by: | ||
| --- | ||
|
|
||
| # CVO TechPreview Manifests | ||
|
|
||
| ## Summary | ||
|
|
||
| [TechPreviewNoUpgrade](https://github.com/openshift/api/blob/be1be0e89115702f8b508d351c4f5c9a16e5ae95/config/v1/types_feature.go#L32-L34) | ||
| is the canonical way to enable tech preview features. | ||
| It is the setting that all operators use to enable Tech Preview features. | ||
| The CVO will honor this setting on a per-manifest basis, so that tech preview manifests are never present on | ||
| non-tech preview clusters. | ||
|
|
||
| ## Motivation | ||
|
|
||
| When introducing tech preview operators, the current state of the art (before this enhancement) is to have the | ||
| operator build an inert mode that detects if tech preview is not enable and then does not take action. | ||
| This approach results in having additional resources, including the clusteroperator/tech-preview itself, | ||
| installed on every cluster, including non-tech-preview upgrades. | ||
| This also means that currently, if the tech preview does not progress to stable, the component has to keep "dead" manifests | ||
| around and marked for deletion by the CVO on upgraded clusters. | ||
| By having the CVO honor tech preview per manifest, we can avoid fanning out inert logic to every tech preview operator, | ||
| we can avoid exposing tech preview operators to customers via clusteroperators, and we can avoid creating unnecessary | ||
| resources like namespaces, roles, rolebindings, serviceacounts, and deployments. | ||
|
|
||
| ### Goals | ||
|
|
||
| 1. CVO honors an "only create, reconcile, and update this manifest in clusters with tech preview enabled". | ||
|
|
||
| ### Non-Goals | ||
|
|
||
| 1. Bootstrap rendering for tech preview operators. | ||
| 2. 4.y upgrades. TechPreviewNoUpgrade explicitly disallows upgrades from 4.y to 4.y+1. | ||
| 3. Cleanup of manifests created as TechPreviewNoUpgrade. TechPreviewNoUpgrade explicitly disallows being unset and this | ||
| is enforced using validation on the apiserver. | ||
| 4. Allow manifests to be removed or un-reconciled if TechPreviewNoUpgrade is *not* set. There are a few case (mostly | ||
| around CRDs), where this capability is useful, but it is not as common as needing to create something additional. | ||
| 5. Allowing manifests condition on other featuresets in this iteration. | ||
|
|
||
| ## Proposal | ||
|
|
||
| Add an annotation that can be set on CVO manifests called `release.openshift.io/feature-gate` with one legal/honored value | ||
| of "TechPreviewNoUpgrade". | ||
deads2k marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| If the value is not "TechPreviewNoUpgrade", the manifest is never created. | ||
| Since most people add manifests in order to see them applied, they will notice if their manifest is never created and | ||
| the choice of annotation allows for potential future usage in the CVO consistent with feature gate handling used by | ||
| other operators. | ||
|
|
||
| If a manifest sets `.metadata.annotations["release.openshift.io/feature-gate"]="TechPreviewNoUpgrade"`, the manifest will | ||
| not be created unless `featuregates.config.openshift.io|.spec.featureSet="TechPreviewNoUpgrade"`. | ||
| If `featuregates.config.openshift.io|.spec.featureSet="TechPreviewNoUpgrade"`, then the manifest is reconciled as normal | ||
| for whatever stage the CVO is in. | ||
| During bootstrapping, the CVO will assume no feature sets are enabled until it can successfully retrieve | ||
| `featuregates.config.openshift.io` from the Kubernetes API server. | ||
| If bootstrapping changes are required, it will be responsibility of the contributing team to manage the proper inclusion | ||
| in the installer and other bootstrapping components. | ||
|
|
||
| Because the cluster cannot upgrade to 4.y+1 and because the TechPreviewNoUpgrade flag cannot be unset, there is no concern | ||
| about removing any manifests that were added for TechPreviewNoUpgrade. | ||
|
|
||
| ### User Stories | ||
|
|
||
| ### API Extensions | ||
|
|
||
| ### Implementation Details/Notes/Constraints [optional] | ||
|
|
||
| ### Risks and Mitigations | ||
|
|
||
| ## Design Details | ||
|
|
||
| ### Open Questions [optional] | ||
|
|
||
| ### Test Plan | ||
|
|
||
| 1. The TechPreview CI jobs (already present), should run after this feature is implemented. | ||
| 2. In 4.10, the cluster-api clusteroperator should not be present in our "normal" CI flows. | ||
|
|
||
| ### Graduation Criteria | ||
|
|
||
| #### Dev Preview -> Tech Preview | ||
|
|
||
| #### Tech Preview -> GA | ||
|
|
||
| #### Removing a deprecated feature | ||
|
|
||
| ### Upgrade / Downgrade Strategy | ||
|
|
||
| When set, `TechPreviewNoUpgrade` prevents all 4.y to 4.y+1 upgrades. | ||
|
|
||
| ### Version Skew Strategy | ||
|
|
||
| Because upgrades are prevented when this feature is active, there is no supported 4.y skew. | ||
| The clusteroperator/kube-apiserver sets upgradeable=false when TechPreviewNoUpgrade is set. | ||
| 4.y.z+1 is still a possible upgrade path so that we can provide CVE service, but tech preview content in the payload | ||
| is not expected to make a drastic change and the underlying cluster capabilities do not change significantly in a z-stream. | ||
| This means that tech preview clusteroperators face the same manifest update restrictions in a single release as normal | ||
| manifests. | ||
|
|
||
| ### Operational Aspects of API Extensions | ||
|
|
||
| #### Failure Modes | ||
|
|
||
| #### Support Procedures | ||
|
|
||
| ## Implementation History | ||
|
|
||
| Major milestones in the life cycle of a proposal should be tracked in `Implementation | ||
| History`. | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| The idea is to find the best form of an argument why this enhancement should _not_ be implemented. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| ### Inert Operators | ||
| It is possible for every tech preview operator to | ||
| 1. create an inert run mode | ||
| 2. self-bootstrap resources "normally" created by the CVO, like CRDS. | ||
| 3. if the operator doesn't graduate, it must continue to be included in the payload and list its CVO managed resources | ||
| to remove them in the next release. | ||
| 5. create a clusteroperator that every customer has ignore | ||
|
|
||
| This fans the problem out from a single point (CVO), to every tech preview operator. | ||
| It also increases the chance that a cluster admin would misinterpret an inert-mode operator as tainting their cluster | ||
| with unsupported, tech-preview bits. | ||
|
|
||
| ### Allowing Other FeatureSets | ||
| The `featuregates.config.openshift.io|.spec.featureSet` takes other values, but TechPreviewNoUpgrade has qualities that | ||
| make it easier to start with. | ||
| 1. 4.y+1 upgrades are not allowed | ||
| 2. it cannot be unset | ||
|
|
||
| The combination of these two things make it easier to quickly deliver an MVP. | ||
| The API is formed in an extensible way, but only supporting TechPreviewNoUpgrade makes the changes tractable for a single release. | ||
|
|
||
| ## Infrastructure Needed [optional] | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the canonical method to enable them or a result of enabling tech preview features? I thought they were enabled via per feature gates but an outcome of enabling some may be TechPreviewNoUpgrade having been set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess part of my confusion is that it's not clear to me if this line is saying what will be the case after this enhancement lands or how things are today.