Add dummy test case to prove that not all hooks are delted on failure

Signed-off-by: Laszlo Uveges <laszlo@giantswarm.io>

pkg/action/hooks_test.go

@@ -1,208 +1,167 @@
-/*
-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 (
-	"bytes"
+	"helm.sh/helm/v3/pkg/chartutil"
-	"fmt"
+	"helm.sh/helm/v3/pkg/kube"
+	kubefake "helm.sh/helm/v3/pkg/kube/fake"
+	"helm.sh/helm/v3/pkg/release"
+	"helm.sh/helm/v3/pkg/storage"
+	"helm.sh/helm/v3/pkg/storage/driver"
 	"io"
+	"io/ioutil"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/cli-runtime/pkg/resource"
+	"reflect"
 	"testing"
-
+	"time"
-	"github.com/stretchr/testify/assert"
-
-	chart "helm.sh/helm/v4/pkg/chart/v2"
-	kubefake "helm.sh/helm/v4/pkg/kube/fake"
-	release "helm.sh/helm/v4/pkg/release/v1"
 )
 
-func podManifestWithOutputLogs(hookDefinitions []release.HookOutputLogPolicy) string {
+type HookFailedError struct{}
-	hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions)
-	return fmt.Sprintf(`kind: Pod
-metadata:
- name: finding-sharky,
- annotations:
-   "helm.sh/hook": pre-install
-   "helm.sh/hook-output-log-policy": %s
-spec:
- containers:
- - name: sharky-test
-   image: fake-image
-   cmd: fake-command`, hookDefinitionString)
-}
 
-func podManifestWithOutputLogWithNamespace(hookDefinitions []release.HookOutputLogPolicy) string {
+func (e *HookFailedError) Error() string {
-	hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions)
+	return "Hook failed!"
-	return fmt.Sprintf(`kind: Pod
-metadata:
- name: finding-george
- namespace: sneaky-namespace
- annotations:
-   "helm.sh/hook": pre-install
-   "helm.sh/hook-output-log-policy": %s
-spec:
- containers:
- - name: george-test
-   image: fake-image
-   cmd: fake-command`, hookDefinitionString)
 }
 
-func jobManifestWithOutputLog(hookDefinitions []release.HookOutputLogPolicy) string {
+type HookFailingKubeClient struct {
-	hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions)
+	kubefake.PrintingKubeClient
-	return fmt.Sprintf(`kind: Job
+	failOn       resource.Info
-apiVersion: batch/v1
+	deleteRecord []resource.Info
-metadata:
- name: losing-religion
- annotations:
-   "helm.sh/hook": pre-install
-   "helm.sh/hook-output-log-policy": %s
-spec:
-  completions: 1
-  parallelism: 1
-  activeDeadlineSeconds: 30
-  template:
-    spec:
-    containers:
-    - name: religion-container
-      image: religion-image
-      cmd: religion-command`, hookDefinitionString)
 }
 
-func jobManifestWithOutputLogWithNamespace(hookDefinitions []release.HookOutputLogPolicy) string {
+func (_ *HookFailingKubeClient) Build(reader io.Reader, _ bool) (kube.ResourceList, error) {
-	hookDefinitionString := convertHooksToCommaSeparated(hookDefinitions)
+	configMap := &v1.ConfigMap{}
-	return fmt.Sprintf(`kind: Job
-apiVersion: batch/v1
-metadata:
- name: losing-religion
- namespace: rem-namespace
- annotations:
-   "helm.sh/hook": pre-install
-   "helm.sh/hook-output-log-policy": %s
-spec:
-  completions: 1
-  parallelism: 1
-  activeDeadlineSeconds: 30
-  template:
-    spec:
-    containers:
-    - name: religion-container
-      image: religion-image
-      cmd: religion-command`, hookDefinitionString)
-}
 
-func convertHooksToCommaSeparated(hookDefinitions []release.HookOutputLogPolicy) string {
+	err := yaml.NewYAMLOrJSONDecoder(reader, 1000).Decode(configMap)
-	var commaSeparated string
-	for i, policy := range hookDefinitions {
-		if i+1 == len(hookDefinitions) {
-			commaSeparated += policy.String()
-		} else {
-			commaSeparated += policy.String() + ","
-		}
-	}
-	return commaSeparated
-}
 
-func TestInstallRelease_HookOutputLogsOnFailure(t *testing.T) {
+	if err != nil {
-	// Should output on failure with expected namespace if hook-failed is set
+		return kube.ResourceList{}, err
-	runInstallForHooksWithFailure(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "spaced", true)
-	runInstallForHooksWithFailure(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "sneaky-namespace", true)
-	runInstallForHooksWithFailure(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "spaced", true)
-	runInstallForHooksWithFailure(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "rem-namespace", true)
-
-	// Should not output on failure with expected namespace if hook-succeed is set
-	runInstallForHooksWithFailure(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false)
-	runInstallForHooksWithFailure(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false)
-	runInstallForHooksWithFailure(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false)
-	runInstallForHooksWithFailure(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "", false)
 	}
 
-func TestInstallRelease_HookOutputLogsOnSuccess(t *testing.T) {
+	return kube.ResourceList{{
-	// Should output on success with expected namespace if hook-succeeded is set
+		Name:      configMap.Name,
-	runInstallForHooksWithSuccess(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "spaced", true)
+		Namespace: configMap.Namespace,
-	runInstallForHooksWithSuccess(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "sneaky-namespace", true)
+	}}, nil
-	runInstallForHooksWithSuccess(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "spaced", true)
-	runInstallForHooksWithSuccess(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded}), "rem-namespace", true)
-
-	// Should not output on success if hook-failed is set
-	runInstallForHooksWithSuccess(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false)
-	runInstallForHooksWithSuccess(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false)
-	runInstallForHooksWithSuccess(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false)
-	runInstallForHooksWithSuccess(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnFailed}), "", false)
 }
 
-func TestInstallRelease_HooksOutputLogsOnSuccessAndFailure(t *testing.T) {
+func (h *HookFailingKubeClient) WatchUntilReady(resources kube.ResourceList, duration time.Duration) error {
-	// Should output on success with expected namespace if hook-succeeded and hook-failed is set
+	for _, res := range resources {
-	runInstallForHooksWithSuccess(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true)
+		if res.Name == h.failOn.Name && res.Namespace == h.failOn.Namespace {
-	runInstallForHooksWithSuccess(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "sneaky-namespace", true)
+			return &HookFailedError{}
-	runInstallForHooksWithSuccess(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true)
+		}
-	runInstallForHooksWithSuccess(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "rem-namespace", true)
-
-	// Should output on failure if hook-succeeded and hook-failed is set
-	runInstallForHooksWithFailure(t, podManifestWithOutputLogs([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true)
-	runInstallForHooksWithFailure(t, podManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "sneaky-namespace", true)
-	runInstallForHooksWithFailure(t, jobManifestWithOutputLog([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "spaced", true)
-	runInstallForHooksWithFailure(t, jobManifestWithOutputLogWithNamespace([]release.HookOutputLogPolicy{release.HookOutputOnSucceeded, release.HookOutputOnFailed}), "rem-namespace", true)
 	}
 
-func runInstallForHooksWithSuccess(t *testing.T, manifest, expectedNamespace string, shouldOutput bool) {
+	return h.PrintingKubeClient.WatchUntilReady(resources, duration)
-	var expectedOutput string
-	if shouldOutput {
-		expectedOutput = fmt.Sprintf("attempted to output logs for namespace: %s", expectedNamespace)
 }
-	is := assert.New(t)
+
-	instAction := installAction(t)
+func (h *HookFailingKubeClient) Delete(resources kube.ResourceList) (*kube.Result, []error) {
-	instAction.ReleaseName = "failed-hooks"
+	for _, res := range resources {
-	outBuffer := &bytes.Buffer{}
+		h.deleteRecord = append(h.deleteRecord, resource.Info{
-	instAction.cfg.KubeClient = &kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer}
+			Name:      res.Name,
-
+			Namespace: res.Namespace,
-	templates := []*chart.File{
+		})
-		{Name: "templates/hello", Data: []byte("hello: world")},
-		{Name: "templates/hooks", Data: []byte(manifest)},
 	}
-	vals := map[string]interface{}{}
 
-	res, err := instAction.Run(buildChartWithTemplates(templates), vals)
+	return h.PrintingKubeClient.Delete(resources)
-	is.NoError(err)
-	is.Equal(expectedOutput, outBuffer.String())
-	is.Equal(release.StatusDeployed, res.Info.Status)
 }
 
-func runInstallForHooksWithFailure(t *testing.T, manifest, expectedNamespace string, shouldOutput bool) {
+func TestHooksCleanUp(t *testing.T) {
-	var expectedOutput string
+	hookKubeClient := &HookFailingKubeClient{kubefake.PrintingKubeClient{Out: ioutil.Discard}, resource.Info{
-	if shouldOutput {
+		Name:      "build-config-2",
-		expectedOutput = fmt.Sprintf("attempted to output logs for namespace: %s", expectedNamespace)
+		Namespace: "test",
+	}, []resource.Info{}}
+
+	configuration := &Configuration{
+		Releases:     storage.Init(driver.NewMemory()),
+		KubeClient:   hookKubeClient,
+		Capabilities: chartutil.DefaultCapabilities,
+		Log: func(format string, v ...interface{}) {
+			t.Helper()
+			if *verbose {
+				t.Logf(format, v...)
 			}
-	is := assert.New(t)
+		},
-	instAction := installAction(t)
-	instAction.ReleaseName = "failed-hooks"
-	failingClient := instAction.cfg.KubeClient.(*kubefake.FailingKubeClient)
-	failingClient.WatchUntilReadyError = fmt.Errorf("failed watch")
-	instAction.cfg.KubeClient = failingClient
-	outBuffer := &bytes.Buffer{}
-	failingClient.PrintingKubeClient = kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: outBuffer}
-
-	templates := []*chart.File{
-		{Name: "templates/hello", Data: []byte("hello: world")},
-		{Name: "templates/hooks", Data: []byte(manifest)},
 	}
-	vals := map[string]interface{}{}
 
-	res, err := instAction.Run(buildChartWithTemplates(templates), vals)
+	hookEvent := release.HookPreInstall
-	is.Error(err)
+
-	is.Contains(res.Info.Description, "failed pre-install")
+	r := &release.Release{
-	is.Equal(expectedOutput, outBuffer.String())
+		Name:      "test-release",
-	is.Equal(release.StatusFailed, res.Info.Status)
+		Namespace: "test",
+		Hooks: []*release.Hook{
+			{
+				Name: "hook-1",
+				Kind: "ConfigMap",
+				Path: "templates/service_account.yaml",
+				Manifest: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: build-config-1
+  namespace: test
+data:
+  foo: bar
+`,
+				Weight: -5,
+				Events: []release.HookEvent{
+					hookEvent,
+				},
+				DeletePolicies: []release.HookDeletePolicy{
+					release.HookBeforeHookCreation,
+					release.HookSucceeded,
+					release.HookFailed,
+				},
+				LastRun: release.HookExecution{
+					Phase: release.HookPhaseSucceeded,
+				},
+			},
+			{
+				Name: "hook-2",
+				Kind: "ConfigMap",
+				Path: "templates/job.yaml",
+				Manifest: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: build-config-2
+  namespace: test
+data:
+  foo: bar
+`,
+				Weight: 0,
+				Events: []release.HookEvent{
+					hookEvent,
+				},
+				DeletePolicies: []release.HookDeletePolicy{
+					release.HookBeforeHookCreation,
+					release.HookSucceeded,
+					release.HookFailed,
+				},
+				LastRun: release.HookExecution{
+					Phase: release.HookPhaseFailed,
+				},
+			},
+		},
+	}
+
+	_ = configuration.execHook(r, hookEvent, 600)
+
+	if !reflect.DeepEqual(hookKubeClient.deleteRecord, []resource.Info{
+		{
+			Name:      "build-config-1",
+			Namespace: "test",
+		},
+		{
+			Name:      "build-config-2",
+			Namespace: "test",
+		},
+		{
+			Name:      "build-config-2",
+			Namespace: "test",
+		},
+	}) {
+		t.Fatalf("Got unexpected delete record")
+	}
+
+	//if err != nil {
+	//	t.Fatalf("An expected error occured: %#v", err)
+	//}
 }