From 6050f59b7b5a0a0b84dc15c46473f7bc53e74212 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Tue, 21 Feb 2017 12:25:24 +0100 Subject: [PATCH 1/3] apimachinery: merge Scheme.AddKnownTypes and Scheme.AddKnownTypeWithName --- staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go index ad260ae491a..fa2e3220e40 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go @@ -163,22 +163,13 @@ func (s *Scheme) AddUnversionedTypes(version schema.GroupVersion, types ...Objec // the struct becomes the "kind" field when encoding. Version may not be empty - use the // APIVersionInternal constant if you have a type that does not have a formal version. func (s *Scheme) AddKnownTypes(gv schema.GroupVersion, types ...Object) { - if len(gv.Version) == 0 { - panic(fmt.Sprintf("version is required on all types: %s %v", gv, types[0])) - } for _, obj := range types { t := reflect.TypeOf(obj) if t.Kind() != reflect.Ptr { panic("All types must be pointers to structs.") } t = t.Elem() - if t.Kind() != reflect.Struct { - panic("All types must be pointers to structs.") - } - - gvk := gv.WithKind(t.Name()) - s.gvkToType[gvk] = t - s.typeToGVK[t] = append(s.typeToGVK[t], gvk) + s.AddKnownTypeWithName(gv.WithKind(t.Name()), obj) } } From 395be3b4010b0eef63e98961a14064360fdca26c Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Tue, 21 Feb 2017 12:49:19 +0100 Subject: [PATCH 2/3] apimachinery: handle duplicated and conflicting type registration --- .../util/eventsink/eventsink.go | 10 ++- pkg/api/serialization_test.go | 3 +- pkg/apis/autoscaling/v2alpha1/register.go | 5 -- pkg/apis/rbac/register.go | 2 - .../garbagecollector/metaonly/metaonly.go | 4 + .../k8s.io/apimachinery/pkg/runtime/scheme.go | 10 +++ .../apimachinery/pkg/runtime/scheme_test.go | 73 +++++++++++++++++++ .../apiserver/pkg/apis/apiserver/register.go | 2 - .../apiserver/pkg/apis/example/register.go | 2 - .../pkg/storage/etcd/etcd_helper_test.go | 1 - .../pkg/apis/apiregistration/register.go | 9 --- .../pkg/apis/wardle/register.go | 2 - 12 files changed, 95 insertions(+), 28 deletions(-) diff --git a/federation/pkg/federation-controller/util/eventsink/eventsink.go b/federation/pkg/federation-controller/util/eventsink/eventsink.go index b716ad5698d..b3dacbb2c5d 100644 --- a/federation/pkg/federation-controller/util/eventsink/eventsink.go +++ b/federation/pkg/federation-controller/util/eventsink/eventsink.go @@ -22,6 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" clientv1 "k8s.io/client-go/pkg/api/v1" "k8s.io/client-go/tools/record" @@ -48,10 +49,11 @@ func NewFederatedEventSink(clientset fedclientset.Interface) *FederatedEventSink var scheme = runtime.NewScheme() func init() { - scheme.AddKnownTypes(clientv1.SchemeGroupVersion, - &clientv1.Event{}, - &kubev1.Event{}, - ) + // register client-go's and kube's Event type under two different GroupVersions + // TODO: switch to client-go client for events + scheme.AddKnownTypes(clientv1.SchemeGroupVersion, &clientv1.Event{}) + scheme.AddKnownTypes(schema.GroupVersion{Group: "fake-kube-" + kubev1.SchemeGroupVersion.Group, Version: kubev1.SchemeGroupVersion.Version}, &kubev1.Event{}) + if err := scheme.AddConversionFuncs( metav1.Convert_unversioned_Time_To_unversioned_Time, ); err != nil { diff --git a/pkg/api/serialization_test.go b/pkg/api/serialization_test.go index a644dedf01d..8ac7dc6b395 100644 --- a/pkg/api/serialization_test.go +++ b/pkg/api/serialization_test.go @@ -174,7 +174,8 @@ func TestCommonKindsRegistered(t *testing.T) { t.Error(err) } defaults := gv.WithKind("") - if _, got, err := api.Codecs.LegacyCodec().Decode([]byte(`{"kind":"`+kind+`"}`), &defaults, nil); err != nil || gvk != *got { + var got *schema.GroupVersionKind + if obj, got, err = api.Codecs.LegacyCodec().Decode([]byte(`{"kind":"`+kind+`"}`), &defaults, obj); err != nil || gvk != *got { t.Errorf("expected %v: %v %v", gvk, got, err) } data, err := runtime.Encode(api.Codecs.LegacyCodec(*gv), obj) diff --git a/pkg/apis/autoscaling/v2alpha1/register.go b/pkg/apis/autoscaling/v2alpha1/register.go index cc2751bc079..2fc437f9393 100644 --- a/pkg/apis/autoscaling/v2alpha1/register.go +++ b/pkg/apis/autoscaling/v2alpha1/register.go @@ -20,7 +20,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/kubernetes/pkg/api/v1" ) // GroupName is the group name use in this package @@ -39,10 +38,6 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &HorizontalPodAutoscaler{}, &HorizontalPodAutoscalerList{}, - &v1.ListOptions{}, - &v1.DeleteOptions{}, - &metav1.GetOptions{}, - &metav1.ExportOptions{}, ) metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/pkg/apis/rbac/register.go b/pkg/apis/rbac/register.go index a7403d34b51..f4a838bd89a 100644 --- a/pkg/apis/rbac/register.go +++ b/pkg/apis/rbac/register.go @@ -17,7 +17,6 @@ limitations under the License. package rbac import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -55,6 +54,5 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ClusterRoleBindingList{}, &ClusterRoleList{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil } diff --git a/pkg/controller/garbagecollector/metaonly/metaonly.go b/pkg/controller/garbagecollector/metaonly/metaonly.go index cfe55b25ba7..bf7053c9b1d 100644 --- a/pkg/controller/garbagecollector/metaonly/metaonly.go +++ b/pkg/controller/garbagecollector/metaonly/metaonly.go @@ -52,6 +52,10 @@ func NewMetadataCodecFactory() serializer.CodecFactory { if kind.Version == runtime.APIVersionInternal { continue } + if kind == api.Unversioned.WithKind("Status") { + // this is added below as unversioned + continue + } metaOnlyObject := gvkToMetadataOnlyObject(kind) scheme.AddKnownTypeWithName(kind, metaOnlyObject) } diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go index fa2e3220e40..fbec6ad9bc7 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go @@ -190,7 +190,17 @@ func (s *Scheme) AddKnownTypeWithName(gvk schema.GroupVersionKind, obj Object) { panic("All types must be pointers to structs.") } + if oldT, found := s.gvkToType[gvk]; found && oldT != t { + panic(fmt.Sprintf("Double registration of different types for %v: old=%v.%v, new=%v.%v", gvk, oldT.PkgPath(), oldT.Name(), t.PkgPath(), t.Name())) + } + s.gvkToType[gvk] = t + + for _, existingGvk := range s.typeToGVK[t] { + if existingGvk == gvk { + return + } + } s.typeToGVK[t] = append(s.typeToGVK[t], gvk) } diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go index 970bde80f68..d34b491bd84 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go @@ -548,6 +548,79 @@ func TestKnownTypes(t *testing.T) { } } +func TestAddKnownTypesIdemPotent(t *testing.T) { + s := runtime.NewScheme() + + gv := schema.GroupVersion{Group: "foo", Version: "v1"} + s.AddKnownTypes(gv, &InternalSimple{}) + s.AddKnownTypes(gv, &InternalSimple{}) + if len(s.KnownTypes(gv)) != 1 { + t.Errorf("expected only one %v type after double registration", gv) + } + if len(s.AllKnownTypes()) != 1 { + t.Errorf("expected only one type after double registration") + } + + s.AddKnownTypeWithName(gv.WithKind("InternalSimple"), &InternalSimple{}) + s.AddKnownTypeWithName(gv.WithKind("InternalSimple"), &InternalSimple{}) + if len(s.KnownTypes(gv)) != 1 { + t.Errorf("expected only one %v type after double registration with custom name", gv) + } + if len(s.AllKnownTypes()) != 1 { + t.Errorf("expected only one type after double registration with custom name") + } + + s.AddUnversionedTypes(gv, &InternalSimple{}) + if len(s.KnownTypes(gv)) != 1 { + t.Errorf("expected only one %v type after double registration with custom name", gv) + } + if len(s.AllKnownTypes()) != 1 { + t.Errorf("expected only one type after double registration with custom name") + } + + kinds, _, err := s.ObjectKinds(&InternalSimple{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(kinds) != 1 { + t.Errorf("expected only one kind for InternalSimple after double registration", gv) + } +} + +func TestConflictingAddKnownTypes(t *testing.T) { + s := runtime.NewScheme() + gv := schema.GroupVersion{Group: "foo", Version: "v1"} + + panicked := make(chan bool) + go func() { + defer func() { + if recover() != nil { + panicked <- true + } + }() + s.AddKnownTypeWithName(gv.WithKind("InternalSimple"), &InternalSimple{}) + s.AddKnownTypeWithName(gv.WithKind("InternalSimple"), &ExternalSimple{}) + panicked <- false + }() + if !<-panicked { + t.Errorf("Expected AddKnownTypesWithName to panic with conflicting type registrations") + } + + go func() { + defer func() { + if recover() != nil { + panicked <- true + } + }() + s.AddUnversionedTypes(gv, &InternalSimple{}) + s.AddUnversionedTypes(gv, &InternalSimple{}) + panicked <- false + }() + if !<-panicked { + t.Errorf("Expected AddUnversionedTypes to panic with conflicting type registrations") + } +} + func TestConvertToVersionBasic(t *testing.T) { s := GetTestScheme() tt := &TestType1{A: "I'm not a pointer object"} diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/register.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/register.go index 4fd86879f1c..1410518b9ed 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/register.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/register.go @@ -17,7 +17,6 @@ limitations under the License. package apiserver import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -47,6 +46,5 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &AdmissionConfiguration{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/example/register.go b/staging/src/k8s.io/apiserver/pkg/apis/example/register.go index fad13e8642d..2b699c4b495 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/example/register.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/example/register.go @@ -17,7 +17,6 @@ limitations under the License. package example import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -48,6 +47,5 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &Pod{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd/etcd_helper_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd/etcd_helper_test.go index 5845e0a2cbe..4cce5f85a0d 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd/etcd_helper_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd/etcd_helper_test.go @@ -77,7 +77,6 @@ func testScheme(t *testing.T) (*runtime.Scheme, serializer.CodecFactory) { scheme := runtime.NewScheme() scheme.Log(t) scheme.AddKnownTypes(schema.GroupVersion{Version: runtime.APIVersionInternal}, &storagetesting.TestResource{}) - scheme.AddKnownTypes(schema.GroupVersion{Version: runtime.APIVersionInternal}, &storagetesting.TestResource{}) example.AddToScheme(scheme) examplev1.AddToScheme(scheme) if err := scheme.AddConversionFuncs( diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/register.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/register.go index 0f3aafa560b..6a18624f511 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/register.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/register.go @@ -17,12 +17,8 @@ limitations under the License. package apiregistration import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - - // we register here until we separate our scheme, which requires updates to client gen - kapi "k8s.io/client-go/pkg/api" ) const GroupName = "apiregistration.k8s.io" @@ -50,11 +46,6 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &APIService{}, &APIServiceList{}, - - &kapi.ListOptions{}, - &metav1.DeleteOptions{}, - &metav1.GetOptions{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil } diff --git a/staging/src/k8s.io/sample-apiserver/pkg/apis/wardle/register.go b/staging/src/k8s.io/sample-apiserver/pkg/apis/wardle/register.go index d7ce354ccf8..ee7d7ef6a46 100644 --- a/staging/src/k8s.io/sample-apiserver/pkg/apis/wardle/register.go +++ b/staging/src/k8s.io/sample-apiserver/pkg/apis/wardle/register.go @@ -17,7 +17,6 @@ limitations under the License. package wardle import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -48,6 +47,5 @@ func addKnownTypes(scheme *runtime.Scheme) error { &Flunder{}, &FlunderList{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil } From f11d76ae44f60b2a515096f6d2f50646a8bfc711 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Mon, 27 Feb 2017 22:01:57 +0100 Subject: [PATCH 3/3] Update client --- federation/pkg/federation-controller/util/eventsink/BUILD | 1 + .../client-go/pkg/apis/autoscaling/v2alpha1/register.go | 5 ----- staging/src/k8s.io/client-go/pkg/apis/rbac/register.go | 2 -- vendor/BUILD | 1 - 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/federation/pkg/federation-controller/util/eventsink/BUILD b/federation/pkg/federation-controller/util/eventsink/BUILD index dbd49de78bc..b52b87416be 100644 --- a/federation/pkg/federation-controller/util/eventsink/BUILD +++ b/federation/pkg/federation-controller/util/eventsink/BUILD @@ -18,6 +18,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/conversion", "//vendor:k8s.io/apimachinery/pkg/runtime", + "//vendor:k8s.io/apimachinery/pkg/runtime/schema", "//vendor:k8s.io/apimachinery/pkg/types", "//vendor:k8s.io/client-go/pkg/api/v1", "//vendor:k8s.io/client-go/tools/record", diff --git a/staging/src/k8s.io/client-go/pkg/apis/autoscaling/v2alpha1/register.go b/staging/src/k8s.io/client-go/pkg/apis/autoscaling/v2alpha1/register.go index 98535e5a365..2fc437f9393 100644 --- a/staging/src/k8s.io/client-go/pkg/apis/autoscaling/v2alpha1/register.go +++ b/staging/src/k8s.io/client-go/pkg/apis/autoscaling/v2alpha1/register.go @@ -20,7 +20,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/pkg/api/v1" ) // GroupName is the group name use in this package @@ -39,10 +38,6 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &HorizontalPodAutoscaler{}, &HorizontalPodAutoscalerList{}, - &v1.ListOptions{}, - &v1.DeleteOptions{}, - &metav1.GetOptions{}, - &metav1.ExportOptions{}, ) metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/staging/src/k8s.io/client-go/pkg/apis/rbac/register.go b/staging/src/k8s.io/client-go/pkg/apis/rbac/register.go index a7403d34b51..f4a838bd89a 100644 --- a/staging/src/k8s.io/client-go/pkg/apis/rbac/register.go +++ b/staging/src/k8s.io/client-go/pkg/apis/rbac/register.go @@ -17,7 +17,6 @@ limitations under the License. package rbac import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -55,6 +54,5 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ClusterRoleBindingList{}, &ClusterRoleList{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil } diff --git a/vendor/BUILD b/vendor/BUILD index 0c1452c6af5..252f530c8b2 100644 --- a/vendor/BUILD +++ b/vendor/BUILD @@ -15836,7 +15836,6 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/conversion", "//vendor:k8s.io/apimachinery/pkg/runtime", "//vendor:k8s.io/apimachinery/pkg/runtime/schema", - "//vendor:k8s.io/client-go/pkg/api", ], )