Tolerate multiple registered versions in a single group

xfye · Mar 12, 2016 · d008283 · d008283
1 parent c9250bb
commit d008283
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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