Add support for hook deletion policies (OnSuccess, OnFailure) (resolves #374) (#412)

common/common.go

@@ -71,6 +71,8 @@ var (
 
 	// AnnotationHook contains the hook type of a resource
 	AnnotationHook = MetadataPrefix + "/hook"
+	// AnnotationHookDeletePolicy is the policy of deleting a hook
+	AnnotationHookDeletePolicy = MetadataPrefix + "/hook-delete-policy"
 
 	// LabelKeyApplicationControllerInstanceID is the label which allows to separate application among multiple running application controllers.
 	LabelKeyApplicationControllerInstanceID = application.ApplicationFullName + "/controller-instanceid"

controller/sync.go

@@ -506,7 +506,7 @@ func (sc *syncContext) runHooks(hooks []*unstructured.Unstructured, hookType app
 		return false
 	}
 	if !successful {
-		sc.setOperationPhase(appv1.OperationFailed, fmt.Sprintf("%s hook failed", appv1.HookTypePreSync))
+		sc.setOperationPhase(appv1.OperationFailed, fmt.Sprintf("%s hook failed", hookType))
 		return false
 	}
 	return true
@@ -519,7 +519,7 @@ func (sc *syncContext) runHook(hook *unstructured.Unstructured, hookType appv1.H
 	// or formulated at the time of the operation (metadata.generateName). If user specifies
 	// metadata.generateName, then we will generate a formulated metadata.name before submission.
 	if hook.GetName() == "" {
-		postfix := strings.ToLower(fmt.Sprintf("%s-%s-%d", hookType, sc.syncRes.Revision[0:7], sc.opState.StartedAt.UTC().Unix()))
+		postfix := strings.ToLower(fmt.Sprintf("%s-%s-%d", sc.syncRes.Revision[0:7], hookType, sc.opState.StartedAt.UTC().Unix()))
 		generatedName := hook.GetGenerateName()
 		hook = hook.DeepCopy()
 		hook.SetName(fmt.Sprintf("%s%s", generatedName, postfix))
@@ -568,9 +568,37 @@ func (sc *syncContext) runHook(hook *unstructured.Unstructured, hookType appv1.H
 		liveObj = existing
 	}
 	hookStatus := newHookStatus(liveObj, hookType)
+	if hookStatus.Status.Completed() {
+		if enforceDeletePolicy(hook, hookStatus.Status) {
+			err = sc.deleteHook(hook.GetName(), hook.GetKind(), hook.GetAPIVersion())
+			if err != nil {
+				hookStatus.Status = appv1.OperationFailed
+				hookStatus.Message = fmt.Sprintf("failed to delete %s hook: %v", hookStatus.Status, err)
+			}
+		}
+	}
 	return sc.updateHookStatus(hookStatus), nil
 }
 
+// enforceDeletePolicy examines the hook deletion policy of a object and deletes it based on the status
+func enforceDeletePolicy(hook *unstructured.Unstructured, phase appv1.OperationPhase) bool {
+	annotations := hook.GetAnnotations()
+	if annotations == nil {
+		return false
+	}
+	deletePolicies := strings.Split(annotations[common.AnnotationHookDeletePolicy], ",")
+	for _, dp := range deletePolicies {
+		policy := appv1.HookDeletePolicy(strings.TrimSpace(dp))
+		if policy == appv1.HookDeletePolicyHookSucceeded && phase == appv1.OperationSucceeded {
+			return true
+		}
+		if policy == appv1.HookDeletePolicyHookFailed && phase == appv1.OperationFailed {
+			return true
+		}
+	}
+	return false
+}
+
 // isHookType tells whether or not the supplied object is a hook of the specified type
 func isHookType(hook *unstructured.Unstructured, hookType appv1.HookType) bool {
 	annotations := hook.GetAnnotations()
@@ -678,6 +706,7 @@ func newHookStatus(hook *unstructured.Unstructured, hookType appv1.HookType) app
 				switch condition.Type {
 				case batch.JobFailed:
 					failed = true
+					complete = true
 					failMsg = condition.Message
 				case batch.JobComplete:
 					complete = true
@@ -721,7 +750,7 @@ func newHookStatus(hook *unstructured.Unstructured, hookType appv1.HookType) app
 	return hookStatus
 }
 
-// updateHookStatus updates the status of a hook. Returns whether or not the hook was changed or not
+// updateHookStatus updates the status of a hook. Returns true if the hook was modified
 func (sc *syncContext) updateHookStatus(hookStatus appv1.HookStatus) bool {
 	sc.lock.Lock()
 	defer sc.lock.Unlock()
@@ -783,9 +812,9 @@ func (sc *syncContext) terminate() {
 		}
 	}
 	if terminateSuccessful {
-		sc.setOperationPhase(appv1.OperationFailed, "Application terminated")
+		sc.setOperationPhase(appv1.OperationFailed, "Operation terminated")
 	} else {
-		sc.setOperationPhase(appv1.OperationError, "Termination had errors")
+		sc.setOperationPhase(appv1.OperationError, "Operation termination had errors")
 	}
 }
 

pkg/apis/application/v1alpha1/types.go

@@ -114,6 +114,13 @@ const (
 	//HookTypeSyncFail     HookType = "SyncFail"
 )
 
+type HookDeletePolicy string
+
+const (
+	HookDeletePolicyHookSucceeded HookDeletePolicy = "HookSucceeded"
+	HookDeletePolicyHookFailed    HookDeletePolicy = "HookFailed"
+)
+
 // HookStatus contains status about a hook invocation
 type HookStatus struct {
 	// Name is the resource name

reposerver/repository/repository.go

@@ -160,7 +160,7 @@ func (s *Service) GenerateManifest(c context.Context, q *ManifestRequest) (*Mani
 
 	params, err := ksApp.ListEnvParams(q.Environment)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Failed to list ksonnet app params: %v", err)
 	}
 
 	if q.ComponentParameterOverrides != nil {

util/ksonnet/ksonnet.go

@@ -9,9 +9,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
-	"github.com/argoproj/argo-cd/util/cli"
-	"github.com/argoproj/argo-cd/util/diff"
 	"github.com/ghodss/yaml"
 	"github.com/ksonnet/ksonnet/pkg/app"
 	"github.com/ksonnet/ksonnet/pkg/component"
@@ -22,6 +19,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	v1ExtBeta1 "k8s.io/api/extensions/v1beta1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	"github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
+	"github.com/argoproj/argo-cd/util/cli"
+	"github.com/argoproj/argo-cd/util/diff"
 )
 
 var (
@@ -127,7 +128,7 @@ func (k *ksonnetApp) Spec() *app.Spec {
 func (k *ksonnetApp) Show(environment string) ([]*unstructured.Unstructured, error) {
 	out, err := k.ksCmd("show", environment)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("`ks show` failed: %v", err)
 	}
 	parts := diffSeparator.Split(out, -1)
 	objs := make([]*unstructured.Unstructured, 0)
@@ -150,8 +151,9 @@ func (k *ksonnetApp) Show(environment string) ([]*unstructured.Unstructured, err
 	return objs, nil
 }
 
-// remarshal checks resource kind and version and re-marshal using corresponding struct custom marshaller. This ensures that expected resource state is formatter same as actual
-// resource state in kubernetes and allows to find differences between actual and target states more accurate.
+// remarshal checks resource kind and version and re-marshal using corresponding struct custom marshaller.
+// This ensures that expected resource state is formatter same as actualresource state in kubernetes
+// and allows to find differences between actual and target states more accurately.
 func remarshal(obj *unstructured.Unstructured) error {
 	var newObj interface{}
 	switch obj.GetAPIVersion() + ":" + obj.GetKind() {