Merge 66f58ba655 into 37cc2fa5ce

cmd/helm/helm.go

@@ -75,6 +75,7 @@ func main() {
 		if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug); err != nil {
 			log.Fatal(err)
 		}
+		actionConfig.HookLogGetter = actionConfig.GetHookLogFromRealCluster
 		if helmDriver == "memory" {
 			loadReleasesInMemory(actionConfig)
 		}

cmd/helm/helm_test.go

@@ -46,6 +46,13 @@ func init() {
 }
 
 func runTestCmd(t *testing.T, tests []cmdTestCase) {
+	t.Helper()
+	runTestCmdWithCustomAssertion(t, tests, func(actualOutput, expectedFilename string) {
+		test.AssertGoldenString(t, actualOutput, expectedFilename)
+	})
+}
+
+func runTestCmdWithCustomAssertion(t *testing.T, tests []cmdTestCase, assertion func(actualOutput, expectedFilename string)) {
 	t.Helper()
 	for _, tt := range tests {
 		for i := 0; i <= tt.repeat; i++ {
@@ -67,7 +74,7 @@ func runTestCmd(t *testing.T, tests []cmdTestCase) {
 					t.Errorf("expected no error, got: '%v'", err)
 				}
 				if tt.golden != "" {
-					test.AssertGoldenString(t, out, tt.golden)
+					assertion(out, tt.golden)
 				}
 			})
 		}
@@ -91,10 +98,11 @@ func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string)
 	buf := new(bytes.Buffer)
 
 	actionConfig := &action.Configuration{
-		Releases:     store,
+		Releases:      store,
-		KubeClient:   &kubefake.PrintingKubeClient{Out: io.Discard},
+		KubeClient:    &kubefake.PrintingKubeClient{Out: io.Discard},
-		Capabilities: chartutil.DefaultCapabilities,
+		Capabilities:  chartutil.DefaultCapabilities,
-		Log:          func(format string, v ...interface{}) {},
+		Log:           func(format string, v ...interface{}) {},
+		HookLogGetter: test.GetMockedHookLog,
 	}
 
 	root, err := newRootCmd(actionConfig, buf, args)

cmd/helm/release_testing_test.go

@@ -17,9 +17,97 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
+	"regexp"
 	"testing"
+	"time"
+
+	"helm.sh/helm/v3/internal/test"
+	"helm.sh/helm/v3/pkg/release"
+	helmtime "helm.sh/helm/v3/pkg/time"
+)
+
+const (
+	tableLinePattern  = `^Last (Completed|Started):\s+(.+)$`
+	verbPosition      = 1 // in the line patterns
+	timestampPosition = 2 // in the line patterns
 )
 
+type outputFormat struct {
+	linePattern regexp.Regexp
+	checkTime   func(raw string) error
+}
+
+func TestReleaseTesting(t *testing.T) {
+	mockReleases := []*release.Release{
+		createMockRelease(),
+	}
+
+	tableOutput := outputFormat{
+		linePattern: *regexp.MustCompile(tableLinePattern),
+		checkTime: func(raw string) error {
+			_, err := helmtime.Parse(time.ANSIC, raw) // Layout/format must be the one actually used in the command output.
+			return err
+		},
+	}
+
+	tests := []cmdTestCase{
+		{
+			name:   "test without logs",
+			cmd:    "test shiny-shibe",
+			golden: "output/test-without-logs.txt",
+			rels:   mockReleases,
+		},
+		{
+			name:   "test with logs",
+			cmd:    "test shiny-shibe --logs",
+			golden: "output/test-with-logs.txt",
+			rels:   mockReleases,
+		},
+	}
+
+	runTestCmdWithCustomAssertion(t, tests, test.AssertGoldenStringWithCustomLineValidation(t, checkLineAs(tableOutput)))
+}
+
+func checkLineAs(out outputFormat) func(expected, actual string) (bool, error) {
+	return func(expected, actual string) (bool, error) {
+		expectedMatch := out.linePattern.FindStringSubmatch(expected)
+		if expectedMatch != nil {
+			maybeTimestamp := expectedMatch[timestampPosition]
+			if out.checkTime(maybeTimestamp) == nil {
+				// This line requires special treatment.
+				actualMatch := out.linePattern.FindStringSubmatch(actual)
+				if actualMatch == nil {
+					return true, fmt.Errorf("expected to match %v", out.linePattern)
+				}
+				expectedVerb := expectedMatch[verbPosition]
+				actualVerb := actualMatch[verbPosition]
+				if actualVerb != expectedVerb {
+					return true, fmt.Errorf("expected '%s', but found '%s'", expectedVerb, actualVerb)
+				}
+				actualTimestamp := actualMatch[timestampPosition]
+				if err := out.checkTime(actualTimestamp); err != nil {
+					return true, fmt.Errorf("expected timestamp of same format, but found '%s' (%s)", actualTimestamp, err.Error())
+				}
+				return true, nil // The actual line was identical to the expected one, modulo the point in time represented by the timestamp.
+			}
+		}
+		// This line does not require special treatment.
+		return false, nil
+	}
+}
+
+func createMockRelease() *release.Release {
+	rel := release.Mock(&release.MockReleaseOptions{Name: "shiny-shibe"})
+	rel.Hooks[0] = &release.Hook{
+		Name:   "shiny-shibe-test-pod",
+		Kind:   "Pod",
+		Path:   "shiny-shibe-test-pod",
+		Events: []release.HookEvent{release.HookTest},
+	}
+	return rel
+}
+
 func TestReleaseTestingCompletion(t *testing.T) {
 	checkReleaseCompletion(t, "test", false)
 }

cmd/helm/testdata/output/test-with-logs.txt

@@ -0,0 +1,14 @@
+NAME: shiny-shibe
+LAST DEPLOYED: Fri Sep  2 22:04:05 1977
+NAMESPACE: default
+STATUS: deployed
+REVISION: 1
+TEST SUITE:     shiny-shibe-test-pod
+Last Started:   Fri Apr 16 13:37:00 2021
+Last Completed: Fri Apr 16 14:00:00 2021
+Phase:          Succeeded
+NOTES:
+Some mock release notes!
+
+POD LOGS: shiny-shibe-test-pod
+example test pod log output

cmd/helm/testdata/output/test-without-logs.txt

@@ -0,0 +1,11 @@
+NAME: shiny-shibe
+LAST DEPLOYED: Fri Sep  2 22:04:05 1977
+NAMESPACE: default
+STATUS: deployed
+REVISION: 1
+TEST SUITE:     shiny-shibe-test-pod
+Last Started:   Fri Apr 16 13:37:00 2021
+Last Completed: Fri Apr 16 14:00:00 2021
+Phase:          Succeeded
+NOTES:
+Some mock release notes!

internal/test/test.go

@@ -19,10 +19,15 @@ package test
 import (
 	"bytes"
 	"flag"
+	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+
+	"helm.sh/helm/v3/pkg/release"
 )
 
 // UpdateGolden writes out the golden files with the latest values, rather than failing the test.
@@ -30,6 +35,7 @@ var updateGolden = flag.Bool("update", false, "update golden files")
 
 // TestingT describes a testing object compatible with the critical functions from the testing.T type
 type TestingT interface {
+	Errorf(format string, args ...interface{})
 	Fatal(...interface{})
 	Fatalf(string, ...interface{})
 	HelperT
@@ -60,6 +66,55 @@ func AssertGoldenFile(t TestingT, actualFileName string, expectedFilename string
 	AssertGoldenString(t, string(actual), expectedFilename)
 }
 
+// AssertGoldenStringWithCustomLineValidation asserts that the given string matches the contents of the given file, using the given function to check each line.
+// It is useful when the output is expected to contain information that cannot be predicted, such as timestamps.
+//
+// The line validation function must return a pair of values representing, respectively,
+//
+// 1. whether the expected line is "special" or not, and
+//
+// 2. if the expected line is special, the validity of the actual line.
+//
+// "Not special" means that the actual line must be exactly equal to the expected line to be considered valid.
+func AssertGoldenStringWithCustomLineValidation(t TestingT, checkLine func(expected, actual string) (bool, error)) func(actualOutput, expectedFilename string) {
+	t.Helper()
+	is := assert.New(t)
+	return func(actualOutput, expectedFilename string) {
+		expectedOutput, err := os.ReadFile(path(expectedFilename))
+		if err != nil {
+			t.Fatalf("%v", err)
+		}
+		expectedLines := lines(expectedOutput)
+		actualLines := lines([]byte(actualOutput))
+		expectedLineCount := len(expectedLines)
+		actualLineCount := len(actualLines)
+		for i := 0; i < max(expectedLineCount, actualLineCount); i++ {
+			lineNumber := i + 1
+			// We need to prevent index-out-of-range errors if the number of lines doesn't match between the expected and the actual output.
+			// But we cannot just use the empty string as a default value, because that's equivalent to downright ignoring trailing empty lines.
+			if lineNumber > expectedLineCount {
+				t.Errorf("Output should only have %d line(s), but has %d. Line %d is: %q", expectedLineCount, actualLineCount, lineNumber, actualLines[i])
+			} else if lineNumber > actualLineCount {
+				t.Errorf("Output should have %d line(s), but has only %d. Line %d should have been: %q", expectedLineCount, actualLineCount, lineNumber, expectedLines[i])
+			} else {
+				actualLine := actualLines[i]
+				expectedLine := expectedLines[i]
+				if isSpecialLine, err := checkLine(expectedLine, actualLine); isSpecialLine {
+					if err != nil {
+						t.Errorf("Unexpected content on line %d (%v): %s", lineNumber, err.Error(), actualLine)
+					}
+				} else {
+					is.Equal(expectedLine, actualLine, fmt.Sprintf("Line %d in the actual output does not match line %d in the expected output (%s).", lineNumber, lineNumber, expectedFilename))
+				}
+			}
+		}
+	}
+}
+
+func lines(raw []byte) []string {
+	return strings.Split(strings.TrimSuffix(string(normalize(raw)), "\n"), "\n") // We first remove the final newline (if any), so that e.g. the 2-line string "a\nb\n" is mapped to ["a", "b"] and not ["a", "b", ""].
+}
+
 func path(filename string) string {
 	if filepath.IsAbs(filename) {
 		return filename
@@ -84,6 +139,11 @@ func compare(actual []byte, filename string) error {
 	return nil
 }
 
+func GetMockedHookLog(rel *release.Release, hook *release.Hook) (*release.HookLog, error) {
+	hookLog := release.HookLog("example test pod log output")
+	return &hookLog, nil
+}
+
 func update(filename string, in []byte) error {
 	if !*updateGolden {
 		return nil
@@ -94,3 +154,10 @@ func update(filename string, in []byte) error {
 func normalize(in []byte) []byte {
 	return bytes.Replace(in, []byte("\r\n"), []byte("\n"), -1)
 }
+
+func max(x, y int) int {
+	if x > y {
+		return x
+	}
+	return y
+}

pkg/action/action.go

@@ -18,7 +18,9 @@ package action
 
 import (
 	"bytes"
+	"context"
 	"fmt"
+	"io"
 	"os"
 	"path"
 	"path/filepath"
@@ -26,6 +28,7 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/client-go/discovery"
@@ -95,8 +98,16 @@ type Configuration struct {
 	Capabilities *chartutil.Capabilities
 
 	Log func(string, ...interface{})
+
+	HookLogGetter HookLogGetter
 }
 
+// A HookLogGetter gets the log of the given hook.
+//
+// The empty string represents the output from a pod that didn't print anything (i.e. the empty log).
+// The nil pointer represents no log at all.
+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.
@@ -280,6 +291,27 @@ func (cfg *Configuration) getCapabilities() (*chartutil.Capabilities, error) {
 	return cfg.Capabilities, 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")
+	}
+	req := client.CoreV1().Pods(rel.Namespace).GetLogs(hook.Name, &v1.PodLogOptions{})
+	responseBody, err := req.Stream(context.Background())
+	if err != nil {
+		return nothing, errors.Wrapf(err, "unable to get pod logs for %s", hook.Name)
+	}
+	stringBuilder := new(strings.Builder)
+	_, err = io.Copy(stringBuilder, responseBody)
+	if err != nil {
+		return nothing, errors.Wrapf(err, "unable to get pod logs for %s", hook.Name)
+	}
+	hookLog := release.HookLog(stringBuilder.String())
+	return &hookLog, nil
+}
+
 // KubernetesClientSet creates a new kubernetes ClientSet based on the configuration
 func (cfg *Configuration) KubernetesClientSet() (kubernetes.Interface, error) {
 	conf, err := cfg.RESTClientGetter.ToRESTConfig()

pkg/action/action_test.go

@@ -22,6 +22,7 @@ import (
 
 	fakeclientset "k8s.io/client-go/kubernetes/fake"
 
+	"helm.sh/helm/v3/internal/test"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
 	kubefake "helm.sh/helm/v3/pkg/kube/fake"
@@ -53,6 +54,7 @@ func actionConfigFixture(t *testing.T) *Configuration {
 				t.Logf(format, v...)
 			}
 		},
+		HookLogGetter: test.GetMockedHookLog,
 	}
 }
 

pkg/action/release_testing.go

@@ -17,13 +17,11 @@ limitations under the License.
 package action
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"time"
 
 	"github.com/pkg/errors"
-	v1 "k8s.io/api/core/v1"
 
 	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/release"
@@ -102,23 +100,14 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) {
 // the given writer. These can be immediately output to the user or captured for
 // other uses
 func (r *ReleaseTesting) GetPodLogs(out io.Writer, rel *release.Release) error {
-	client, err := r.cfg.KubernetesClientSet()
-	if err != nil {
-		return errors.Wrap(err, "unable to get kubernetes client to fetch pod logs")
-	}
-
 	for _, h := range rel.Hooks {
 		for _, e := range h.Events {
 			if e == release.HookTest {
-				req := client.CoreV1().Pods(r.Namespace).GetLogs(h.Name, &v1.PodLogOptions{})
+				hookLog, err := r.cfg.HookLogGetter(rel, h)
-				logReader, err := req.Stream(context.Background())
 				if err != nil {
-					return errors.Wrapf(err, "unable to get pod logs for %s", h.Name)
+					return err
 				}
-
+				_, err = fmt.Fprintf(out, "POD LOGS: %s\n%s\n", h.Name, string(*hookLog))
-				fmt.Fprintf(out, "POD LOGS: %s\n", h.Name)
-				_, err = io.Copy(out, logReader)
-				fmt.Fprintln(out)
 				if err != nil {
 					return errors.Wrapf(err, "unable to write pod logs for %s", h.Name)
 				}

pkg/action/release_testing_test.go

@@ -0,0 +1,72 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package action
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"helm.sh/helm/v3/pkg/release"
+)
+
+func releaseTestingAction(t *testing.T) *ReleaseTesting {
+	config := actionConfigFixture(t)
+	testAction := NewReleaseTesting(config)
+	return testAction
+}
+
+func TestReleaseTesting(t *testing.T) {
+	is := assert.New(t)
+
+	testAction := releaseTestingAction(t)
+
+	rel := releaseStub()
+	testAction.cfg.Releases.Create(rel)
+	expectedConfigMapHook := release.Hook{
+		Name: "test-cm",
+		Kind: "ConfigMap",
+		Path: "test-cm",
+		LastRun: release.HookExecution{
+			Phase: "",
+		},
+	}
+	expectedPodHook := release.Hook{
+		Name: "finding-nemo",
+		Kind: "Pod",
+		Path: "finding-nemo",
+		LastRun: release.HookExecution{
+			Phase: "Succeeded",
+		},
+	}
+	res, err := testAction.Run(rel.Name)
+	is.NoError(err)
+	is.Equal("angry-panda", res.Name)
+	is.Len(res.Hooks, 2, "The action seems to have changed the number of hooks on the release.")
+	checkHook(t, expectedConfigMapHook, res.Hooks[0])
+	checkHook(t, expectedPodHook, res.Hooks[1])
+}
+
+func checkHook(t *testing.T, expected release.Hook, actual *release.Hook) {
+	t.Helper()
+	is := assert.New(t)
+	is.Equal(expected.Name, actual.Name)
+	is.Equal(expected.Kind, actual.Kind)
+	is.Equal(expected.Path, actual.Path)
+	is.Equal(expected.LastRun.Phase, actual.LastRun.Phase)
+	// Cannot expect start and completion times because they cannot be predicted.
+}

pkg/release/hook.go

@@ -78,6 +78,9 @@ type Hook struct {
 	DeletePolicies []HookDeletePolicy `json:"delete_policies,omitempty"`
 }
 
+// A HookLog represents a log associated with a hook.
+type HookLog string
+
 // A HookExecution records the result for the last execution of a hook for a given release.
 type HookExecution struct {
 	// StartedAt indicates the date/time this hook was started