From d0082839424832897675b16aabc5905e095f8f99 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 11 Mar 2016 11:41:18 -0500 Subject: [PATCH] Tolerate multiple registered versions in a single group --- .../fake/generator_fake_for_clientset.go | 3 ++- pkg/api/install/install_test.go | 2 +- pkg/api/meta/multirestmapper.go | 18 ++++++++++++------ pkg/api/testapi/testapi.go | 2 +- pkg/apimachinery/registered/registered.go | 8 ++++++-- .../fake/clientset_generated.go | 3 ++- .../release_1_2/fake/clientset_generated.go | 3 ++- ...stentvolume_claim_binder_controller_test.go | 5 +++-- pkg/kubectl/cmd/util/factory.go | 2 +- 9 files changed, 30 insertions(+), 16 deletions(-) diff --git a/cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_clientset.go b/cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_clientset.go index 690c07893d49f..6eaf63aab80e4 100644 --- a/cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_clientset.go +++ b/cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_clientset.go @@ -70,6 +70,7 @@ func (g *genClientset) Imports(c *generator.Context) (imports []string) { // imports for the code in commonTemplate imports = append(imports, "k8s.io/kubernetes/pkg/api", + "k8s.io/kubernetes/pkg/apimachinery/registered", "k8s.io/kubernetes/pkg/client/testing/core", "k8s.io/kubernetes/pkg/client/typed/discovery", "k8s.io/kubernetes/pkg/runtime", @@ -118,7 +119,7 @@ func NewSimpleClientset(objects ...runtime.Object) *Clientset { } fakePtr := core.Fake{} - fakePtr.AddReactor("*", "*", core.ObjectReaction(o, api.RESTMapper)) + fakePtr.AddReactor("*", "*", core.ObjectReaction(o, registered.RESTMapper())) fakePtr.AddWatchReactor("*", core.DefaultWatchReactor(watch.NewFake(), nil)) diff --git a/pkg/api/install/install_test.go b/pkg/api/install/install_test.go index 0b161535023ed..07c7f5d51dd4c 100644 --- a/pkg/api/install/install_test.go +++ b/pkg/api/install/install_test.go @@ -80,7 +80,7 @@ func TestRESTMapper(t *testing.T) { rcGVK := gv.WithKind("ReplicationController") podTemplateGVK := gv.WithKind("PodTemplate") - if gvk, err := registered.GroupOrDie(internal.GroupName).RESTMapper.KindFor(internal.SchemeGroupVersion.WithResource("replicationcontrollers")); err != nil || gvk != rcGVK { + if gvk, err := registered.RESTMapper().KindFor(internal.SchemeGroupVersion.WithResource("replicationcontrollers")); err != nil || gvk != rcGVK { t.Errorf("unexpected version mapping: %v %v", gvk, err) } diff --git a/pkg/api/meta/multirestmapper.go b/pkg/api/meta/multirestmapper.go index 3071d45072c68..b720f8fa2ef34 100644 --- a/pkg/api/meta/multirestmapper.go +++ b/pkg/api/meta/multirestmapper.go @@ -22,6 +22,7 @@ import ( "k8s.io/kubernetes/pkg/api/unversioned" utilerrors "k8s.io/kubernetes/pkg/util/errors" + "k8s.io/kubernetes/pkg/util/sets" ) // MultiRESTMapper is a wrapper for multiple RESTMappers. @@ -169,24 +170,29 @@ func (m MultiRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...strin if len(allMappings) == 1 { return allMappings[0], nil } + if len(allMappings) > 1 { + return nil, fmt.Errorf("multiple matches found for %v in %v", gk, versions) + } if len(errors) > 0 { return nil, utilerrors.NewAggregate(errors) } - if len(allMappings) == 0 { - return nil, fmt.Errorf("no match found for %v in %v", gk, versions) - } - - return nil, fmt.Errorf("multiple matches found for %v in %v", gk, versions) + return nil, fmt.Errorf("no match found for %v in %v", gk, versions) } // AliasesForResource finds the first alias response for the provided mappers. func (m MultiRESTMapper) AliasesForResource(alias string) ([]string, bool) { + seenAliases := sets.NewString() allAliases := []string{} handled := false for _, t := range m { if currAliases, currOk := t.AliasesForResource(alias); currOk { - allAliases = append(allAliases, currAliases...) + for _, currAlias := range currAliases { + if !seenAliases.Has(currAlias) { + allAliases = append(allAliases, currAlias) + seenAliases.Insert(currAlias) + } + } handled = true } } diff --git a/pkg/api/testapi/testapi.go b/pkg/api/testapi/testapi.go index d5d5452adc667..b5da44abfff59 100644 --- a/pkg/api/testapi/testapi.go +++ b/pkg/api/testapi/testapi.go @@ -232,7 +232,7 @@ func (g TestGroup) ResourcePath(resource, namespace, name string) string { } func (g TestGroup) RESTMapper() meta.RESTMapper { - return registered.GroupOrDie(g.externalGroupVersion.Group).RESTMapper + return registered.RESTMapper() } // Get codec based on runtime.Object diff --git a/pkg/apimachinery/registered/registered.go b/pkg/apimachinery/registered/registered.go index 8903fd3a5ce15..a660c223f3148 100644 --- a/pkg/apimachinery/registered/registered.go +++ b/pkg/apimachinery/registered/registered.go @@ -181,9 +181,13 @@ func GroupOrDie(group string) *apimachinery.GroupMeta { // all other groups alphabetical. func RESTMapper(versionPatterns ...unversioned.GroupVersion) meta.RESTMapper { unionMapper := meta.MultiRESTMapper{} + unionedGroups := sets.NewString() for enabledVersion := range enabledVersions { - groupMeta := groupMetaMap[enabledVersion.Group] - unionMapper = append(unionMapper, groupMeta.RESTMapper) + if !unionedGroups.Has(enabledVersion.Group) { + unionedGroups.Insert(enabledVersion.Group) + groupMeta := groupMetaMap[enabledVersion.Group] + unionMapper = append(unionMapper, groupMeta.RESTMapper) + } } if len(versionPatterns) != 0 { diff --git a/pkg/client/clientset_generated/internalclientset/fake/clientset_generated.go b/pkg/client/clientset_generated/internalclientset/fake/clientset_generated.go index d2861b7d3c045..6c7f6313d8299 100644 --- a/pkg/client/clientset_generated/internalclientset/fake/clientset_generated.go +++ b/pkg/client/clientset_generated/internalclientset/fake/clientset_generated.go @@ -18,6 +18,7 @@ package fake import ( "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apimachinery/registered" clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/client/testing/core" "k8s.io/kubernetes/pkg/client/typed/discovery" @@ -39,7 +40,7 @@ func NewSimpleClientset(objects ...runtime.Object) *Clientset { } fakePtr := core.Fake{} - fakePtr.AddReactor("*", "*", core.ObjectReaction(o, api.RESTMapper)) + fakePtr.AddReactor("*", "*", core.ObjectReaction(o, registered.RESTMapper())) fakePtr.AddWatchReactor("*", core.DefaultWatchReactor(watch.NewFake(), nil)) diff --git a/pkg/client/clientset_generated/release_1_2/fake/clientset_generated.go b/pkg/client/clientset_generated/release_1_2/fake/clientset_generated.go index c99ad713b07c4..26822b3c80849 100644 --- a/pkg/client/clientset_generated/release_1_2/fake/clientset_generated.go +++ b/pkg/client/clientset_generated/release_1_2/fake/clientset_generated.go @@ -18,6 +18,7 @@ package fake import ( "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apimachinery/registered" clientset "k8s.io/kubernetes/pkg/client/clientset_generated/release_1_2" "k8s.io/kubernetes/pkg/client/testing/core" "k8s.io/kubernetes/pkg/client/typed/discovery" @@ -39,7 +40,7 @@ func NewSimpleClientset(objects ...runtime.Object) *Clientset { } fakePtr := core.Fake{} - fakePtr.AddReactor("*", "*", core.ObjectReaction(o, api.RESTMapper)) + fakePtr.AddReactor("*", "*", core.ObjectReaction(o, registered.RESTMapper())) fakePtr.AddWatchReactor("*", core.DefaultWatchReactor(watch.NewFake(), nil)) diff --git a/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go b/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go index 49be54c425066..37ef08c15bdc4 100644 --- a/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go +++ b/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/kubernetes/pkg/api/errors" "k8s.io/kubernetes/pkg/api/resource" "k8s.io/kubernetes/pkg/api/testapi" + "k8s.io/kubernetes/pkg/apimachinery/registered" "k8s.io/kubernetes/pkg/client/cache" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/client/testing/core" @@ -365,7 +366,7 @@ func TestExampleObjects(t *testing.T) { } clientset := &fake.Clientset{} - clientset.AddReactor("*", "*", core.ObjectReaction(o, api.RESTMapper)) + clientset.AddReactor("*", "*", core.ObjectReaction(o, registered.RESTMapper())) if reflect.TypeOf(scenario.expected) == reflect.TypeOf(&api.PersistentVolumeClaim{}) { pvc, err := clientset.Core().PersistentVolumeClaims("ns").Get("doesntmatter") @@ -432,7 +433,7 @@ func TestBindingWithExamples(t *testing.T) { } clientset := &fake.Clientset{} - clientset.AddReactor("*", "*", core.ObjectReaction(o, api.RESTMapper)) + clientset.AddReactor("*", "*", core.ObjectReaction(o, registered.RESTMapper())) pv, err := clientset.Core().PersistentVolumes().Get("any") if err != nil { diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 78b2503b40b8e..f87a36f09c78a 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -183,7 +183,7 @@ func DefaultGenerators(cmdName string) map[string]kubectl.Generator { // if optionalClientConfig is nil, then flags will be bound to a new clientcmd.ClientConfig. // if optionalClientConfig is not nil, then this factory will make use of it. func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory { - mapper := kubectl.ShortcutExpander{RESTMapper: api.RESTMapper} + mapper := kubectl.ShortcutExpander{RESTMapper: registered.RESTMapper()} flags := pflag.NewFlagSet("", pflag.ContinueOnError) flags.SetNormalizeFunc(util.WarnWordSepNormalizeFunc) // Warn for "_" flags