Skip to content

Commit

Permalink
Merge pull request #63165 from deads2k/api-08-kubeapiversion
Browse files Browse the repository at this point in the history 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Remove KUBE_API_VERSIONS

Fixes #63102

KUBE_API_VERSIONS is an attempt to control the available serialization of types. It pre-dates the idea that we'll have separate schemes, so it's not a thing that makes sense anymore.

Server-side we've had a very clear message about breaks in the logs for a year "KUBE_API_VERSIONS is only for testing. Things will break.".

Client-side it became progressively more broken as we moved to generic types for CRUD more than a year ago. What is registered doesn't matter when everything is unstructured.

We should remove this piece of legacy since it doesn't behave predictable server-side or client-side.

@smarterclayton @lavalamp
@kubernetes/sig-api-machinery-bugs 

```release-note
KUBE_API_VERSIONS is no longer respected.  It was used for testing, but runtime-config is the proper flag to set.
```
  • Loading branch information
Kubernetes Submit Queue committed Apr 26, 2018
2 parents b8ab289 + a68c571 commit dd5f030
Show file tree
Hide file tree
Showing 35 changed files with 63 additions and 255 deletions.
5 changes: 4 additions & 1 deletion cmd/kube-apiserver/app/aggregator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ func createAggregatorServer(aggregatorConfig *aggregatorapiserver.Config, delega
go func() {
// let the CRD controller process the initial set of CRDs before starting the autoregistration controller.
// this prevents the autoregistration controller's initial sync from deleting APIServices for CRDs that still exist.
crdRegistrationController.WaitForInitialSync()
// we only need to do this if CRDs are enabled on this server. We can't use discovery because we are the source for discovery.
if aggregatorConfig.GenericConfig.MergedResourceConfig.AnyVersionForGroupEnabled("apiextensions.k8s.io") {
crdRegistrationController.WaitForInitialSync()
}
autoRegistrationController.Run(5, context.StopCh)
}()
return nil
Expand Down
64 changes: 14 additions & 50 deletions cmd/kube-apiserver/app/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,19 +179,6 @@ func CreateServerChain(completedOptions completedServerRunOptions, stopCh <-chan
return nil, err
}

// if we're starting up a hacked up version of this API server for a weird test case,
// just start the API server as is because clients don't get built correctly when you do this
if len(os.Getenv("KUBE_API_VERSIONS")) > 0 {
if insecureServingOptions != nil {
insecureHandlerChain := kubeserver.BuildInsecureHandlerChain(kubeAPIServer.GenericAPIServer.UnprotectedHandler(), kubeAPIServerConfig.GenericConfig)
if err := kubeserver.NonBlockingRun(insecureServingOptions, insecureHandlerChain, kubeAPIServerConfig.GenericConfig.RequestTimeout, stopCh); err != nil {
return nil, err
}
}

return kubeAPIServer.GenericAPIServer, nil
}

// otherwise go down the normal path of standing the aggregator up in front of the API server
// this wires up openapi
kubeAPIServer.GenericAPIServer.PrepareRun()
Expand Down Expand Up @@ -474,18 +461,8 @@ func BuildGenericConfig(

client, err := internalclientset.NewForConfig(genericConfig.LoopbackClientConfig)
if err != nil {
kubeAPIVersions := os.Getenv("KUBE_API_VERSIONS")
if len(kubeAPIVersions) == 0 {
lastErr = fmt.Errorf("failed to create clientset: %v", err)
return
}

// KUBE_API_VERSIONS is used in test-update-storage-objects.sh, disabling a number of API
// groups. This leads to a nil client above and undefined behaviour further down.
//
// TODO: get rid of KUBE_API_VERSIONS or define sane behaviour if set
glog.Errorf("Failed to create clientset with KUBE_API_VERSIONS=%q: %v. KUBE_API_VERSIONS is only for testing. Things will break.",
kubeAPIVersions, err)
lastErr = fmt.Errorf("failed to create clientset: %v", err)
return
}

kubeClientConfig := genericConfig.LoopbackClientConfig
Expand Down Expand Up @@ -596,30 +573,20 @@ func BuildAdmissionPluginInitializers(
}
}

// TODO: drop this REST mapper once client is guaranteed to be non-nil
// See KUBE_API_VERSIONS comment above
restMapper := legacyscheme.Registry.RESTMapper()
admissionPostStartHook := func(_ genericapiserver.PostStartHookContext) error {
return nil
}

// We have a functional client so we can use that to build our discovery backed REST mapper
if client != nil && client.Discovery() != nil {
// Use a discovery client capable of being refreshed.
discoveryClient := cacheddiscovery.NewMemCacheClient(client.Discovery())
discoveryRESTMapper := discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, meta.InterfacesForUnstructured)

restMapper = discoveryRESTMapper
admissionPostStartHook = func(context genericapiserver.PostStartHookContext) error {
discoveryRESTMapper.Reset()
go utilwait.Until(discoveryRESTMapper.Reset, 10*time.Second, context.StopCh)
return nil
}
// Use a discovery client capable of being refreshed.
discoveryClient := cacheddiscovery.NewMemCacheClient(client.Discovery())
discoveryRESTMapper := discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, meta.InterfacesForUnstructured)

admissionPostStartHook := func(context genericapiserver.PostStartHookContext) error {
discoveryRESTMapper.Reset()
go utilwait.Until(discoveryRESTMapper.Reset, 10*time.Second, context.StopCh)
return nil
}

quotaConfiguration := quotainstall.NewQuotaConfigurationForAdmission()

kubePluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, cloudConfig, restMapper, quotaConfiguration)
kubePluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, cloudConfig, discoveryRESTMapper, quotaConfiguration)
webhookPluginInitializer := webhookinit.NewPluginInitializer(webhookAuthWrapper, serviceResolver)

return []admission.PluginInitializer{webhookPluginInitializer, kubePluginInitializer}, admissionPostStartHook, nil
Expand All @@ -631,12 +598,9 @@ func BuildAuthenticator(s *options.ServerRunOptions, extclient clientgoclientset
if s.Authentication.ServiceAccounts.Lookup {
authenticatorConfig.ServiceAccountTokenGetter = serviceaccountcontroller.NewGetterFromClient(extclient)
}
kubeAPIVersions := os.Getenv("KUBE_API_VERSIONS")
if len(kubeAPIVersions) == 0 {
authenticatorConfig.BootstrapTokenAuthenticator = bootstrap.NewTokenAuthenticator(
sharedInformers.Core().InternalVersion().Secrets().Lister().Secrets(v1.NamespaceSystem),
)
}
authenticatorConfig.BootstrapTokenAuthenticator = bootstrap.NewTokenAuthenticator(
sharedInformers.Core().InternalVersion().Secrets().Lister().Secrets(v1.NamespaceSystem),
)

return authenticatorConfig.New()
}
Expand Down
1 change: 0 additions & 1 deletion cmd/kube-controller-manager/app/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ go_library(
"//cmd/controller-manager/app/options:go_default_library",
"//cmd/kube-controller-manager/app/config:go_default_library",
"//cmd/kube-controller-manager/app/options:go_default_library",
"//pkg/api/legacyscheme:go_default_library",
"//pkg/apis/apps/install:go_default_library",
"//pkg/apis/authentication/install:go_default_library",
"//pkg/apis/authorization/install:go_default_library",
Expand Down
9 changes: 0 additions & 9 deletions cmd/kube-controller-manager/app/import_known_versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ package app

// These imports are the API groups the client will support.
import (
"fmt"

"k8s.io/kubernetes/pkg/api/legacyscheme"
_ "k8s.io/kubernetes/pkg/apis/apps/install"
_ "k8s.io/kubernetes/pkg/apis/authentication/install"
_ "k8s.io/kubernetes/pkg/apis/authorization/install"
Expand All @@ -38,9 +35,3 @@ import (
_ "k8s.io/kubernetes/pkg/apis/settings/install"
_ "k8s.io/kubernetes/pkg/apis/storage/install"
)

func init() {
if missingVersions := legacyscheme.Registry.ValidateEnvRequestedVersions(); len(missingVersions) != 0 {
panic(fmt.Sprintf("KUBE_API_VERSIONS contains versions that are not installed: %q.", missingVersions))
}
}
21 changes: 8 additions & 13 deletions hack/test-update-storage-objects.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ ETCD_PORT=${ETCD_PORT:-2379}
ETCD_PREFIX=${ETCD_PREFIX:-randomPrefix}
API_PORT=${API_PORT:-8080}
API_HOST=${API_HOST:-127.0.0.1}
KUBE_API_VERSIONS=""
RUNTIME_CONFIG=""

ETCDCTL=$(which etcdctl)
Expand All @@ -51,13 +50,12 @@ DISABLE_ADMISSION_PLUGINS="ServiceAccount,NamespaceLifecycle,LimitRanger,Mutatin
function startApiServer() {
local storage_versions=${1:-""}
local storage_media_type=${2:-""}
kube::log::status "Starting kube-apiserver with KUBE_API_VERSIONS: ${KUBE_API_VERSIONS}"
kube::log::status " and storage-media-type: ${storage_media_type}"
kube::log::status " and runtime-config: ${RUNTIME_CONFIG}"
kube::log::status " and storage-version overrides: ${storage_versions}"
kube::log::status "Starting kube-apiserver with..."
kube::log::status " storage-media-type: ${storage_media_type}"
kube::log::status " runtime-config: ${RUNTIME_CONFIG}"
kube::log::status " storage-version overrides: ${storage_versions}"

KUBE_API_VERSIONS="${KUBE_API_VERSIONS}" \
"${KUBE_OUTPUT_HOSTBIN}/kube-apiserver" \
"${KUBE_OUTPUT_HOSTBIN}/kube-apiserver" \
--insecure-bind-address="${API_HOST}" \
--bind-address="${API_HOST}" \
--insecure-port="${API_PORT}" \
Expand Down Expand Up @@ -120,8 +118,7 @@ KUBE_NEW_STORAGE_VERSIONS="storage.k8s.io/v1"
# but KUBE_OLD_API_VERSION is the latest (storage) version.
# Additionally use KUBE_STORAGE_MEDIA_TYPE_JSON for storage encoding.
#######################################################
KUBE_API_VERSIONS="v1,autoscaling/v1,${KUBE_OLD_API_VERSION},${KUBE_NEW_API_VERSION}"
RUNTIME_CONFIG="api/all=false,api/v1=true,${KUBE_OLD_API_VERSION}=true,${KUBE_NEW_API_VERSION}=true"
RUNTIME_CONFIG="api/all=false,api/v1=true,apiregistration.k8s.io/v1=true,${KUBE_OLD_API_VERSION}=true,${KUBE_NEW_API_VERSION}=true"
startApiServer ${KUBE_OLD_STORAGE_VERSIONS} ${KUBE_STORAGE_MEDIA_TYPE_JSON}


Expand Down Expand Up @@ -155,8 +152,7 @@ killApiServer
# Still use KUBE_STORAGE_MEDIA_TYPE_JSON for storage encoding.
#######################################################

KUBE_API_VERSIONS="v1,autoscaling/v1,${KUBE_NEW_API_VERSION},${KUBE_OLD_API_VERSION}"
RUNTIME_CONFIG="api/all=false,api/v1=true,${KUBE_OLD_API_VERSION}=true,${KUBE_NEW_API_VERSION}=true"
RUNTIME_CONFIG="api/all=false,api/v1=true,apiregistration.k8s.io/v1=true,${KUBE_OLD_API_VERSION}=true,${KUBE_NEW_API_VERSION}=true"
startApiServer ${KUBE_NEW_STORAGE_VERSIONS} ${KUBE_STORAGE_MEDIA_TYPE_JSON}

# Update etcd objects, so that will now be stored in the new api version.
Expand Down Expand Up @@ -186,8 +182,7 @@ killApiServer
# However, change storage encoding to KUBE_STORAGE_MEDIA_TYPE_PROTOBUF.
#######################################################

KUBE_API_VERSIONS="v1,autoscaling/v1,${KUBE_NEW_API_VERSION}"
RUNTIME_CONFIG="api/all=false,api/v1=true,${KUBE_NEW_API_VERSION}=true"
RUNTIME_CONFIG="api/all=false,api/v1=true,apiregistration.k8s.io/v1=true,${KUBE_NEW_API_VERSION}=true"

# This seems to reduce flakiness.
sleep 1
Expand Down
4 changes: 1 addition & 3 deletions pkg/api/legacyscheme/scheme.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,14 @@ limitations under the License.
package legacyscheme

import (
"os"

"k8s.io/apimachinery/pkg/apimachinery/registered"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
)

// Registry is an instance of an API registry. This is an interim step to start removing the idea of a global
// API registry.
var Registry = registered.NewOrDie(os.Getenv("KUBE_API_VERSIONS"))
var Registry = registered.NewAPIRegistrationManager()

// Scheme is the default instance of runtime.Scheme to which types in the Kubernetes API are already registered.
// NOTE: If you are copying this file to start a new api group, STOP! Copy the
Expand Down
4 changes: 1 addition & 3 deletions pkg/client/clientset_generated/internalclientset/scheme/register.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions pkg/kubeapiserver/options/storage_versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ func (s *StorageSerializationOptions) AddFlags(fs *pflag.FlagSet) {
"In the case where objects are moved from one group to the other, "+
"you may specify the format \"group1=group2/v1beta1,group3/v1beta1,...\". "+
"You only need to pass the groups you wish to change from the defaults. "+
"It defaults to a list of preferred versions of all registered groups, "+
"which is derived from the KUBE_API_VERSIONS environment variable.")
"It defaults to a list of preferred versions of all known groups.")

}
4 changes: 1 addition & 3 deletions pkg/kubectl/scheme/scheme.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package scheme

import (
"os"

"k8s.io/apimachinery/pkg/apimachinery/registered"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -29,7 +27,7 @@ import (
// All kubectl code should eventually switch to use this Registry and Scheme instead of the global ones.

// Registry is an instance of an API registry.
var Registry = registered.NewOrDie(os.Getenv("KUBE_API_VERSIONS"))
var Registry = registered.NewAPIRegistrationManager()

// Scheme is the default instance of runtime.Scheme to which types in the Kubernetes API are already registered.
var Scheme = runtime.NewScheme()
Expand Down
1 change: 0 additions & 1 deletion pkg/master/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ go_library(
],
importpath = "k8s.io/kubernetes/pkg/master",
deps = [
"//pkg/api/legacyscheme:go_default_library",
"//pkg/apis/admission/install:go_default_library",
"//pkg/apis/admissionregistration/install:go_default_library",
"//pkg/apis/apps/install:go_default_library",
Expand Down
10 changes: 0 additions & 10 deletions pkg/master/import_known_versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ package master

// These imports are the API groups the API server will support.
import (
"fmt"

"k8s.io/kubernetes/pkg/api/legacyscheme"

_ "k8s.io/kubernetes/pkg/apis/admission/install"
_ "k8s.io/kubernetes/pkg/apis/admissionregistration/install"
_ "k8s.io/kubernetes/pkg/apis/apps/install"
Expand All @@ -42,9 +38,3 @@ import (
_ "k8s.io/kubernetes/pkg/apis/settings/install"
_ "k8s.io/kubernetes/pkg/apis/storage/install"
)

func init() {
if missingVersions := legacyscheme.Registry.ValidateEnvRequestedVersions(); len(missingVersions) != 0 {
panic(fmt.Sprintf("KUBE_API_VERSIONS contains versions that are not installed: %q.", missingVersions))
}
}
3 changes: 1 addition & 2 deletions plugin/pkg/admission/eventratelimit/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"io"
"io/ioutil"
"os"

"k8s.io/apimachinery/pkg/apimachinery/registered"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -31,7 +30,7 @@ import (
)

var (
registry = registered.NewOrDie(os.Getenv("KUBE_API_VERSIONS"))
registry = registered.NewAPIRegistrationManager()
scheme = runtime.NewScheme()
codecs = serializer.NewCodecFactory(scheme)
)
Expand Down
3 changes: 1 addition & 2 deletions plugin/pkg/admission/podtolerationrestriction/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"io"
"io/ioutil"
"os"

"k8s.io/apimachinery/pkg/apimachinery/registered"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -32,7 +31,7 @@ import (
)

var (
registry = registered.NewOrDie(os.Getenv("KUBE_API_VERSIONS"))
registry = registered.NewAPIRegistrationManager()
scheme = runtime.NewScheme()
codecs = serializer.NewCodecFactory(scheme)
)
Expand Down
3 changes: 1 addition & 2 deletions plugin/pkg/admission/resourcequota/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"io"
"io/ioutil"
"os"

"k8s.io/apimachinery/pkg/apimachinery/registered"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -31,7 +30,7 @@ import (
)

var (
registry = registered.NewOrDie(os.Getenv("KUBE_API_VERSIONS"))
registry = registered.NewAPIRegistrationManager()
scheme = runtime.NewScheme()
codecs = serializer.NewCodecFactory(scheme)
)
Expand Down
22 changes: 2 additions & 20 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,8 @@ package apiserver
import (
"fmt"
"net/http"
"os"
"time"

"github.com/golang/glog"

"k8s.io/apimachinery/pkg/apimachinery/registered"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -52,7 +49,7 @@ import (
)

var (
Registry = registered.NewOrDie("")
Registry = registered.NewAPIRegistrationManager()
Scheme = runtime.NewScheme()
Codecs = serializer.NewCodecFactory(Scheme)

Expand Down Expand Up @@ -151,17 +148,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
if err != nil {
// it's really bad that this is leaking here, but until we can fix the test (which I'm pretty sure isn't even testing what it wants to test),
// we need to be able to move forward
kubeAPIVersions := os.Getenv("KUBE_API_VERSIONS")
if len(kubeAPIVersions) == 0 {
return nil, fmt.Errorf("failed to create clientset: %v", err)
}

// KUBE_API_VERSIONS is used in test-update-storage-objects.sh, disabling a number of API
// groups. This leads to a nil client above and undefined behaviour further down.
//
// TODO: get rid of KUBE_API_VERSIONS or define sane behaviour if set
glog.Errorf("Failed to create clientset with KUBE_API_VERSIONS=%q: %v. KUBE_API_VERSIONS is only for testing. Things will break.",
kubeAPIVersions, err)
return nil, fmt.Errorf("failed to create clientset: %v", err)
}
s.Informers = internalinformers.NewSharedInformerFactory(crdClient, 5*time.Minute)

Expand Down Expand Up @@ -189,11 +176,6 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
s.GenericAPIServer.Handler.NonGoRestfulMux.Handle("/apis", crdHandler)
s.GenericAPIServer.Handler.NonGoRestfulMux.HandlePrefix("/apis/", crdHandler)

// this only happens when KUBE_API_VERSIONS is set. We must return without adding controllers or poststarthooks which would affect healthz
if crdClient == nil {
return s, nil
}

crdController := NewDiscoveryController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), versionDiscoveryHandler, groupDiscoveryHandler)
namingController := status.NewNamingConditionController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), crdClient.Apiextensions())
finalizingController := finalizer.NewCRDFinalizer(
Expand Down

0 comments on commit dd5f030

Please sign in to comment.