Fix interactive apply asks in no change situation

This commit moves the pre-apply hooks that are now always run
since #522 outside of the check for interactivity or asking for
confirmation.

It also moves
- the early exit condition for the case when there
are no releases to Delete or Update
- the cleanup of released with no change
to before the interactivity check

This does subtly change the behaviour of preapply hooks
when apply is invoked in interactive mode. The hooks will
always be run before confirmation is asked for. In the event
there are no releases to delete or update the preapply hooks
will still run. Given that each preapply hook command must be
idempotent I don't believe this is an issue.

Fixes: #679
Signed-off-by: Thomas Arrow <thomas.arrow@wikimedia.de>

Author: tarrow

pkg/app/app.go

@@ -1418,20 +1418,31 @@ Do you really want to apply?
 	// Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies
 	st.Releases = selectedAndNeededReleases
 
-	if !interactive || interactive && r.askForConfirmation(confMsg) {
-		if _, preapplyErrors := withDAG(st, helm, a.Logger, state.PlanOptions{Purpose: "invoking preapply hooks for", Reverse: true, SelectedReleases: toApplyWithNeeds, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error {
-			for _, r := range subst.Releases {
-				release := r
-				if _, err := st.TriggerPreapplyEvent(&release, "apply"); err != nil {
-					return []error{err}
-				}
+	if _, preapplyErrors := withDAG(st, helm, a.Logger, state.PlanOptions{Purpose: "invoking preapply hooks for", Reverse: true, SelectedReleases: toApplyWithNeeds, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error {
+		for _, r := range subst.Releases {
+			release := r
+			if _, err := st.TriggerPreapplyEvent(&release, "apply"); err != nil {
+				return []error{err}
 			}
+		}
 
-			return nil
-		})); len(preapplyErrors) > 0 {
-			return true, false, preapplyErrors
+		return nil
+	})); len(preapplyErrors) > 0 {
+		return true, false, preapplyErrors
+	}
+
+	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 {
+		return true, false, nil
+	}
 
+	if !interactive || interactive && r.askForConfirmation(confMsg) {
 		// We deleted releases by traversing the DAG in reverse order
 		if len(releasesToBeDeleted) > 0 {
 			_, deletionErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: true, SelectedReleases: toDelete, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error {
@@ -1489,16 +1500,6 @@ Do you really want to apply?
 
 	affectedReleases.DisplayAffectedReleases(c.Logger())
 
-	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 {
-		return true, false, nil
-	}
-
 	return true, true, applyErrs
 }