add featuregate status#1405
Conversation
openshift-ci
bot
added
do-not-merge/work-in-progress
|
Hello @deads2k! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
approved
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
config/v1/types_feature.go
Outdated
|
|
||
| type FeatureGateAttributes struct { | ||
| // name is the name of the FeatureGate | ||
| // +kubebuilder:validation:Pattern=`^([A-Za-z0-9-]+\.)*[A-Za-z0-9-]+\.?$` |
There was a problem hiding this comment.
Since this will be synced from the spec in a CustomNoUpgrade scenario, do we have the same validation on the spec for that? How do we validate that the OpenShift default feature gates for each set adhere to this?
There was a problem hiding this comment.
All known featuregates follow this pattern. Because of that, no existing cluster can have set value that doesn't match and had that value work. Because of that, I'll update the customnoupgrade validation in this PR.
config/v1/types_feature.go
Outdated
| // name is the name of the FeatureGate | ||
| // +kubebuilder:validation:Pattern=`^([A-Za-z0-9-]+\.)*[A-Za-z0-9-]+\.?$` | ||
| // +kubebuilder:validation:Required | ||
| // +required |
There was a problem hiding this comment.
Do we need +required or does the kubebuilder version suffice? Is there a tool that does not understand the kubebuilder tag? (I know kubeubilder doesn't understand +required)
There was a problem hiding this comment.
removing it appeared to have no impact
config/v1/types_feature.go
Outdated
| // enabled is a list of all feature gates that are enabled in the cluster for the named version | ||
| // +optional | ||
| Enabled []FeatureGateAttributes `json:"enabled"` | ||
| // disabled is a list of all feature gates that are disabled in the cluster for the named version | ||
| // +optional | ||
| Disabled []FeatureGateAttributes `json:"disabled"` |
There was a problem hiding this comment.
| // enabled is a list of all feature gates that are enabled in the cluster for the named version | |
| // +optional | |
| Enabled []FeatureGateAttributes `json:"enabled"` | |
| // disabled is a list of all feature gates that are disabled in the cluster for the named version | |
| // +optional | |
| Disabled []FeatureGateAttributes `json:"disabled"` | |
| // enabled is a list of all feature gates that are enabled in the cluster for the named version. | |
| // +optional | |
| Enabled []FeatureGateAttributes `json:"enabled"` | |
| // disabled is a list of all feature gates that are disabled in the cluster for the named version. | |
| // +optional | |
| Disabled []FeatureGateAttributes `json:"disabled"` |
Nit, other godocs here are punctuated
config/v1/types_feature.go
Outdated
| } | ||
|
|
||
| type FeatureGateAttributes struct { | ||
| // name is the name of the FeatureGate |
There was a problem hiding this comment.
| // name is the name of the FeatureGate | |
| // name is the name of the FeatureGate. |
All valid featuregates follow this pattern today. This means that anyone setting a value that doesn't match the pattern would have gotten a cluster that didn't work and those people couldn't upgrade anyway, so adding this validation is acceptable.
deads2k
force-pushed
the
make-tech-preview-great
branch
from
d24a78b to
5838758
Compare
openshift-ci
bot
added
the
size/L
|
@deads2k: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/approve Hold cancel at your will |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
confirmed with staff eng we will do this to free up the ccm payload problem. |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
2 participants
For openshift/enhancements#1373
Part 1 of a multi-step plan to make individual featuregates observable via the API. This was previously avoided because when featuregates get promoted from TechPreview to Default, during an upgrade, an old operator may try to enable a TechPreview variant of a feature.
This can be addressed by keying the featuregates by version. Let's see how ugly that will be in practice now that we have a decent build-out of library-go.
/hold
This is proving the concept, not to be merged until it shows as working end-to-end.
Other steps include