controller: add reconcile release tests

Plus some minor improvements to the logic, based on writing tests.

Signed-off-by: Hidde Beydals <[email protected]>

go.mod

@@ -22,6 +22,7 @@ require (
 	github.com/fluxcd/pkg/apis/meta v1.1.1
 	github.com/fluxcd/pkg/runtime v0.40.0
 	github.com/fluxcd/pkg/ssa v0.28.2
+	github.com/fluxcd/pkg/testserver v0.4.0
 	github.com/fluxcd/source-controller/api v1.0.0
 	github.com/go-logr/logr v1.2.4
 	github.com/google/go-cmp v0.5.9

go.sum

@@ -177,6 +177,8 @@ github.com/fluxcd/pkg/runtime v0.40.0 h1:uGiiEbMZwd7xmbKaVmcH7iilCFW9betWbz0r1ta
 github.com/fluxcd/pkg/runtime v0.40.0/go.mod h1:BqHEOVrZmt19p0q1OlGFWAYh3rZ28+IBpxLB2yPjjQ4=
 github.com/fluxcd/pkg/ssa v0.28.2 h1:BL42JK9Cg17PHh/VIh64e6GZ+rjO47uDMGXGSGeCdao=
 github.com/fluxcd/pkg/ssa v0.28.2/go.mod h1:nL0bCiFTAMvMrEKGlVCAgtLdP8VIPBGizY5xgasaypI=
+github.com/fluxcd/pkg/testserver v0.4.0 h1:pDZ3gistqYhwlf3sAjn1Q8NzN4Qe6I1BEmHMHi46lMg=
+github.com/fluxcd/pkg/testserver v0.4.0/go.mod h1:gjOKX41okmrGYOa4oOF2fiLedDAfPo1XaG/EzrUUGBI=
 github.com/fluxcd/source-controller/api v1.0.0 h1:lPjmCXmEiI3tY4pReeVQBMuyLgdH8462W5ewUa9kgYM=
 github.com/fluxcd/source-controller/api v1.0.0/go.mod h1:rAY5FRFGZUKpIFNyYANHIgPgJPvbALBHWsq/zHw/cXQ=
 github.com/foxcpp/go-mockdns v1.0.0 h1:7jBqxd3WDWwi/6WhDvacvH1XsN3rOLXyHM1uhvIx6FI=

internal/controller/helmrelease_controller.go

@@ -43,6 +43,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	aclv1 "github.com/fluxcd/pkg/apis/acl"
+	eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/runtime/acl"
 	runtimeClient "github.com/fluxcd/pkg/runtime/client"
@@ -211,9 +212,10 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 		if err := r.checkDependencies(ctx, obj); err != nil {
 			msg := fmt.Sprintf("dependencies do not meet ready condition (%s): retrying in %s",
 				err.Error(), r.requeueDependency.String())
-			r.Eventf(obj, obj.GetCurrent().ChartVersion, corev1.EventTypeWarning, err.Error())
-			log.Info(msg)
 			conditions.MarkFalse(obj, meta.ReadyCondition, v2.DependencyNotReadyReason, err.Error())
+			r.Eventf(obj, eventv1.EventSeverityInfo, v2.DependencyNotReadyReason, err.Error())
+			log.Info(msg)
+
 			// Exponential backoff would cause execution to be prolonged too much,
 			// instead we requeue on a fixed interval.
 			return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
@@ -226,8 +228,14 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 	hc, err := r.getHelmChart(ctx, obj)
 	if err != nil {
 		if acl.IsAccessDenied(err) {
+			conditions.MarkStalled(obj, aclv1.AccessDeniedReason, err.Error())
 			conditions.MarkFalse(obj, meta.ReadyCondition, aclv1.AccessDeniedReason, err.Error())
-			return ctrl.Result{}, err
+			conditions.Delete(obj, meta.ReconcilingCondition)
+
+			// Recovering from this is not possible without a restart of the
+			// controller or a change of spec, both triggering a new
+			// reconciliation.
+			return ctrl.Result{}, reconcile.TerminalError(err)
 		}
 
 		msg := fmt.Sprintf("could not get HelmChart object: %s", err.Error())
@@ -236,7 +244,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 	}
 
 	// Check chart readiness.
-	if hc.Generation != hc.Status.ObservedGeneration || !conditions.IsReady(hc) {
+	if hc.Generation != hc.Status.ObservedGeneration || !conditions.IsReady(hc) || hc.GetArtifact() == nil {
 		msg := fmt.Sprintf("HelmChart '%s/%s' is not ready", hc.GetNamespace(), hc.GetName())
 		log.Info(msg)
 		conditions.MarkFalse(obj, meta.ReadyCondition, "HelmChartNotReady", msg)