Merge pull request #18384 from deads2k/gv-clientcache

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
+
+	// `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
+	}
 
-	c.clients[config.GroupVersion.String()] = client
-	return client, nil
+	return kubeclient, nil
 }

pkg/kubectl/cmd/util/factory.go

@@ -148,17 +148,18 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			return kubectl.OutputVersionMapper{RESTMapper: mapper, OutputVersions: []unversioned.GroupVersion{cmdApiVersion}}, api.Scheme
 		},
 		Client: func() (*client.Client, error) {
-			return clients.ClientForVersion("")
+			return clients.ClientForVersion(nil)
 		},
 		ClientConfig: func() (*client.Config, error) {
-			return clients.ClientConfigForVersion("")
+			return clients.ClientConfigForVersion(nil)
 		},
 		RESTClient: func(mapping *meta.RESTMapping) (resource.RESTClient, error) {
 			gvk, err := api.RESTMapper.KindFor(mapping.Resource)
 			if err != nil {
 				return nil, err
 			}
-			client, err := clients.ClientForVersion(mapping.GroupVersionKind.GroupVersion().String())
+			mappingVersion := mapping.GroupVersionKind.GroupVersion()
+			client, err := clients.ClientForVersion(&mappingVersion)
 			if err != nil {
 				return nil, err
 			}
@@ -171,7 +172,8 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			return nil, fmt.Errorf("unable to get RESTClient for resource '%s'", mapping.Resource)
 		},
 		Describer: func(mapping *meta.RESTMapping) (kubectl.Describer, error) {
-			client, err := clients.ClientForVersion(mapping.GroupVersionKind.GroupVersion().String())
+			mappingVersion := mapping.GroupVersionKind.GroupVersion()
+			client, err := clients.ClientForVersion(&mappingVersion)
 			if err != nil {
 				return nil, err
 			}
@@ -227,7 +229,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			return meta.NewAccessor().Labels(object)
 		},
 		LogsForObject: func(object, options runtime.Object) (*client.Request, error) {
-			c, err := clients.ClientForVersion("")
+			c, err := clients.ClientForVersion(nil)
 			if err != nil {
 				return nil, err
 			}
@@ -248,14 +250,16 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			}
 		},
 		Scaler: func(mapping *meta.RESTMapping) (kubectl.Scaler, error) {
-			client, err := clients.ClientForVersion(mapping.GroupVersionKind.GroupVersion().String())
+			mappingVersion := mapping.GroupVersionKind.GroupVersion()
+			client, err := clients.ClientForVersion(&mappingVersion)
 			if err != nil {
 				return nil, err
 			}
 			return kubectl.ScalerFor(mapping.GroupVersionKind.GroupKind(), client)
 		},
 		Reaper: func(mapping *meta.RESTMapping) (kubectl.Reaper, error) {
-			client, err := clients.ClientForVersion(mapping.GroupVersionKind.GroupVersion().String())
+			mappingVersion := mapping.GroupVersionKind.GroupVersion()
+			client, err := clients.ClientForVersion(&mappingVersion)
 			if err != nil {
 				return nil, err
 			}
@@ -263,7 +267,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 		},
 		Validator: func(validate bool, cacheDir string) (validation.Schema, error) {
 			if validate {
-				client, err := clients.ClientForVersion("")
+				client, err := clients.ClientForVersion(nil)
 				if err != nil {
 					return nil, err
 				}
@@ -309,7 +313,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			return nil
 		},
 		AttachablePodForObject: func(object runtime.Object) (*api.Pod, error) {
-			client, err := clients.ClientForVersion("")
+			client, err := clients.ClientForVersion(nil)
 			if err != nil {
 				return nil, err
 			}