From 3de27312d68a0f17c92ec30afffe592952eae6e9 Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Mon, 6 May 2019 18:31:05 -0700 Subject: [PATCH] fix(pkg/action): load clients after flags have been parsed Signed-off-by: Adam Reese --- Gopkg.lock | 5 +++-- cmd/helm/helm.go | 8 ++++---- cmd/helm/helm_test.go | 10 +++++----- cmd/helm/template.go | 8 ++++---- pkg/action/action.go | 40 +++++++++++++++++++++++++++++++++------ pkg/action/action_test.go | 8 ++++---- pkg/action/install.go | 10 ++++++++-- pkg/action/uninstall.go | 2 +- pkg/action/upgrade.go | 20 +------------------- 9 files changed, 64 insertions(+), 47 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 4edbe1b21..1467695ba 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1404,11 +1404,12 @@ revision = "44a48934c135b31e4f1c0d12e91d384e1cb2304c" [[projects]] - digest = "1:8774c035808c344c148ba5135b282d487561233ee3be10f6115ea9a9da5e1c56" + digest = "1:fe724e5bfc9e388624ccf76b77051c0c7da8695a264b3ab4103de270a8965b18" name = "k8s.io/client-go" packages = [ "discovery", "discovery/cached/disk", + "discovery/cached/memory", "discovery/fake", "dynamic", "dynamic/fake", @@ -1795,7 +1796,6 @@ "github.com/opencontainers/go-digest", "github.com/opencontainers/image-spec/specs-go/v1", "github.com/pkg/errors", - "github.com/sirupsen/logrus", "github.com/spf13/cobra", "github.com/spf13/cobra/doc", "github.com/spf13/pflag", @@ -1832,6 +1832,7 @@ "k8s.io/cli-runtime/pkg/genericclioptions", "k8s.io/cli-runtime/pkg/resource", "k8s.io/client-go/discovery", + "k8s.io/client-go/discovery/cached/memory", "k8s.io/client-go/kubernetes", "k8s.io/client-go/kubernetes/fake", "k8s.io/client-go/kubernetes/scheme", diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 199b8f369..8bd4dc45a 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -91,10 +91,10 @@ func newActionConfig(allNamespaces bool) *action.Configuration { } return &action.Configuration{ - KubeClient: kc, - Releases: store, - Discovery: clientset.Discovery(), - Log: logf, + RESTClientGetter: kubeConfig(), + KubeClient: kc, + Releases: store, + Log: logf, } } diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 755c0a3a2..c9013136e 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -26,10 +26,10 @@ import ( shellwords "github.com/mattn/go-shellwords" "github.com/spf13/cobra" - "k8s.io/client-go/kubernetes/fake" "helm.sh/helm/internal/test" "helm.sh/helm/pkg/action" + "helm.sh/helm/pkg/chartutil" "helm.sh/helm/pkg/helmpath" "helm.sh/helm/pkg/kube" "helm.sh/helm/pkg/release" @@ -115,10 +115,10 @@ func executeActionCommandC(store *storage.Storage, cmd string) (*cobra.Command, buf := new(bytes.Buffer) actionConfig := &action.Configuration{ - Releases: store, - KubeClient: &kube.PrintingKubeClient{Out: ioutil.Discard}, - Discovery: fake.NewSimpleClientset().Discovery(), - Log: func(format string, v ...interface{}) {}, + Releases: store, + KubeClient: &kube.PrintingKubeClient{Out: ioutil.Discard}, + Capabilities: chartutil.DefaultCapabilities, + Log: func(format string, v ...interface{}) {}, } root := newRootCmd(actionConfig, buf, args) diff --git a/cmd/helm/template.go b/cmd/helm/template.go index d3168c069..5dd723abe 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -23,10 +23,10 @@ import ( "strings" "github.com/spf13/cobra" - "k8s.io/client-go/kubernetes/fake" "helm.sh/helm/cmd/helm/require" "helm.sh/helm/pkg/action" + "helm.sh/helm/pkg/chartutil" "helm.sh/helm/pkg/kube" "helm.sh/helm/pkg/storage" "helm.sh/helm/pkg/storage/driver" @@ -48,9 +48,9 @@ To render just one template in a chart, use '-x': func newTemplateCmd(out io.Writer) *cobra.Command { customConfig := &action.Configuration{ // Add mock objects in here so it doesn't use Kube API server - Releases: storage.Init(driver.NewMemory()), - KubeClient: &kube.PrintingKubeClient{Out: ioutil.Discard}, - Discovery: fake.NewSimpleClientset().Discovery(), + Releases: storage.Init(driver.NewMemory()), + KubeClient: &kube.PrintingKubeClient{Out: ioutil.Discard}, + Capabilities: chartutil.DefaultCapabilities, Log: func(format string, v ...interface{}) { fmt.Fprintf(out, format, v...) }, diff --git a/pkg/action/action.go b/pkg/action/action.go index fbd51114f..11c3107d4 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -21,8 +21,10 @@ import ( "time" "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" "helm.sh/helm/pkg/chartutil" "helm.sh/helm/pkg/kube" @@ -61,8 +63,8 @@ var ValidName = regexp.MustCompile("^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+ // Configuration injects the dependencies that all actions share. type Configuration struct { - // Discovery contains a discovery client - Discovery discovery.DiscoveryInterface + // RESTClientGetter is an interface that loads Kuberbetes clients. + RESTClientGetter RESTClientGetter // Releases stores records of releases. Releases *storage.Storage @@ -73,17 +75,37 @@ type Configuration struct { // RegistryClient is a client for working with registries RegistryClient *registry.Client + // Capabilities describes the capabilities of the Kubernetes cluster. Capabilities *chartutil.Capabilities Log func(string, ...interface{}) } // capabilities builds a Capabilities from discovery information. -func (c *Configuration) capabilities() *chartutil.Capabilities { - if c.Capabilities == nil { - return chartutil.DefaultCapabilities +func (c *Configuration) getCapabilities() (*chartutil.Capabilities, error) { + if c.Capabilities != nil { + return c.Capabilities, nil } - return c.Capabilities + + dc, err := c.RESTClientGetter.ToDiscoveryClient() + if err != nil { + return nil, errors.Wrap(err, "could not get Kubernetes discovery client") + } + kubeVersion, err := dc.ServerVersion() + if err != nil { + return nil, errors.Wrap(err, "could not get server version from Kubernetes") + } + + apiVersions, err := GetVersionSet(dc) + if err != nil { + return nil, errors.Wrap(err, "could not get apiVersions from Kubernetes") + } + + c.Capabilities = &chartutil.Capabilities{ + KubeVersion: kubeVersion, + APIVersions: apiVersions, + } + return c.Capabilities, nil } // Now generates a timestamp @@ -131,3 +153,9 @@ func (c *Configuration) recordRelease(r *release.Release) { c.Log("warning: Failed to update release %s: %s", r.Name, err) } } + +type RESTClientGetter interface { + ToRESTConfig() (*rest.Config, error) + ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) + ToRESTMapper() (meta.RESTMapper, error) +} diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index a99fe44e2..f641457ea 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -23,9 +23,9 @@ import ( "time" "github.com/pkg/errors" - "k8s.io/client-go/kubernetes/fake" "helm.sh/helm/pkg/chart" + "helm.sh/helm/pkg/chartutil" "helm.sh/helm/pkg/kube" "helm.sh/helm/pkg/release" "helm.sh/helm/pkg/storage" @@ -38,9 +38,9 @@ func actionConfigFixture(t *testing.T) *Configuration { t.Helper() return &Configuration{ - Releases: storage.Init(driver.NewMemory()), - KubeClient: &kube.PrintingKubeClient{Out: ioutil.Discard}, - Discovery: fake.NewSimpleClientset().Discovery(), + Releases: storage.Init(driver.NewMemory()), + KubeClient: &kube.PrintingKubeClient{Out: ioutil.Discard}, + Capabilities: chartutil.DefaultCapabilities, Log: func(format string, v ...interface{}) { t.Helper() if *verbose { diff --git a/pkg/action/install.go b/pkg/action/install.go index 24d33d26a..090d650ba 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -115,7 +115,10 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { return nil, err } - caps := i.cfg.capabilities() + caps, err := i.cfg.getCapabilities() + if err != nil { + return nil, err + } options := chartutil.ReleaseOptions{ Name: i.ReleaseName, @@ -295,7 +298,10 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values hs := []*release.Hook{} b := bytes.NewBuffer(nil) - caps := c.capabilities() + caps, err := c.getCapabilities() + if err != nil { + return hs, b, "", err + } if ch.Metadata.KubeVersion != "" { gitVersion := caps.KubeVersion.String() diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 11d1b5225..6831e746b 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -203,7 +203,7 @@ func (u *Uninstall) execHook(hs []*release.Hook, namespace, hook string) error { // deleteRelease deletes the release and returns manifests that were kept in the deletion process func (u *Uninstall) deleteRelease(rel *release.Release) (kept string, errs []error) { - caps, err := newCapabilities(u.cfg.Discovery) + caps, err := u.cfg.getCapabilities() if err != nil { return rel.Manifest, []error{errors.Wrap(err, "could not get apiVersions from Kubernetes")} } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 2520fc181..477d47646 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -23,7 +23,6 @@ import ( "time" "github.com/pkg/errors" - "k8s.io/client-go/discovery" "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chartutil" @@ -150,7 +149,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele IsUpgrade: true, } - caps, err := newCapabilities(u.cfg.Discovery) + caps, err := u.cfg.getCapabilities() if err != nil { return nil, nil, err } @@ -275,23 +274,6 @@ func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release) erro return nil } -func newCapabilities(dc discovery.DiscoveryInterface) (*chartutil.Capabilities, error) { - kubeVersion, err := dc.ServerVersion() - if err != nil { - return nil, err - } - - apiVersions, err := GetVersionSet(dc) - if err != nil { - return nil, errors.Wrap(err, "could not get apiVersions from Kubernetes") - } - - return &chartutil.Capabilities{ - KubeVersion: kubeVersion, - APIVersions: apiVersions, - }, nil -} - func validateManifest(c kube.KubernetesClient, ns string, manifest []byte) error { _, err := c.BuildUnstructured(ns, bytes.NewReader(manifest)) return err