Use logger for helm output (#1585)

* use logger for helm output

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

* update integration test output

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

* make logging output configurable

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

* also compare stderr in integration tests

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

---------

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

cmd/root.go

@@ -60,7 +60,13 @@ func NewRootCmd(globalConfig *config.GlobalOptions) (*cobra.Command, error) {
 			case globalConfig.Quiet:
 				logLevel = "warn"
 			}
-			logger = helmexec.NewLogger(os.Stderr, logLevel)
+
+			// If the log output is not set, default to stderr.
+			logOut := globalConfig.LogOutput
+			if logOut == nil {
+				logOut = os.Stderr
+			}
+			logger = helmexec.NewLogger(logOut, logLevel)
 			globalConfig.SetLogger(logger)
 			return nil
 		},

pkg/config/global.go

@@ -3,6 +3,7 @@ package config
 import (
 	"errors"
 	"fmt"
+	"io"
 	"os"
 
 	"go.uber.org/zap"
@@ -65,6 +66,8 @@ type GlobalOptions struct {
 	Interactive bool
 	// Args is the list of arguments to pass to the Helm binary.
 	Args string
+	// LogOutput is the writer to use for writing logs.
+	LogOutput io.Writer
 }
 
 // Logger returns the logger to use.

pkg/helmexec/exec.go

@@ -279,7 +279,7 @@ func (helm *execer) SyncRelease(context HelmContext, name, chart string, flags .
 	flags = append(flags, "--history-max", strconv.Itoa(context.HistoryMax))
 
 	out, err := helm.exec(append(append(preArgs, "upgrade", "--install", name, chart), flags...), env, nil)
-	helm.write(nil, out)
+	helm.info(out)
 	return err
 }
 
@@ -288,7 +288,7 @@ func (helm *execer) ReleaseStatus(context HelmContext, name string, flags ...str
 	preArgs := make([]string, 0)
 	env := make(map[string]string)
 	out, err := helm.exec(append(append(preArgs, "status", name), flags...), env, nil)
-	helm.write(nil, out)
+	helm.info(out)
 	return err
 }
 
@@ -309,7 +309,7 @@ func (helm *execer) List(context HelmContext, filter string, flags ...string) (s
 	lines := strings.Split(string(out), "\n")
 	lines = lines[1:]
 	out = []byte(strings.Join(lines, "\n"))
-	helm.write(nil, out)
+	helm.info(out)
 	return string(out), err
 }
 
@@ -451,7 +451,7 @@ func (helm *execer) TemplateRelease(name string, chart string, flags ...string)
 }
 
 func (helm *execer) DiffRelease(context HelmContext, name, chart string, suppressDiff bool, flags ...string) error {
-	if context.Writer != nil {
+	if context.Writer != nil && !suppressDiff {
 		fmt.Fprintf(context.Writer, "Comparing release=%v, chart=%v\n", name, redactedURL(chart))
 	} else {
 		helm.logger.Infof("Comparing release=%v, chart=%v", name, redactedURL(chart))
@@ -491,6 +491,7 @@ func (helm *execer) DiffRelease(context HelmContext, name, chart string, suppres
 func (helm *execer) Lint(name, chart string, flags ...string) error {
 	helm.logger.Infof("Linting release=%v, chart=%v", name, chart)
 	out, err := helm.exec(append([]string{"lint", chart}, flags...), map[string]string{}, nil)
+	// Always write to stdout to write the linting result to eg. a file
 	helm.write(nil, out)
 	return err
 }
@@ -541,7 +542,7 @@ func (helm *execer) DeleteRelease(context HelmContext, name string, flags ...str
 	preArgs := make([]string, 0)
 	env := make(map[string]string)
 	out, err := helm.exec(append(append(preArgs, "delete", name), flags...), env, nil)
-	helm.write(nil, out)
+	helm.info(out)
 	return err
 }
 
@@ -551,7 +552,7 @@ func (helm *execer) TestRelease(context HelmContext, name string, flags ...strin
 	env := make(map[string]string)
 	args := []string{"test", name}
 	out, err := helm.exec(append(append(preArgs, args...), flags...), env, nil)
-	helm.write(nil, out)
+	helm.info(out)
 	return err
 }
 

test/integration/test-cases/diff-args.sh

@@ -3,26 +3,35 @@ diff_args_output_dir="${cases_dir}/diff-args/output"
 
 diff_args_tmp=$(mktemp -d)
 diff_args_reverse=${diff_args_tmp}/diff.args.build.yaml
+diff_args_reverse_stderr=${diff_args_tmp}/diff.args.build.stderr.yaml
 
 case_title="diff args"
 diff_out_file=${diff_args_output_dir}/diff
 apply_out_file=${diff_args_output_dir}/apply
+diff_out_stderr_file=${diff_args_output_dir}/diff-stderr
+apply_out_stderr_file=${diff_args_output_dir}/apply-stderr
 if [[ $EXTRA_HELMFILE_FLAGS == *--enable-live-output* ]]; then
-    apply_out_file=${diff_args_output_dir}/apply-live
     diff_out_file=${diff_args_output_dir}/diff-live
+    apply_out_file=${diff_args_output_dir}/apply-live
+    diff_out_stderr_file=${diff_args_output_dir}/diff-live-stderr
+    apply_out_stderr_file=${diff_args_output_dir}/apply-live-stderr
 fi
 
 test_start "$case_title"
 info "Comparing ${case_title} diff for output ${diff_args_reverse} with ${diff_out_file}"
+info "Comparing ${case_title} diff for output ${diff_args_reverse_stderr} with ${diff_out_stderr_file}"
 for i in $(seq 10); do
     info "Comparing diff-args diff log #$i"
-    ${helmfile} -f ${diff_args_input_dir}/helmfile.yaml diff > ${diff_args_reverse} || fail "\"helmfile diff\" shouldn't fail"
+    ${helmfile} -f ${diff_args_input_dir}/helmfile.yaml diff 1> ${diff_args_reverse} 2> ${diff_args_reverse_stderr} || fail "\"helmfile diff\" shouldn't fail"
     diff -u ${diff_out_file} ${diff_args_reverse} || fail "\"helmfile diff\" should be consistent"
+    diff -u ${diff_out_stderr_file} ${diff_args_reverse_stderr} || fail "\"helmfile diff\" should be consistent (stderr)"
     echo code=$?
 done
 info "Comparing ${case_title} apply for output ${diff_args_reverse} with ${apply_out_file}"
-${helmfile} -f ${diff_args_input_dir}/helmfile.yaml apply | grep -vE "^(LAST DEPLOYED|installed)"  > ${diff_args_reverse} || fail "\"helmfile apply\" shouldn't fail"
+info "Comparing ${case_title} apply for stdout ${diff_args_reverse_stderr} with ${apply_out_stderr_file}"
-diff -u ${apply_out_file} ${diff_args_reverse} || fail "\"helmfile apply\" should be consistent"
+${helmfile} -f ${diff_args_input_dir}/helmfile.yaml apply 1> ${diff_args_reverse} 2> ${diff_args_reverse_stderr} || fail "\"helmfile apply\" shouldn't fail"
+diff -u ${apply_out_file} <(grep -vE "^(LAST DEPLOYED|installed)" ${diff_args_reverse}) || fail "\"helmfile apply\" should be consistent"
+diff -u ${apply_out_stderr_file} <(grep -vE "^(LAST DEPLOYED|installed)" ${diff_args_reverse_stderr}) || fail "\"helmfile apply\" should be consistent (stderr)"
 echo "clean up diff args resources"
 ${helmfile} -f ${diff_args_input_dir}/helmfile.yaml destroy || fail "\"helmfile destroy\" shouldn't fail"
 test_pass "$case_title"

test/integration/test-cases/diff-args/output/apply

@@ -67,11 +67,3 @@ helmfile-tests, installed-httpbin, Service (v1) has been added:
 +     release: installed
 +   type: LoadBalancer
 
-Release "installed" does not exist. Installing it now.
-NAME: installed
-NAMESPACE: helmfile-tests
-STATUS: deployed
-REVISION: 1
-TEST SUITE: None
-
-

test/integration/test-cases/diff-args/output/apply-live

@@ -74,4 +74,3 @@ NAMESPACE: helmfile-tests
 STATUS: deployed
 REVISION: 1
 TEST SUITE: None
-

test/integration/test-cases/diff-args/output/apply-live-stderr

@@ -0,0 +1,10 @@
+Live output is enabled
+Building dependency release=installed, chart=../../../charts/httpbin
+Listing releases matching ^uninstalled$
+Upgrading release=installed, chart=../../../charts/httpbin
+Listing releases matching ^installed$
+
+
+UPDATED RELEASES:
+NAME        NAMESPACE        CHART                     VERSION   DURATION
+

test/integration/test-cases/diff-args/output/apply-stderr

@@ -0,0 +1,16 @@
+Building dependency release=installed, chart=../../../charts/httpbin
+Listing releases matching ^uninstalled$
+Upgrading release=installed, chart=../../../charts/httpbin
+Release "installed" does not exist. Installing it now.
+NAME: installed
+NAMESPACE: helmfile-tests
+STATUS: deployed
+REVISION: 1
+TEST SUITE: None
+
+Listing releases matching ^installed$
+
+
+UPDATED RELEASES:
+NAME        NAMESPACE        CHART                     VERSION   DURATION
+

test/integration/test-cases/diff-args/output/diff-live-stderr

@@ -0,0 +1,3 @@
+Live output is enabled
+Building dependency release=installed, chart=../../../charts/httpbin
+Listing releases matching ^uninstalled$

test/integration/test-cases/diff-args/output/diff-stderr

@@ -0,0 +1,2 @@
+Building dependency release=installed, chart=../../../charts/httpbin
+Listing releases matching ^uninstalled$

test/integration/test-cases/skip-diff-output/output/diff-result

@@ -1,4 +1,3 @@
-Comparing release=foo, chart=../../../charts/raw
 Comparing release=baz, chart=../../../charts/raw
 ********************
 

test/integration/test-cases/skip-diff-output/output/diff-result-live

@@ -13,5 +13,4 @@ helmfile-tests, baz-2, ConfigMap (v1) has been added:
 +   namespace: helmfile-tests
 + data:
 +   baz: BAZ
-Comparing release=foo, chart=../../../charts/raw
 Comparing release=baz, chart=../../../charts/raw