Merge pull request #8527 from mattfarina/fix-hooks-always-executing

Fix issue with install and upgrade running all hooks

cmd/helm/template.go

@@ -20,6 +20,8 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -79,6 +81,25 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 			if rel != nil {
 				var manifests bytes.Buffer
 				fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest))
+				if !client.DisableHooks {
+					fileWritten := make(map[string]bool)
+					for _, m := range rel.Hooks {
+						if client.OutputDir == "" {
+							fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest)
+						} else {
+							newDir := client.OutputDir
+							if client.UseReleaseName {
+								newDir = filepath.Join(client.OutputDir, client.ReleaseName)
+							}
+							err = writeToFile(newDir, m.Path, m.Manifest, fileWritten[m.Path])
+							if err != nil {
+								return err
+							}
+							fileWritten[m.Path] = true
+						}
+
+					}
+				}
 
 				// if we have a list of files to render, then check that each of the
 				// provided files exists in the chart.
@@ -149,3 +170,50 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 
 	return cmd
 }
+
+// The following functions (writeToFile, createOrOpenFile, and ensureDirectoryForFile)
+// are coppied from the actions package. This is part of a change to correct a
+// bug introduced by #8156. As part of the todo to refactor renderResources
+// this duplicate code should be removed. It is added here so that the API
+// surface area is as minimally impacted as possible in fixing the issue.
+func writeToFile(outputDir string, name string, data string, append bool) error {
+	outfileName := strings.Join([]string{outputDir, name}, string(filepath.Separator))
+
+	err := ensureDirectoryForFile(outfileName)
+	if err != nil {
+		return err
+	}
+
+	f, err := createOrOpenFile(outfileName, append)
+	if err != nil {
+		return err
+	}
+
+	defer f.Close()
+
+	_, err = f.WriteString(fmt.Sprintf("---\n# Source: %s\n%s\n", name, data))
+
+	if err != nil {
+		return err
+	}
+
+	fmt.Printf("wrote %s\n", outfileName)
+	return nil
+}
+
+func createOrOpenFile(filename string, append bool) (*os.File, error) {
+	if append {
+		return os.OpenFile(filename, os.O_APPEND|os.O_WRONLY, 0600)
+	}
+	return os.Create(filename)
+}
+
+func ensureDirectoryForFile(file string) error {
+	baseDir := path.Dir(file)
+	_, err := os.Stat(baseDir)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+
+	return os.MkdirAll(baseDir, 0755)
+}

pkg/action/action.go

@@ -99,7 +99,9 @@ type Configuration struct {
 // renderResources renders the templates in a chart
 //
 // TODO: This function is badly in need of a refactor.
-func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, disableHooks bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) {
+// TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed
+//       This code has to do with writing files to dick.
+func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) {
 	hs := []*release.Hook{}
 	b := bytes.NewBuffer(nil)
 
@@ -212,24 +214,6 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values
 		}
 	}
 
-	if !disableHooks && len(hs) > 0 {
-		for _, h := range hs {
-			if outputDir == "" {
-				fmt.Fprintf(b, "---\n# Source: %s\n%s\n", h.Path, h.Manifest)
-			} else {
-				newDir := outputDir
-				if useReleaseName {
-					newDir = filepath.Join(outputDir, releaseName)
-				}
-				err = writeToFile(newDir, h.Path, h.Manifest, fileWritten[h.Path])
-				if err != nil {
-					return hs, b, "", err
-				}
-				fileWritten[h.Path] = true
-			}
-		}
-	}
-
 	if pr != nil {
 		b, err = pr.Run(b)
 		if err != nil {

pkg/action/install.go

@@ -236,7 +236,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.
 	rel := i.createRelease(chrt, vals)
 
 	var manifestDoc *bytes.Buffer
-	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.DisableHooks, i.PostRenderer, i.DryRun)
+	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun)
 	// Even for errors, attach this if available
 	if manifestDoc != nil {
 		rel.Manifest = manifestDoc.String()

pkg/action/install_test.go

@@ -499,9 +499,6 @@ func TestInstallReleaseOutputDir(t *testing.T) {
 	_, err = os.Stat(filepath.Join(dir, "hello/templates/with-partials"))
 	is.NoError(err)
 
-	_, err = os.Stat(filepath.Join(dir, "hello/templates/hooks"))
-	is.NoError(err)
-
 	_, err = os.Stat(filepath.Join(dir, "hello/templates/rbac"))
 	is.NoError(err)
 
@@ -542,9 +539,6 @@ func TestInstallOutputDirWithReleaseName(t *testing.T) {
 	_, err = os.Stat(filepath.Join(newDir, "hello/templates/with-partials"))
 	is.NoError(err)
 
-	_, err = os.Stat(filepath.Join(newDir, "hello/templates/hooks"))
-	is.NoError(err)
-
 	_, err = os.Stat(filepath.Join(newDir, "hello/templates/rbac"))
 	is.NoError(err)
 

pkg/action/upgrade.go

@@ -222,7 +222,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		return nil, nil, err
 	}
 
-	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, false, u.PostRenderer, u.DryRun)
+	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun)
 	if err != nil {
 		return nil, nil, err
 	}