diff --git a/pkg/kubectl/cmd/util/BUILD b/pkg/kubectl/cmd/util/BUILD index 4033061a65..e39d587f1f 100644 --- a/pkg/kubectl/cmd/util/BUILD +++ b/pkg/kubectl/cmd/util/BUILD @@ -113,13 +113,10 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apimachinery/pkg/version:go_default_library", "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library", - "//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library", "//vendor/k8s.io/client-go/discovery:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", "//vendor/k8s.io/client-go/rest/fake:go_default_library", "//vendor/k8s.io/client-go/testing:go_default_library", - "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", - "//vendor/k8s.io/client-go/tools/clientcmd/api:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", ], ) diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 7d0c4350eb..167f9f2cef 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -118,8 +118,6 @@ type ClientAccessFactory interface { // LabelsForObject returns the labels associated with the provided object LabelsForObject(object runtime.Object) (map[string]string, error) - // Returns internal flagset - FlagSet() *pflag.FlagSet // Command will stringify and return all environment arguments ie. a command run by a client // using the factory. Command(cmd *cobra.Command, showSecrets bool) string diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index 2e662bbed0..34e27f8c78 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/pkg/kubectl/cmd/util/factory_client_access.go @@ -68,9 +68,9 @@ import ( ) type ring0Factory struct { - flags *pflag.FlagSet - clientConfig clientcmd.ClientConfig - discoveryFactory DiscoveryClientFactory + flags *pflag.FlagSet + clientConfig clientcmd.ClientConfig + discoveryCacheDir string requireMatchedServerVersion bool checkServerVersion sync.Once @@ -85,62 +85,16 @@ func NewClientAccessFactory(optionalClientConfig clientcmd.ClientConfig) ClientA clientConfig = DefaultClientConfig(flags) } - return NewClientAccessFactoryFromDiscovery(flags, clientConfig, &discoveryFactory{clientConfig: clientConfig}) -} - -// NewClientAccessFactoryFromDiscovery allows an external caller to substitute a different discoveryFactory -// Which allows for the client cache to be built in ring0, but still rely on a custom discovery client -func NewClientAccessFactoryFromDiscovery(flags *pflag.FlagSet, clientConfig clientcmd.ClientConfig, discoveryFactory DiscoveryClientFactory) ClientAccessFactory { flags.SetNormalizeFunc(utilflag.WarnWordSepNormalizeFunc) // Warn for "_" flags f := &ring0Factory{ - flags: flags, - clientConfig: clientConfig, - discoveryFactory: discoveryFactory, + flags: flags, + clientConfig: clientConfig, } return f } -type discoveryFactory struct { - clientConfig clientcmd.ClientConfig - cacheDir string -} - -func (f *discoveryFactory) DiscoveryClient() (discovery.CachedDiscoveryInterface, error) { - cfg, err := f.clientConfig.ClientConfig() - if err != nil { - return nil, err - } - - // The more groups you have, the more discovery requests you need to make. - // given 25 groups (our groups + a few custom resources) with one-ish version each, discovery needs to make 50 requests - // double it just so we don't end up here again for a while. This config is only used for discovery. - cfg.Burst = 100 - - if f.cacheDir != "" { - wt := cfg.WrapTransport - cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper { - if wt != nil { - rt = wt(rt) - } - return transport.NewCacheRoundTripper(f.cacheDir, rt) - } - } - - discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg) - if err != nil { - return nil, err - } - cacheDir := computeDiscoverCacheDir(filepath.Join(homedir.HomeDir(), ".kube", "cache", "discovery"), cfg.Host) - return NewCachedDiscoveryClient(discoveryClient, cacheDir, time.Duration(10*time.Minute)), nil -} - -func (f *discoveryFactory) BindFlags(flags *pflag.FlagSet) { - defaultCacheDir := filepath.Join(homedir.HomeDir(), ".kube", "http-cache") - flags.StringVar(&f.cacheDir, FlagHTTPCacheDir, defaultCacheDir, "Default HTTP cache directory") -} - // DefaultClientConfig creates a clientcmd.ClientConfig with the following hierarchy: // 1. Use the kubeconfig builder. The number of merges and overrides here gets a little crazy. Stay with me. // 1. Merge the kubeconfig itself. This is done with the following hierarchy rules: @@ -201,7 +155,32 @@ func DefaultClientConfig(flags *pflag.FlagSet) clientcmd.ClientConfig { } func (f *ring0Factory) DiscoveryClient() (discovery.CachedDiscoveryInterface, error) { - return f.discoveryFactory.DiscoveryClient() + cfg, err := f.clientConfig.ClientConfig() + if err != nil { + return nil, err + } + + // The more groups you have, the more discovery requests you need to make. + // given 25 groups (our groups + a few custom resources) with one-ish version each, discovery needs to make 50 requests + // double it just so we don't end up here again for a while. This config is only used for discovery. + cfg.Burst = 100 + + if f.discoveryCacheDir != "" { + wt := cfg.WrapTransport + cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper { + if wt != nil { + rt = wt(rt) + } + return transport.NewCacheRoundTripper(f.discoveryCacheDir, rt) + } + } + + discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg) + if err != nil { + return nil, err + } + cacheDir := computeDiscoverCacheDir(filepath.Join(homedir.HomeDir(), ".kube", "cache", "discovery"), cfg.Host) + return NewCachedDiscoveryClient(discoveryClient, cacheDir, time.Duration(10*time.Minute)), nil } func (f *ring0Factory) KubernetesClientSet() (*kubernetes.Clientset, error) { @@ -387,10 +366,6 @@ func (f *ring0Factory) LabelsForObject(object runtime.Object) (map[string]string return meta.NewAccessor().Labels(object) } -func (f *ring0Factory) FlagSet() *pflag.FlagSet { - return f.flags -} - // Set showSecrets false to filter out stuff like secrets. func (f *ring0Factory) Command(cmd *cobra.Command, showSecrets bool) string { if len(os.Args) == 0 { @@ -432,7 +407,8 @@ func (f *ring0Factory) BindFlags(flags *pflag.FlagSet) { // to do that automatically for every subcommand. flags.BoolVar(&f.requireMatchedServerVersion, FlagMatchBinaryVersion, false, "Require server version to match client version") - f.discoveryFactory.BindFlags(flags) + defaultCacheDir := filepath.Join(homedir.HomeDir(), ".kube", "http-cache") + flags.StringVar(&f.discoveryCacheDir, FlagHTTPCacheDir, defaultCacheDir, "Default HTTP cache directory") // Normalize all flags that are coming from other packages or pre-configurations // a.k.a. change all "_" to "-". e.g. glog package diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index e86298fc62..6c6dfa16ce 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/pkg/kubectl/cmd/util/factory_test.go @@ -32,11 +32,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/watch" - "k8s.io/apiserver/pkg/util/flag" manualfake "k8s.io/client-go/rest/fake" testcore "k8s.io/client-go/testing" - "k8s.io/client-go/tools/clientcmd" - clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/testapi" api "k8s.io/kubernetes/pkg/apis/core" @@ -47,23 +44,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl/resource" ) -func TestNewFactoryDefaultFlagBindings(t *testing.T) { - factory := NewFactory(nil) - - if !factory.FlagSet().HasFlags() { - t.Errorf("Expected flags, but didn't get any") - } -} - -func TestNewFactoryNoFlagBindings(t *testing.T) { - clientConfig := clientcmd.NewDefaultClientConfig(*clientcmdapi.NewConfig(), &clientcmd.ConfigOverrides{}) - factory := NewFactory(clientConfig) - - if factory.FlagSet().HasFlags() { - t.Errorf("Expected zero flags, but got %v", factory.FlagSet()) - } -} - func TestPortsForObject(t *testing.T) { f := NewFactory(nil) @@ -211,18 +191,6 @@ func TestCanBeExposed(t *testing.T) { } } -func TestFlagUnderscoreRenaming(t *testing.T) { - factory := NewFactory(nil) - - factory.FlagSet().SetNormalizeFunc(flag.WordSepNormalizeFunc) - factory.FlagSet().Bool("valid_flag", false, "bool value") - - // In case of failure of this test check this PR: spf13/pflag#23 - if factory.FlagSet().Lookup("valid_flag").Name != "valid-flag" { - t.Fatalf("Expected flag name to be valid-flag, got %s", factory.FlagSet().Lookup("valid_flag").Name) - } -} - func newPodList(count, isUnready, isUnhealthy int, labels map[string]string) *api.PodList { pods := []api.Pod{} for i := 0; i < count; i++ {