Fix race on/sometimes missing postsync and cleanup hooks (#1407)

So that those hooks are fully executed as intended. The documentation about hooks are updated as well to clarify the intended behaviour.

Fixes #1398

README.md

@@ -963,19 +963,21 @@ Currently supported `events` are:
 - `cleanup`
 
 Hooks associated to `prepare` events are triggered after each release in your helmfile is loaded from YAML, before execution.
-
-Hooks associated to `cleanup` events are triggered after each release is processed.
+`prepare` hooks are triggered on the release as long as it is not excluded by the helmfile selector(e.g. `helmfile -l key=value`).
 
 Hooks associated to `presync` events are triggered before each release is applied to the remote cluster.
 This is the ideal event to execute any commands that may mutate the cluster state as it will not be run for read-only operations like `lint`, `diff` or `template`.
 
 `preuninstall` hooks are triggered immediately before a release is uninstalled as part of `helmfile apply`, `helmfile sync`, `helmfile delete`, and `helmfile destroy`.
 
-Hooks associated to `postsync` events are triggered after each release is applied to the remote cluster.
-This is the ideal event to execute any commands that may mutate the cluster state as it will not be run for read-only operations like `lint`, `diff` or `template`.
-
 `postuninstall` hooks are triggered immediately after successful uninstall of a release while running `helmfile apply`, `helmfile sync`, `helmfile delete`, `helmfile destroy`.
 
+`postsync` hooks are triggered after each release is synced(installed, updated, or uninstalled) to/from the cluster, regardless of the sync was successful or not.
+This is the ideal place to execute any commands that may mutate the cluster state as it will not be run for read-only operations like `lint`, `diff` or `template`.
+
+`cleanup` hooks are triggered after each release is processed.
+This is the counterpart to `prepare`, as any release on which `prepare` has been triggered gets `cleanup` triggered as well.
+
 The following is an example hook that just prints the contextual information provided to hook:
 
 ```

pkg/app/app.go

@@ -987,6 +987,23 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) {
 		return false, false, errs
 	}
 
+	releasesWithNoChange := map[string]state.ReleaseSpec{}
+	for _, r := range toApply {
+		id := state.ReleaseToID(&r)
+		_, uninstalled := releasesToBeUpdated[id]
+		_, updated := releasesToBeDeleted[id]
+		if !uninstalled && !updated {
+			releasesWithNoChange[id] = r
+		}
+	}
+
+	for id := range releasesWithNoChange {
+		r := releasesWithNoChange[id]
+		if _, err := st.TriggerCleanupEvent(&r, "apply"); err != nil {
+			a.Logger.Warnf("warn: %v\n", err)
+		}
+	}
+
 	if releasesToBeDeleted == nil && releasesToBeUpdated == nil {
 		if infoMsg != nil {
 			logger := c.Logger()
@@ -1091,6 +1108,22 @@ func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error
 		releasesToDelete[id] = r
 	}
 
+	releasesWithNoChange := map[string]state.ReleaseSpec{}
+	for _, r := range toSync {
+		id := state.ReleaseToID(&r)
+		_, uninstalled := releasesToDelete[id]
+		if !uninstalled {
+			releasesWithNoChange[id] = r
+		}
+	}
+
+	for id := range releasesWithNoChange {
+		r := releasesWithNoChange[id]
+		if _, err := st.TriggerCleanupEvent(&r, "delete"); err != nil {
+			a.Logger.Warnf("warn: %v\n", err)
+		}
+	}
+
 	names := make([]string, len(toSync))
 	for i, r := range toSync {
 		names[i] = fmt.Sprintf("  %s (%s)", r.Name, r.Chart)
@@ -1175,6 +1208,23 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) {
 		releasesToUpdate[id] = r
 	}
 
+	releasesWithNoChange := map[string]state.ReleaseSpec{}
+	for _, r := range toSync {
+		id := state.ReleaseToID(&r)
+		_, uninstalled := releasesToDelete[id]
+		_, updated := releasesToUpdate[id]
+		if !uninstalled && !updated {
+			releasesWithNoChange[id] = r
+		}
+	}
+
+	for id := range releasesWithNoChange {
+		r := releasesWithNoChange[id]
+		if _, err := st.TriggerCleanupEvent(&r, "sync"); err != nil {
+			a.Logger.Warnf("warn: %v\n", err)
+		}
+	}
+
 	names := []string{}
 	for _, r := range releasesToUpdate {
 		names = append(names, fmt.Sprintf("  %s (%s) UPDATED", r.Name, r.Chart))

pkg/state/state.go

@@ -596,19 +596,19 @@ func (st *HelmState) DeleteReleasesForSync(affectedReleases *AffectedReleases, h
 					m.Unlock()
 				}
 
-				if relErr == nil {
-					results <- syncResult{}
-				} else {
-					results <- syncResult{errors: []*ReleaseError{relErr}}
-				}
-
 				if _, err := st.triggerPostsyncEvent(release, relErr, "sync"); err != nil {
 					st.logger.Warnf("warn: %v\n", err)
 				}
 
-				if _, err := st.triggerCleanupEvent(release, "sync"); err != nil {
+				if _, err := st.TriggerCleanupEvent(release, "sync"); err != nil {
 					st.logger.Warnf("warn: %v\n", err)
 				}
+
+				if relErr == nil {
+					results <- syncResult{}
+				} else {
+					results <- syncResult{errors: []*ReleaseError{relErr}}
+				}
 			}
 		},
 		func() {
@@ -722,19 +722,19 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme
 					}
 				}
 
-				if relErr == nil {
-					results <- syncResult{}
-				} else {
-					results <- syncResult{errors: []*ReleaseError{relErr}}
-				}
-
 				if _, err := st.triggerPostsyncEvent(release, relErr, "sync"); err != nil {
 					st.logger.Warnf("warn: %v\n", err)
 				}
 
-				if _, err := st.triggerCleanupEvent(release, "sync"); err != nil {
+				if _, err := st.TriggerCleanupEvent(release, "sync"); err != nil {
 					st.logger.Warnf("warn: %v\n", err)
 				}
+
+				if relErr == nil {
+					results <- syncResult{}
+				} else {
+					results <- syncResult{errors: []*ReleaseError{relErr}}
+				}
 			}
 		},
 		func() {
@@ -1054,7 +1054,7 @@ func (st *HelmState) TemplateReleases(helm helmexec.Interface, outputDir string,
 			}
 		}
 
-		if _, err := st.triggerCleanupEvent(release, "template"); err != nil {
+		if _, err := st.TriggerCleanupEvent(release, "template"); err != nil {
 			st.logger.Warnf("warn: %v\n", err)
 		}
 	}
@@ -1130,7 +1130,7 @@ func (st *HelmState) LintReleases(helm helmexec.Interface, additionalValues []st
 			}
 		}
 
-		if _, err := st.triggerCleanupEvent(&release, "lint"); err != nil {
+		if _, err := st.TriggerCleanupEvent(&release, "lint"); err != nil {
 			st.logger.Warnf("warn: %v\n", err)
 		}
 	}
@@ -1359,7 +1359,7 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st
 				}
 
 				if triggerCleanupEvents {
-					if _, err := st.triggerCleanupEvent(prep.release, "diff"); err != nil {
+					if _, err := st.TriggerCleanupEvent(prep.release, "diff"); err != nil {
 						st.logger.Warnf("warn: %v\n", err)
 					}
 				}
@@ -1597,7 +1597,7 @@ func (st *HelmState) triggerPrepareEvent(r *ReleaseSpec, helmfileCommand string)
 	return st.triggerReleaseEvent("prepare", nil, r, helmfileCommand)
 }
 
-func (st *HelmState) triggerCleanupEvent(r *ReleaseSpec, helmfileCommand string) (bool, error) {
+func (st *HelmState) TriggerCleanupEvent(r *ReleaseSpec, helmfileCommand string) (bool, error) {
 	return st.triggerReleaseEvent("cleanup", nil, r, helmfileCommand)
 }