Checking YAML merge error and show it to user as status. (#365)

* controller context changed

* apply controllercontext changes

* cancel if configmap have a parsing error and left a status

* cancel if secret have a parsing error and left a status

* adding more unittest

* apply status changes

* cancel configmap resource when merging configmap failed

* cancel the secret resource when merging failed

* cancel the chart resource

* comparing reason in status resource

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/giantswarm/e2etemplates v0.2.0
 	github.com/giantswarm/errors v0.2.3
 	github.com/giantswarm/exporterkit v0.2.0
-	github.com/giantswarm/helmclient v0.2.2
+	github.com/giantswarm/helmclient v0.2.3
 	github.com/giantswarm/k8sclient v0.2.0
 	github.com/giantswarm/kubeconfig v0.2.0
 	github.com/giantswarm/microendpoint v0.2.0

go.sum

@@ -229,10 +229,9 @@ github.com/giantswarm/errors v0.2.3 h1:gE1dDSD0RNwYUah5hG6oUQh0unPyZ74tVwR1/PW3Z
 github.com/giantswarm/errors v0.2.3/go.mod h1:Oj1LIWs9Uoj6JGtK5HxTb50iZuqe9f60LDI0nLXA5vU=
 github.com/giantswarm/exporterkit v0.2.0 h1:+HTGl4fmT/1OkTox8HbV3JXeavmsZv94+2CDW8cSrq0=
 github.com/giantswarm/exporterkit v0.2.0/go.mod h1:b4fuHVjsvZgznC9OEPfn0y5zvN8SnEKTiI5zTTfck94=
-github.com/giantswarm/helmclient v0.2.0 h1:etBQoz0QN9quIA/blbkIkrfr0CuzwjxA89/1MgE5YoU=
 github.com/giantswarm/helmclient v0.2.0/go.mod h1:KRqUJrPL3QO/iISztTp+Ds+PJYBU8iGWNe9ZlJCdEAs=
-github.com/giantswarm/helmclient v0.2.2 h1:4/nSCIxEAYLdbNw8xLPeHw7SNWFY6ic5KK6URQVrhik=
-github.com/giantswarm/helmclient v0.2.2/go.mod h1:jhrck2Juio1/LfjGitrOmvysu7RdhKWNqCthds52NXo=
+github.com/giantswarm/helmclient v0.2.3 h1:nwp76CDc+snWZpZ9FUoCm9z0sTAvO2nEkmMls6YQZdU=
+github.com/giantswarm/helmclient v0.2.3/go.mod h1:jhrck2Juio1/LfjGitrOmvysu7RdhKWNqCthds52NXo=
 github.com/giantswarm/k8sclient v0.2.0 h1:Az+Knfs2UJrNFoqfOsfkudg+rt3fn5EM1aKrnLSg3fU=
 github.com/giantswarm/k8sclient v0.2.0/go.mod h1:tUgUynwKgMnjnN2PldGmxJW6+8eZn2p7tT+tEhXDwOg=
 github.com/giantswarm/k8sportforward v0.2.0 h1:nzyKUtSWEbayvF74u3Pkr8AKKa3rk6LccxuIONDq4T8=

service/controller/app/controllercontext/context.go

@@ -27,10 +27,16 @@ type Clients struct {
 }
 
 type Status struct {
-	TenantCluster TenantCluster
+	ChartStatus   ChartStatus
+	ClusterStatus ClusterStatus
 }
 
-type TenantCluster struct {
+type ChartStatus struct {
+	Reason string
+	Status string
+}
+
+type ClusterStatus struct {
 	IsDeleting    bool
 	IsUnavailable bool
 }

service/controller/app/resource/appnamespace/resource.go

@@ -68,9 +68,9 @@ func (r *Resource) addNamespaceStatusToContext(ctx context.Context, cr v1alpha1.
 	}
 
 	if ns.GetDeletionTimestamp() != nil {
-		cc.Status.TenantCluster.IsDeleting = true
+		cc.Status.ClusterStatus.IsDeleting = true
 	} else {
-		cc.Status.TenantCluster.IsDeleting = false
+		cc.Status.ClusterStatus.IsDeleting = false
 	}
 
 	return nil

service/controller/app/resource/chart/current.go

@@ -27,14 +27,21 @@ func (r *Resource) GetCurrentState(ctx context.Context, obj interface{}) (interf
 		return nil, microerror.Mask(err)
 	}
 
-	if cc.Status.TenantCluster.IsDeleting {
+	if cc.Status.ChartStatus.Status != "" {
+		r.logger.LogCtx(ctx, "level", "debug", "message", fmt.Sprintf("chart %#q failed to merge configMaps/secrets, no need to reconcile resource", cr.Name))
+		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
+		resourcecanceledcontext.SetCanceled(ctx)
+		return nil, nil
+	}
+
+	if cc.Status.ClusterStatus.IsDeleting {
 		r.logger.LogCtx(ctx, "level", "debug", "message", fmt.Sprintf("namespace %#q is being deleted, no need to reconcile resource", cr.Namespace))
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		resourcecanceledcontext.SetCanceled(ctx)
 		return nil, nil
 	}
 
-	if cc.Status.TenantCluster.IsUnavailable {
+	if cc.Status.ClusterStatus.IsUnavailable {
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster is unavailable")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		resourcecanceledcontext.SetCanceled(ctx)

service/controller/app/resource/chartoperator/create.go

@@ -31,7 +31,7 @@ func (r Resource) EnsureCreated(ctx context.Context, obj interface{}) error {
 		return nil
 	}
 
-	if cc.Status.TenantCluster.IsUnavailable {
+	if cc.Status.ClusterStatus.IsUnavailable {
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster is unavailable")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		return nil

service/controller/app/resource/clients/resource.go

@@ -79,7 +79,7 @@ func (r *Resource) addClientsToContext(ctx context.Context, cr v1alpha1.App) err
 		return microerror.Mask(err)
 	}
 
-	if cc.Status.TenantCluster.IsDeleting {
+	if cc.Status.ClusterStatus.IsDeleting {
 		return nil
 	}
 
@@ -100,7 +100,7 @@ func (r *Resource) addClientsToContext(ctx context.Context, cr v1alpha1.App) err
 	if kubeconfig.IsNotFoundError(err) {
 		// Set status so we don't try to connect to the tenant cluster
 		// again in this reconciliation loop.
-		cc.Status.TenantCluster.IsUnavailable = true
+		cc.Status.ClusterStatus.IsUnavailable = true
 
 		r.logger.LogCtx(ctx, "level", "debug", "message", "kubeconfig secret not found")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")

service/controller/app/resource/configmap/current.go

@@ -29,14 +29,14 @@ func (r *Resource) GetCurrentState(ctx context.Context, obj interface{}) (interf
 		return nil, microerror.Mask(err)
 	}
 
-	if cc.Status.TenantCluster.IsDeleting {
+	if cc.Status.ClusterStatus.IsDeleting {
 		r.logger.LogCtx(ctx, "level", "debug", "message", fmt.Sprintf("namespace %#q is being deleted, no need to reconcile resource", cr.Namespace))
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		resourcecanceledcontext.SetCanceled(ctx)
 		return nil, nil
 	}
 
-	if cc.Status.TenantCluster.IsUnavailable {
+	if cc.Status.ClusterStatus.IsUnavailable {
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster is unavailable")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		resourcecanceledcontext.SetCanceled(ctx)
@@ -67,7 +67,7 @@ func (r *Resource) GetCurrentState(ctx context.Context, obj interface{}) (interf
 	case <-time.After(3 * time.Second):
 		// Set status so we don't try to connect to the tenant cluster
 		// again in this reconciliation loop.
-		cc.Status.TenantCluster.IsUnavailable = true
+		cc.Status.ClusterStatus.IsUnavailable = true
 
 		r.logger.LogCtx(ctx, "level", "debug", "message", "timeout getting configmap")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
@@ -82,7 +82,7 @@ func (r *Resource) GetCurrentState(ctx context.Context, obj interface{}) (interf
 	} else if tenant.IsAPINotAvailable(err) {
 		// Set status so we don't try to connect to the tenant cluster
 		// again in this reconciliation loop.
-		cc.Status.TenantCluster.IsUnavailable = true
+		cc.Status.ClusterStatus.IsUnavailable = true
 
 		// We should not hammer tenant API if it is not available. We cancel
 		// the reconciliation because its likely following resources will also

service/controller/app/resource/configmap/desired.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/giantswarm/microerror"
+	"github.com/giantswarm/operatorkit/controller/context/resourcecanceledcontext"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -40,7 +41,18 @@ func (r *Resource) GetDesiredState(ctx context.Context, obj interface{}) (interf
 
 	mergedData, err := r.values.MergeConfigMapData(ctx, cr, cc.AppCatalog)
 	if values.IsNotFound(err) {
-		r.logger.LogCtx(ctx, "level", "warning", "message", fmt.Sprintf("dependent configMaps are not found"), "stack", fmt.Sprintf("%#v", err))
+		r.logger.LogCtx(ctx, "level", "warning", "message", "dependent configMaps are not found")
+		addStatusToContext(cc, err.Error(), configmapMergeFailedStatus)
+
+		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
+		resourcecanceledcontext.SetCanceled(ctx)
+		return nil, nil
+	} else if values.IsParsingError(err) {
+		r.logger.LogCtx(ctx, "level", "warning", "message", "failed to merging configMaps")
+		addStatusToContext(cc, err.Error(), configmapMergeFailedStatus)
+
+		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
+		resourcecanceledcontext.SetCanceled(ctx)
 		return nil, nil
 	} else if err != nil {
 		return nil, microerror.Mask(err)

service/controller/app/resource/configmap/resource.go

@@ -7,12 +7,17 @@ import (
 	"github.com/giantswarm/micrologger"
 	corev1 "k8s.io/api/core/v1"
 
+	"github.com/giantswarm/app-operator/service/controller/app/controllercontext"
 	"github.com/giantswarm/app-operator/service/controller/app/values"
 )
 
 const (
 	// Name is the identifier of the resource.
 	Name = "configmap"
+
+	// configmapMergeFailedStatus is set in the CR status when there is an failure during
+	// merge configmaps.
+	configmapMergeFailedStatus = "configmap-merge-failed"
 )
 
 // Config represents the configuration used to create a new configmap resource.
@@ -62,6 +67,17 @@ func (r *Resource) Name() string {
 	return Name
 }
 
+// addStatusToContext adds the status to the controller context. It will be
+// used to set the CR status in the status resource.
+func addStatusToContext(cc *controllercontext.Context, reason, status string) {
+	cc.Status = controllercontext.Status{
+		ChartStatus: controllercontext.ChartStatus{
+			Reason: reason,
+			Status: status,
+		},
+	}
+}
+
 // equals asseses the equality of ConfigMaps with regards to distinguishing
 // fields.
 func equals(a, b *corev1.ConfigMap) bool {

service/controller/app/resource/secret/current.go

@@ -27,14 +27,14 @@ func (r *Resource) GetCurrentState(ctx context.Context, obj interface{}) (interf
 		return nil, microerror.Mask(err)
 	}
 
-	if cc.Status.TenantCluster.IsDeleting {
+	if cc.Status.ClusterStatus.IsDeleting {
 		r.logger.LogCtx(ctx, "level", "debug", "message", fmt.Sprintf("namespace %#q is being deleted, no need to reconcile resource", cr.Namespace))
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		resourcecanceledcontext.SetCanceled(ctx)
 		return nil, nil
 	}
 
-	if cc.Status.TenantCluster.IsUnavailable {
+	if cc.Status.ClusterStatus.IsUnavailable {
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster is unavailable")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		resourcecanceledcontext.SetCanceled(ctx)

service/controller/app/resource/secret/desired.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/giantswarm/microerror"
+	"github.com/giantswarm/operatorkit/controller/context/resourcecanceledcontext"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -40,7 +41,18 @@ func (r *Resource) GetDesiredState(ctx context.Context, obj interface{}) (interf
 
 	mergedData, err := r.values.MergeSecretData(ctx, cr, cc.AppCatalog)
 	if values.IsNotFound(err) {
-		r.logger.LogCtx(ctx, "level", "warning", "message", fmt.Sprintf("dependent secrets are not found"), "stack", fmt.Sprintf("%#v", err))
+		r.logger.LogCtx(ctx, "level", "warning", "message", "dependent secrets are not found")
+		addStatusToContext(cc, err.Error(), secretMergeFailedStatus)
+
+		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
+		resourcecanceledcontext.SetCanceled(ctx)
+		return nil, nil
+	} else if values.IsParsingError(err) {
+		r.logger.LogCtx(ctx, "level", "warning", "message", "failed to merging secrets")
+		addStatusToContext(cc, err.Error(), secretMergeFailedStatus)
+
+		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
+		resourcecanceledcontext.SetCanceled(ctx)
 		return nil, nil
 	} else if err != nil {
 		return nil, microerror.Mask(err)

service/controller/app/resource/secret/resource.go

@@ -7,12 +7,17 @@ import (
 	"github.com/giantswarm/micrologger"
 	corev1 "k8s.io/api/core/v1"
 
+	"github.com/giantswarm/app-operator/service/controller/app/controllercontext"
 	"github.com/giantswarm/app-operator/service/controller/app/values"
 )
 
 const (
 	// Name is the identifier of the resource.
 	Name = "secret"
+
+	// secretMergeFailedStatus is set in the CR status when there is an failure during
+	// merge secrets.
+	secretMergeFailedStatus = "secret-merge-failed"
 )
 
 // Config represents the configuration used to create a new secret resource.
@@ -62,6 +67,17 @@ func (r *Resource) Name() string {
 	return Name
 }
 
+// addStatusToContext adds the status to the controller context. It will be
+// used to set the CR status in the status resource.
+func addStatusToContext(cc *controllercontext.Context, reason, status string) {
+	cc.Status = controllercontext.Status{
+		ChartStatus: controllercontext.ChartStatus{
+			Reason: reason,
+			Status: status,
+		},
+	}
+}
+
 // equals asseses the equality of Secrets with regards to distinguishing
 // fields.
 func equals(a, b *corev1.Secret) bool {

service/controller/app/resource/status/create.go

@@ -25,13 +25,13 @@ func (r *Resource) EnsureCreated(ctx context.Context, obj interface{}) error {
 		return microerror.Mask(err)
 	}
 
-	if cc.Status.TenantCluster.IsDeleting {
+	if cc.Status.ClusterStatus.IsDeleting {
 		r.logger.LogCtx(ctx, "level", "debug", "message", fmt.Sprintf("namespace %#q is being deleted, no need to reconcile resource", cr.Namespace))
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		return nil
 	}
 
-	if cc.Status.TenantCluster.IsUnavailable {
+	if cc.Status.ClusterStatus.IsUnavailable {
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster is unavailable")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		return nil
@@ -57,14 +57,24 @@ func (r *Resource) EnsureCreated(ctx context.Context, obj interface{}) error {
 	r.logger.LogCtx(ctx, "level", "debug", "message", fmt.Sprintf("found status for chart %#q in namespace %#q", cr.Name, r.chartNamespace))
 
 	chartStatus := key.ChartStatus(*chart)
-	desiredStatus := v1alpha1.AppStatus{
-		AppVersion: chartStatus.AppVersion,
-		Release: v1alpha1.AppStatusRelease{
-			LastDeployed: chartStatus.Release.LastDeployed,
-			Reason:       chartStatus.Reason,
-			Status:       chartStatus.Release.Status,
-		},
-		Version: chartStatus.Version,
+	var desiredStatus v1alpha1.AppStatus
+	if cc.Status.ChartStatus.Status != "" {
+		desiredStatus = v1alpha1.AppStatus{
+			Release: v1alpha1.AppStatusRelease{
+				Reason: cc.Status.ChartStatus.Reason,
+				Status: cc.Status.ChartStatus.Status,
+			},
+		}
+	} else {
+		desiredStatus = v1alpha1.AppStatus{
+			AppVersion: chartStatus.AppVersion,
+			Release: v1alpha1.AppStatusRelease{
+				LastDeployed: chartStatus.Release.LastDeployed,
+				Reason:       chartStatus.Reason,
+				Status:       chartStatus.Release.Status,
+			},
+			Version: chartStatus.Version,
+		}
 	}
 
 	if !equals(desiredStatus, key.AppStatus(cr)) {

service/controller/app/resource/status/resource.go

@@ -59,6 +59,9 @@ func equals(a, b v1alpha1.AppStatus) bool {
 	if a.Release.LastDeployed != b.Release.LastDeployed {
 		return false
 	}
+	if a.Release.Reason != b.Release.Reason {
+		return false
+	}
 	if a.Release.Status != b.Release.Status {
 		return false
 	}

service/controller/app/resource/tcnamespace/create.go

@@ -41,7 +41,7 @@ func (r *Resource) EnsureCreated(ctx context.Context, obj interface{}) error {
 		return nil
 	}
 
-	if cc.Status.TenantCluster.IsUnavailable {
+	if cc.Status.ClusterStatus.IsUnavailable {
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster is unavailable")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		return nil
@@ -73,7 +73,7 @@ func (r *Resource) EnsureCreated(ctx context.Context, obj interface{}) error {
 	case <-time.After(3 * time.Second):
 		// Set status so we don't try to connect to the tenant cluster
 		// again in this reconciliation loop.
-		cc.Status.TenantCluster.IsUnavailable = true
+		cc.Status.ClusterStatus.IsUnavailable = true
 
 		r.logger.LogCtx(ctx, "level", "debug", "message", "timeout creating namespace")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
@@ -85,7 +85,7 @@ func (r *Resource) EnsureCreated(ctx context.Context, obj interface{}) error {
 	} else if tenant.IsAPINotAvailable(err) {
 		// Set status so we don't try to connect to the tenant cluster
 		// again in this reconciliation loop.
-		cc.Status.TenantCluster.IsUnavailable = true
+		cc.Status.ClusterStatus.IsUnavailable = true
 
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster not available")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")

service/controller/app/resource/tiller/create.go

@@ -34,7 +34,7 @@ func (r *Resource) EnsureCreated(ctx context.Context, obj interface{}) error {
 		return nil
 	}
 
-	if cc.Status.TenantCluster.IsUnavailable {
+	if cc.Status.ClusterStatus.IsUnavailable {
 		r.logger.LogCtx(ctx, "level", "debug", "message", "tenant cluster is unavailable")
 		r.logger.LogCtx(ctx, "level", "debug", "message", "canceling resource")
 		return nil