From b0a6828ac76e92b79958bfc2424e9766d39e6c54 Mon Sep 17 00:00:00 2001 From: Simon Alling Date: Wed, 12 Jan 2022 16:45:10 +0100 Subject: [PATCH] Only get logs if --logs is given Signed-off-by: Simon Alling --- cmd/helm/helm.go | 2 +- cmd/helm/helm_test.go | 5 ++- cmd/helm/list.go | 2 +- cmd/helm/release_testing.go | 8 ++-- cmd/helm/release_testing_test.go | 10 ++++- ...aml.txt => test-output-yaml-with-logs.txt} | 0 .../output/test-output-yaml-without-logs.txt | 41 +++++++++++++++++++ pkg/action/action.go | 22 ++++++---- pkg/action/action_test.go | 7 ++-- pkg/action/hooks.go | 4 +- pkg/action/release_testing.go | 4 +- 11 files changed, 81 insertions(+), 24 deletions(-) rename cmd/helm/testdata/output/{test-output-yaml.txt => test-output-yaml-with-logs.txt} (100%) create mode 100644 cmd/helm/testdata/output/test-output-yaml-without-logs.txt diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 1766f8646..123cb8608 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -76,7 +76,7 @@ func main() { // run when each command's execute method is called cobra.OnInitialize(func() { helmDriver := os.Getenv("HELM_DRIVER") - if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug); err != nil { + if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug, actionConfig.GetHookLogFromRealCluster); err != nil { log.Fatal(err) } if helmDriver == "memory" { diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 7e5f9e622..2e4e89139 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -120,8 +120,9 @@ func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string) KubeClient: &kubefake.PrintingKubeClient{Out: ioutil.Discard}, Capabilities: chartutil.DefaultCapabilities, Log: func(format string, v ...interface{}) {}, - GetHookLog: func(rel *release.Release, hook *release.Hook) (release.HookLog, error) { - return release.HookLog("example test pod log output"), nil + HookLogGetter: func(rel *release.Release, hook *release.Hook) (*release.HookLog, error) { + hookLog := release.HookLog("example test pod log output") + return &hookLog, nil }, } diff --git a/cmd/helm/list.go b/cmd/helm/list.go index c361b550d..345f9d927 100644 --- a/cmd/helm/list.go +++ b/cmd/helm/list.go @@ -71,7 +71,7 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: noCompletions, RunE: func(cmd *cobra.Command, args []string) error { if client.AllNamespaces { - if err := cfg.Init(settings.RESTClientGetter(), "", os.Getenv("HELM_DRIVER"), debug); err != nil { + if err := cfg.Init(settings.RESTClientGetter(), "", os.Getenv("HELM_DRIVER"), debug, cfg.GetHookLogFromRealCluster); err != nil { return err } } diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go index 05438f0a7..bec9fc108 100644 --- a/cmd/helm/release_testing.go +++ b/cmd/helm/release_testing.go @@ -55,6 +55,9 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command return compListReleases(toComplete, args, cfg) }, RunE: func(cmd *cobra.Command, args []string) error { + if !outputLogs { + cfg.HookLogGetter = cfg.GetNoHookLogAtAll + } client.Namespace = settings.Namespace() notName := regexp.MustCompile(`^!\s?name=`) for _, f := range filter { @@ -76,9 +79,6 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command return err } - // The logs are always included when JSON or YAML output is used. - // With table output, we print logs if and only if explicitly requested, - // to preserve backwards compatibility. if outfmt == output.Table && outputLogs { // Print a newline to stdout to separate the output fmt.Fprintln(out) @@ -93,7 +93,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command f := cmd.Flags() f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") - f.BoolVar(&outputLogs, "logs", false, "dump the logs from test pods even with table output (this runs after all tests are complete, but before any cleanup)") + f.BoolVar(&outputLogs, "logs", false, "dump the logs from test pods (this runs after all tests are complete, but before any cleanup)") f.StringSliceVar(&filter, "filter", []string{}, "specify tests by attribute (currently \"name\") using attribute=value syntax or '!attribute=value' to exclude a test (can specify multiple or separate values with commas: name=test1,name=test2)") bindOutputFlag(cmd, &outfmt) diff --git a/cmd/helm/release_testing_test.go b/cmd/helm/release_testing_test.go index eefb48d7a..712ea3b72 100644 --- a/cmd/helm/release_testing_test.go +++ b/cmd/helm/release_testing_test.go @@ -94,9 +94,15 @@ func TestReleaseTestingYamlOutput(t *testing.T) { tests := []cmdTestCase{ { - name: "test with yaml output format", + name: "test with yaml output format without logs", cmd: "test doge --output yaml", - golden: "output/test-output-yaml.txt", + golden: "output/test-output-yaml-without-logs.txt", + rels: mockReleases, + }, + { + name: "test with yaml output format with logs", + cmd: "test doge --output yaml --logs", + golden: "output/test-output-yaml-with-logs.txt", rels: mockReleases, }, } diff --git a/cmd/helm/testdata/output/test-output-yaml.txt b/cmd/helm/testdata/output/test-output-yaml-with-logs.txt similarity index 100% rename from cmd/helm/testdata/output/test-output-yaml.txt rename to cmd/helm/testdata/output/test-output-yaml-with-logs.txt diff --git a/cmd/helm/testdata/output/test-output-yaml-without-logs.txt b/cmd/helm/testdata/output/test-output-yaml-without-logs.txt new file mode 100644 index 000000000..20ab32254 --- /dev/null +++ b/cmd/helm/testdata/output/test-output-yaml-without-logs.txt @@ -0,0 +1,41 @@ +chart: + files: null + lock: null + metadata: + appVersion: "1.0" + name: foo + version: 0.1.0-beta.1 + schema: null + templates: + - data: YXBpVmVyc2lvbjogdjEKa2luZDogU2VjcmV0Cm1ldGFkYXRhOgogIG5hbWU6IGZpeHR1cmUK + name: templates/foo.tpl + values: null +config: + name: value +hooks: +- delete_policies: + - before-hook-creation + events: + - test + kind: Pod + last_run: + completed_at: "2021-04-20T14:00:00.000001337Z" + phase: Succeeded + started_at: "2021-04-20T13:37:00.000001337Z" + name: doge-test-pod + path: doge-test-pod +info: + deleted: "" + description: Release mock + first_deployed: "1977-09-02T22:04:05Z" + last_deployed: "1977-09-02T22:04:05Z" + notes: Some mock release notes! + status: deployed +manifest: | + apiVersion: v1 + kind: Secret + metadata: + name: fixture +name: doge +namespace: default +version: 1 diff --git a/pkg/action/action.go b/pkg/action/action.go index 48212c98e..65537cb97 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -99,9 +99,11 @@ type Configuration struct { Log func(string, ...interface{}) - GetHookLog func(rel *release.Release, hook *release.Hook) (release.HookLog, error) + HookLogGetter HookLogGetter } +type HookLogGetter func(rel *release.Release, hook *release.Hook) (*release.HookLog, error) + // renderResources renders the templates in a chart // // TODO: This function is badly in need of a refactor. @@ -281,9 +283,14 @@ func (cfg *Configuration) getCapabilities() (*chartutil.Capabilities, error) { return cfg.Capabilities, nil } -// getHookLogFromCluster gets the log from the pod associated with the given hook, which is expected to be a test hook. -func (cfg *Configuration) getHookLogFromCluster(rel *release.Release, hook *release.Hook) (release.HookLog, error) { - var nothing release.HookLog +// GetNoHookLogAtAll doesn't get any log. +func (cfg *Configuration) GetNoHookLogAtAll(rel *release.Release, hook *release.Hook) (*release.HookLog, error) { + return nil, nil +} + +// GetHookLogFromRealCluster gets the log from the pod associated with the given hook, which is expected to be a test hook. +func (cfg *Configuration) GetHookLogFromRealCluster(rel *release.Release, hook *release.Hook) (*release.HookLog, error) { + var nothing *release.HookLog client, err := cfg.KubernetesClientSet() if err != nil { return nothing, errors.Wrapf(err, "unable to create Kubernetes client set to fetch pod logs") @@ -298,7 +305,8 @@ func (cfg *Configuration) getHookLogFromCluster(rel *release.Release, hook *rele if err != nil { return nothing, errors.Wrapf(err, "unable to get pod logs for %s", hook.Name) } - return release.HookLog(stringBuilder.String()), nil + hookLog := release.HookLog(stringBuilder.String()) + return &hookLog, nil } // KubernetesClientSet creates a new kubernetes ClientSet based on the configuration @@ -387,7 +395,7 @@ func (cfg *Configuration) recordRelease(r *release.Release) { } // Init initializes the action configuration -func (cfg *Configuration) Init(getter genericclioptions.RESTClientGetter, namespace, helmDriver string, log DebugLog) error { +func (cfg *Configuration) Init(getter genericclioptions.RESTClientGetter, namespace, helmDriver string, log DebugLog, hookLogGetter HookLogGetter) error { kc := kube.New(getter) kc.Log = log @@ -440,7 +448,7 @@ func (cfg *Configuration) Init(getter genericclioptions.RESTClientGetter, namesp cfg.KubeClient = kc cfg.Releases = store cfg.Log = log - cfg.GetHookLog = cfg.getHookLogFromCluster + cfg.HookLogGetter = hookLogGetter return nil } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index d1f54650b..3c96a0363 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -50,8 +50,9 @@ func actionConfigFixture(t *testing.T) *Configuration { t.Fatal(err) } - mockHookLogGetter := func(rel *release.Release, hook *release.Hook) (release.HookLog, error) { - return release.HookLog("example test pod log output"), nil + mockHookLogGetter := func(rel *release.Release, hook *release.Hook) (*release.HookLog, error) { + hookLog := release.HookLog("example test pod log output") + return &hookLog, nil } return &Configuration{ @@ -65,7 +66,7 @@ func actionConfigFixture(t *testing.T) *Configuration { t.Logf(format, v...) } }, - GetHookLog: mockHookLogGetter, + HookLogGetter: mockHookLogGetter, } } diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 93eb90b4e..5f66eaf20 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -85,11 +85,11 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, h.LastRun.CompletedAt = helmtime.Now() if isTestHook(h) { - hookLog, err := cfg.GetHookLog(rl, h) + hookLog, err := cfg.HookLogGetter(rl, h) if err != nil { return err } - h.LastRun.Log = &hookLog + h.LastRun.Log = hookLog } // Mark hook as succeeded or failed diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index 901256c49..1bef3079d 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -102,11 +102,11 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) { func (r *ReleaseTesting) GetPodLogs(out io.Writer, rel *release.Release) error { for _, h := range rel.Hooks { if isTestHook(h) { - hookLog, err := r.cfg.GetHookLog(rel, h) + hookLog, err := r.cfg.HookLogGetter(rel, h) if err != nil { return err } - _, err = fmt.Fprintf(out, "POD LOGS: %s\n%s\n", h.Name, string(hookLog)) + _, err = fmt.Fprintf(out, "POD LOGS: %s\n%s\n", h.Name, string(*hookLog)) if err != nil { return errors.Wrapf(err, "unable to write pod logs for %s", h.Name) }