diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD index be54b00ccbf..0619b7cc609 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD @@ -33,6 +33,7 @@ go_library( "//vendor/k8s.io/apiextensions-apiserver/pkg/controller/status:go_default_library", "//vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresource:go_default_library", "//vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apimachinery/announced:go_default_library", diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 50b2f4fe007..4470684447f 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -30,6 +30,7 @@ import ( "github.com/go-openapi/validate" "github.com/golang/glog" + apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -79,6 +80,11 @@ type crdHandler struct { // crdInfo stores enough information to serve the storage for the custom resource type crdInfo struct { + // spec and acceptedNames are used to compare against if a change is made on a CRD. We only update + // the storage if one of these changes. + spec *apiextensions.CustomResourceDefinitionSpec + acceptedNames *apiextensions.CustomResourceDefinitionNames + storage *customresource.REST requestScope handlers.RequestScope } @@ -108,6 +114,9 @@ func NewCustomResourceDefinitionHandler( crdInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ UpdateFunc: ret.updateCustomResourceDefinition, + DeleteFunc: func(obj interface{}) { + ret.removeDeadStorage() + }, }) ret.customStorage.Store(crdStorageMap{}) @@ -245,7 +254,7 @@ func (r *crdHandler) removeDeadStorage() { return } - for uid := range storageMap { + for uid, s := range storageMap { found := false for _, crd := range allCustomResourceDefinitions { if crd.UID == uid { @@ -254,6 +263,8 @@ func (r *crdHandler) removeDeadStorage() { } } if !found { + glog.V(4).Infof("Removing dead CRD storage for %v", s.requestScope.Resource) + s.storage.DestroyFunc() delete(storageMap, uid) } } @@ -301,7 +312,7 @@ func (r *crdHandler) getServingInfoFor(crd *apiextensions.CustomResourceDefiniti parameterScheme.AddGeneratedDeepCopyFuncs(metav1.GetGeneratedDeepCopyFuncs()...) parameterCodec := runtime.NewParameterCodec(parameterScheme) - kind := schema.GroupVersionKind{Group: crd.Spec.Group, Version: crd.Spec.Version, Kind: crd.Spec.Names.Kind} + kind := schema.GroupVersionKind{Group: crd.Spec.Group, Version: crd.Spec.Version, Kind: crd.Status.AcceptedNames.Kind} typer := unstructuredObjectTyper{ delegate: parameterScheme, unstructuredTyper: discovery.NewUnstructuredObjectTyper(nil), @@ -319,8 +330,8 @@ func (r *crdHandler) getServingInfoFor(crd *apiextensions.CustomResourceDefiniti validator := validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default) storage := customresource.NewREST( - schema.GroupResource{Group: crd.Spec.Group, Resource: crd.Spec.Names.Plural}, - schema.GroupVersionKind{Group: crd.Spec.Group, Version: crd.Spec.Version, Kind: crd.Spec.Names.ListKind}, + schema.GroupResource{Group: crd.Spec.Group, Resource: crd.Status.AcceptedNames.Plural}, + schema.GroupVersionKind{Group: crd.Spec.Group, Version: crd.Spec.Version, Kind: crd.Status.AcceptedNames.ListKind}, customresource.NewStrategy( typer, crd.Spec.Scope == apiextensions.NamespaceScoped, @@ -367,7 +378,7 @@ func (r *crdHandler) getServingInfoFor(crd *apiextensions.CustomResourceDefiniti Typer: typer, UnsafeConvertor: unstructured.UnstructuredObjectConverter{}, - Resource: schema.GroupVersionResource{Group: crd.Spec.Group, Version: crd.Spec.Version, Resource: crd.Spec.Names.Plural}, + Resource: schema.GroupVersionResource{Group: crd.Spec.Group, Version: crd.Spec.Version, Resource: crd.Status.AcceptedNames.Plural}, Kind: kind, Subresource: "", @@ -375,6 +386,9 @@ func (r *crdHandler) getServingInfoFor(crd *apiextensions.CustomResourceDefiniti } ret = &crdInfo{ + spec: &crd.Spec, + acceptedNames: &crd.Status.AcceptedNames, + storage: storage, requestScope: requestScope, } @@ -410,23 +424,36 @@ func (c crdObjectConverter) ConvertFieldLabel(version, kind, label, value string } } -func (c *crdHandler) updateCustomResourceDefinition(oldObj, _ interface{}) { +func (c *crdHandler) updateCustomResourceDefinition(oldObj, newObj interface{}) { oldCRD := oldObj.(*apiextensions.CustomResourceDefinition) - glog.V(4).Infof("Updating customresourcedefinition %s", oldCRD.Name) + newCRD := newObj.(*apiextensions.CustomResourceDefinition) c.customStorageLock.Lock() defer c.customStorageLock.Unlock() - storageMap := c.customStorage.Load().(crdStorageMap) + + oldInfo, found := storageMap[newCRD.UID] + if !found { + return + } + if apiequality.Semantic.DeepEqual(&newCRD.Spec, oldInfo.spec) && apiequality.Semantic.DeepEqual(&newCRD.Status.AcceptedNames, oldInfo.acceptedNames) { + glog.V(6).Infof("Ignoring customresourcedefinition %s update because neither spec, nor accepted names changed", oldCRD.Name) + return + } + + glog.V(4).Infof("Updating customresourcedefinition %s", oldCRD.Name) storageMap2 := make(crdStorageMap, len(storageMap)) // Copy because we cannot write to storageMap without a race // as it is used without locking elsewhere for k, v := range storageMap { + if k == oldCRD.UID { + v.storage.DestroyFunc() + continue + } storageMap2[k] = v } - delete(storageMap2, oldCRD.UID) c.customStorage.Store(storageMap2) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go index 3c771ef1825..d540fa59b88 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/wait" @@ -379,15 +380,18 @@ func TestCRValidationOnCRDUpdate(t *testing.T) { // update the CRD to a less stricter schema gottenCRD.Spec.Validation.OpenAPIV3Schema.Required = []string{"alpha", "beta"} - - updatedCRD, err := apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(gottenCRD) - if err != nil { + if _, err = apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(gottenCRD); err != nil { t.Fatal(err) } // CR is now accepted err = wait.Poll(500*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) { - _, err = instantiateCustomResource(t, newNoxuValidationInstance(ns, "foo"), noxuResourceClient, updatedCRD) + _, err := noxuResourceClient.Create(newNoxuValidationInstance(ns, "foo")) + if statusError, isStatus := err.(*apierrors.StatusError); isStatus { + if strings.Contains(statusError.Error(), "is invalid") { + return false, nil + } + } if err != nil { return false, err }