-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
migrate more rest handlers to select by resource enablement #108263
Conversation
@deads2k: The label(s) 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 kubernetes/test-infra repository. |
3ef85c3
to
85f9646
Compare
@@ -392,24 +392,9 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) | |||
} | |||
|
|||
// install legacy rest storage | |||
if c.ExtraConfig.APIResourceConfigSource.VersionEnabled(apiv1.SchemeGroupVersion) { | |||
legacyRESTStorageProvider := corerest.LegacyRESTStorageProvider{ |
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.
moved into InstallLegacyAPI to make it impossible for someone to rely on it later in this method.
3684dc3
to
46cfec7
Compare
/kind cleanup |
storage[resource] = podStorage.LegacyBinding | ||
} | ||
|
||
if resource := "podTemplates"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
These have to change to lowercase if we're comparing against enablement by --runtime-config
. We normalize this to lower-case in GenericAPIServer#getAPIGroupVersion
and DefaultStorageFactory#ResourcePrefix
, and forbid upper-case characters in --runtime-config
in MergeAPIResourceConfigs
if resource := "podTemplates"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "podtemplates"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
I would maybe change the normalization to start erroring in GenericAPIServer#getAPIGroupVersion
to flag bad setup
storage[resource] = podTemplateStorage | ||
} | ||
|
||
if resource := "replicationControllers"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "replicationControllers"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "replicationcontrollers"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
if resource := "replicationControllers"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | ||
storage[resource] = controllerStorage.Controller | ||
storage[resource+"/status"] = controllerStorage.Status | ||
if legacyscheme.Scheme.IsVersionRegistered(schema.GroupVersion{Group: "autoscaling", Version: "v1"}) { |
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.
pre-existing, I know, but checking scheme registration here is so weird...
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.
pre-existing, I know, but checking scheme registration here is so weird...
mistakes were made. I think it's just always "true" for us too. I'm willing to remove the check. I don't think we need it.
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 would drop the scheme registration check. if anything, I would check if the Scale field is non-nil
"services": serviceRESTStorage, | ||
"services/proxy": serviceRESTProxy, | ||
"services/status": serviceStatusStorage, | ||
if resource := "limitRanges"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "limitRanges"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "limitranges"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
|
||
"endpoints": endpointsStorage, | ||
if resource := "resourceQuotas"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "resourceQuotas"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "resourcequotas"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
"persistentVolumeClaims": persistentVolumeClaimStorage, | ||
"persistentVolumeClaims/status": persistentVolumeClaimStatusStorage, | ||
"configMaps": configMapStorage, | ||
if resource := "serviceAccounts"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "serviceAccounts"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "serviceaccounts"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
|
||
"componentStatuses": componentstatus.NewStorage(componentStatusStorage{c.StorageFactory}.serversToValidate), | ||
if resource := "persistentVolumes"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "persistentVolumes"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "persistentvolumes"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
if legacyscheme.Scheme.IsVersionRegistered(schema.GroupVersion{Group: "autoscaling", Version: "v1"}) { | ||
restStorageMap["replicationControllers/scale"] = controllerStorage.Scale | ||
|
||
if resource := "persistentVolumeClaims"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "persistentVolumeClaims"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "persistentvolumeclaims"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
if podStorage.Eviction != nil { | ||
restStorageMap["pods/eviction"] = podStorage.Eviction | ||
|
||
if resource := "configMaps"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "configMaps"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "configmaps"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
if serviceAccountStorage.Token != nil { | ||
restStorageMap["serviceaccounts/token"] = serviceAccountStorage.Token | ||
|
||
if resource := "componentStatuses"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
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.
if resource := "componentStatuses"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { | |
if resource := "componentstatuses"; apiResourceConfigSource.ResourceEnabled(corev1.SchemeGroupVersion.WithResource(resource)) { |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, liggitt 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 |
5c9afe9
to
0ec20f9
Compare
squashed. /hold cancel |
/lgtm |
/retest |
/triage accepted |
In support of, "no new beta APIs enabled by default"
/kind clean-up
/sig api-machinery
/priority important-soon
/assign @liggitt