update client cache to key on groupversion

pkg/kubectl/cmd/util/clientcache.go

@@ -25,8 +25,8 @@ import (
 
 func NewClientCache(loader clientcmd.ClientConfig) *ClientCache {
 	return &ClientCache{
-		clients: make(map[string]*client.Client),
-		configs: make(map[string]*client.Config),
+		clients: make(map[unversioned.GroupVersion]*client.Client),
+		configs: make(map[unversioned.GroupVersion]*client.Config),
 		loader:  loader,
 	}
 }
@@ -35,15 +35,15 @@ func NewClientCache(loader clientcmd.ClientConfig) *ClientCache {
 // is invoked only once
 type ClientCache struct {
 	loader        clientcmd.ClientConfig
-	clients       map[string]*client.Client
-	configs       map[string]*client.Config
+	clients       map[unversioned.GroupVersion]*client.Client
+	configs       map[unversioned.GroupVersion]*client.Config
 	defaultConfig *client.Config
 	defaultClient *client.Client
 	matchVersion  bool
 }
 
 // ClientConfigForVersion returns the correct config for a server
-func (c *ClientCache) ClientConfigForVersion(version string) (*client.Config, error) {
+func (c *ClientCache) ClientConfigForVersion(version *unversioned.GroupVersion) (*client.Config, error) {
 	if c.defaultConfig == nil {
 		config, err := c.loader.ClientConfig()
 		if err != nil {
@@ -56,20 +56,20 @@ func (c *ClientCache) ClientConfigForVersion(version string) (*client.Config, er
 			}
 		}
 	}
-	if config, ok := c.configs[version]; ok {
-		return config, nil
+	if version != nil {
+		if config, ok := c.configs[*version]; ok {
+			return config, nil
+		}
 	}
+
 	// TODO: have a better config copy method
 	config := *c.defaultConfig
 
 	// TODO these fall out when we finish the refactor
 	var preferredGV *unversioned.GroupVersion
-	if len(version) > 0 {
-		gv, err := unversioned.ParseGroupVersion(version)
-		if err != nil {
-			return nil, err
-		}
-		preferredGV = &gv
+	if version != nil {
+		versionCopy := *version
+		preferredGV = &versionCopy
 	}
 
 	negotiatedVersion, err := client.NegotiateVersion(c.defaultClient, &config, preferredGV, registered.RegisteredGroupVersions)
@@ -78,31 +78,49 @@ func (c *ClientCache) ClientConfigForVersion(version string) (*client.Config, er
 	}
 	config.GroupVersion = negotiatedVersion
 	client.SetKubernetesDefaults(&config)
-	c.configs[version] = &config
+
+	if version != nil {
+		c.configs[*version] = &config
+	}
 
 	// `version` does not necessarily equal `config.Version`.  However, we know that we call this method again with
 	// `config.Version`, we should get the the config we've just built.
 	configCopy := config
-	c.configs[config.GroupVersion.String()] = &configCopy
+	c.configs[*config.GroupVersion] = &configCopy
 
 	return &config, nil
 }
 
 // ClientForVersion initializes or reuses a client for the specified version, or returns an
 // error if that is not possible
-func (c *ClientCache) ClientForVersion(version string) (*client.Client, error) {
-	if client, ok := c.clients[version]; ok {
-		return client, nil
+func (c *ClientCache) ClientForVersion(version *unversioned.GroupVersion) (*client.Client, error) {
+	if version != nil {
+		if client, ok := c.clients[*version]; ok {
+			return client, nil
+		}
 	}
 	config, err := c.ClientConfigForVersion(version)
 	if err != nil {
 		return nil, err
 	}
-	client, err := client.New(config)
+
+	kubeclient, err := client.New(config)
 	if err != nil {
 		return nil, err
 	}
+	c.clients[*config.GroupVersion] = kubeclient
 
-	c.clients[config.GroupVersion.String()] = client
-	return client, nil
+	// `version` does not necessarily equal `config.Version`.  However, we know that if we call this method again with
+	// `version`, we should get a client based on the same config we just found.  There's no guarantee that a client
+	// is copiable, so create a new client and save it in the cache.
+	if version != nil {
+		configCopy := *config
+		kubeclient, err := client.New(&configCopy)
+		if err != nil {
+			return nil, err
+		}
+		c.clients[*version] = kubeclient
+	}
+
+	return kubeclient, nil
 }