From 6a36f34c7db20e95fdac16bd835c4751012ceb16 Mon Sep 17 00:00:00 2001
From: yxxhero <11087727+yxxhero@users.noreply.github.com>
Date: Sun, 9 Oct 2022 16:51:52 +0800
Subject: [PATCH] fix: child process not exit when recive term signal (#418)

* fix: child process not exit when recive term signal

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: wait for clean done

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: Yusuke Kuoka <ykuoka@gmail.com>
---
 main.go        | 24 ++++++++++++++++++++++++
 pkg/app/app.go | 51 +++++++++++++++++++++++++++++---------------------
 2 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/main.go b/main.go
index 4cfae4da..7f51f67c 100644
--- a/main.go
+++ b/main.go
@@ -1,19 +1,43 @@
 package main
 
 import (
+	"fmt"
 	"os"
+	"os/signal"
+	"syscall"
 
 	"github.com/helmfile/helmfile/cmd"
+	"github.com/helmfile/helmfile/pkg/app"
 	"github.com/helmfile/helmfile/pkg/config"
 	"github.com/helmfile/helmfile/pkg/errors"
 )
 
 func main() {
+	var sig os.Signal
+	sigs := make(chan os.Signal, 1)
+	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
+
+	go func() {
+		sig = <-sigs
+	}()
+
 	globalConfig := new(config.GlobalOptions)
 	rootCmd, err := cmd.NewRootCmd(globalConfig, os.Args[1:])
 	errors.HandleExitCoder(err)
 
 	if err := rootCmd.Execute(); err != nil {
+		if sig != nil {
+			fmt.Fprintln(os.Stderr, err)
+			app.CleanWaitGroup.Wait()
+
+			// See http://tldp.org/LDP/abs/html/exitcodes.html
+			switch sig {
+			case syscall.SIGINT:
+				os.Exit(130)
+			case syscall.SIGTERM:
+				os.Exit(143)
+			}
+		}
 		errors.HandleExitCoder(err)
 	}
 }
diff --git a/pkg/app/app.go b/pkg/app/app.go
index 2ef6f1ee..96fde0e5 100644
--- a/pkg/app/app.go
+++ b/pkg/app/app.go
@@ -5,12 +5,10 @@ import (
 	"fmt"
 	"log"
 	"os"
-	"os/signal"
 	"path/filepath"
 	"sort"
 	"strings"
 	"sync"
-	"syscall"
 	"text/tabwriter"
 
 	"github.com/variantdev/vals"
@@ -24,6 +22,8 @@ import (
 	"github.com/helmfile/helmfile/pkg/state"
 )
 
+var CleanWaitGroup sync.WaitGroup
+
 // App is the main application object.
 type App struct {
 	OverrideKubeContext string
@@ -776,7 +776,7 @@ func (a *App) getHelm(st *state.HelmState) helmexec.Interface {
 func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*state.HelmState) (bool, []error)) error {
 	noMatchInHelmfiles := true
 
-	err := a.visitStateFiles(fileOrDir, defOpts, func(f, d string) error {
+	err := a.visitStateFiles(fileOrDir, defOpts, func(f, d string) (retErr error) {
 		opts := defOpts.DeepCopy()
 
 		if opts.CalleePath == "" {
@@ -785,22 +785,6 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta
 
 		st, err := a.loadDesiredStateFromYaml(f, opts)
 
-		sigs := make(chan os.Signal, 1)
-		signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
-		go func() {
-			sig := <-sigs
-
-			errs := []error{fmt.Errorf("received [%s] to shutdown ", sig)}
-			_ = context{app: a, st: st, retainValues: defOpts.RetainValuesFiles}.clean(errs)
-			// See http://tldp.org/LDP/abs/html/exitcodes.html
-			switch sig {
-			case syscall.SIGINT:
-				os.Exit(130)
-			case syscall.SIGTERM:
-				os.Exit(143)
-			}
-		}()
-
 		ctx := context{app: a, st: st, retainValues: defOpts.RetainValuesFiles}
 
 		if err != nil {
@@ -864,7 +848,32 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta
 			return appError(fmt.Sprintf("failed executing release templates in \"%s\"", f), tmplErr)
 		}
 
-		processed, errs := converge(templated)
+		var (
+			processed bool
+			errs      []error
+		)
+
+		// Ensure every temporary files and directories generated while running
+		// the converge function is clean up before exiting this function in all the three cases below:
+		// - This function returned nil
+		// - This function returned an err
+		// - Helmfile received SIGINT or SIGTERM while running this function
+		// For the last case you also need a signal handler in main.go.
+		// Ideally though, this CleanWaitGroup should gone and be replaced by a context cancellation propagation.
+		// See https://github.com/helmfile/helmfile/pull/418 for more details.
+		CleanWaitGroup.Add(1)
+		defer func() {
+			defer CleanWaitGroup.Done()
+			cleanErr := context{app: a, st: templated, retainValues: defOpts.RetainValuesFiles}.clean(errs)
+			if retErr == nil {
+				retErr = cleanErr
+			} else if cleanErr != nil {
+				a.Logger.Debugf("Failed to clean up temporary files generated while processing %q: %v", templated.FilePath, cleanErr)
+			}
+		}()
+
+		processed, errs = converge(templated)
+
 		noMatchInHelmfiles = noMatchInHelmfiles && !processed
 
 		if opts.Reverse {
@@ -874,7 +883,7 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta
 			}
 		}
 
-		return context{app: a, st: templated, retainValues: defOpts.RetainValuesFiles}.clean(errs)
+		return nil
 	})
 
 	if err != nil {