Merge pull request #41329 from deads2k/cli-02-negotiation

Automatic merge from submit-queue (batch tested with PRs 41299, 41325, 41386, 41329, 41418)

stop senseless negotiation

Most client commands don't respect a negotiated version at all.  If you request a particular version, then of course it should be respected, but if you have none to request, then the current negotiation step doesn't return anything useful so we may as well have nothing so we can at least detect the situation.

@jwforres @kubernetes/sig-cli-pr-reviews 

Added a TODO to make the negotiate function useful.  I think I'm inclined to remove it entirely unless someone can come up with a useful reason to have it.
This commit is contained in:
Kubernetes Submit Queue 2017-02-14 11:42:38 -08:00 committed by GitHub
commit f26890b801
2 changed files with 21 additions and 5 deletions

View File

@ -48,6 +48,9 @@ type ClientCache struct {
fedClientSets map[schema.GroupVersion]fedclientset.Interface
configs map[schema.GroupVersion]*restclient.Config
// noVersionConfig provides a cached config for the case of no required version specified
noVersionConfig *restclient.Config
matchVersion bool
defaultConfigLock sync.Mutex
@ -104,8 +107,10 @@ func (c *ClientCache) ClientConfigForVersion(requiredVersion *schema.GroupVersio
// before looking up from the cache
if requiredVersion != nil {
if config, ok := c.configs[*requiredVersion]; ok {
return config, nil
return copyConfig(config), nil
}
} else if c.noVersionConfig != nil {
return copyConfig(c.noVersionConfig), nil
}
negotiatedVersion, err := discovery.NegotiateVersion(discoveryClient, requiredVersion, api.Registry.EnabledVersions())
@ -118,15 +123,23 @@ func (c *ClientCache) ClientConfigForVersion(requiredVersion *schema.GroupVersio
oldclient.SetKubernetesDefaults(&config)
if requiredVersion != nil {
c.configs[*requiredVersion] = &config
c.configs[*requiredVersion] = copyConfig(&config)
} else {
c.noVersionConfig = copyConfig(&config)
}
// `version` does not necessarily equal `config.Version`. However, we know that we call this method again with
// `config.Version`, we should get the config we've just built.
configCopy := config
c.configs[*config.GroupVersion] = &configCopy
c.configs[*config.GroupVersion] = copyConfig(&config)
return &config, nil
return copyConfig(&config), nil
}
func copyConfig(in *restclient.Config) *restclient.Config {
configCopy := *in
copyGroupVersion := *configCopy.GroupVersion
configCopy.GroupVersion = &copyGroupVersion
return &configCopy
}
// ClientSetForVersion initializes or reuses a clientset for the specified version, or returns an

View File

@ -49,6 +49,9 @@ func MatchesServerVersion(clientVersion apimachineryversion.Info, client Discove
// preference.
// - If version is provided and the server does not support it,
// return an error.
// TODO negotiation should be reserved for cases where we need a version for a given group. In those cases, it should return an ordered list of
// server preferences. From that list, a separate function can match from an ordered list of client versions.
// This is not what the function has ever done before, but it makes more logical sense.
func NegotiateVersion(client DiscoveryInterface, requiredGV *schema.GroupVersion, clientRegisteredGVs []schema.GroupVersion) (*schema.GroupVersion, error) {
clientVersions := sets.String{}
for _, gv := range clientRegisteredGVs {