WIP: Refactor HelmChartReconciler

Signed-off-by: Hidde Beydals <hello@hidde.co>

Author: hiddeco

api/v1beta1/condition_types.go

@@ -24,6 +24,8 @@ const (
 	SourceVerifiedCondition string = "SourceVerified"
 
 	ArtifactAvailableCondition string = "ArtifactAvailable"
+
+	SourceRefReadyCondition string = "SourceRefReady"
 )
 
 const (

api/v1beta1/helmchart_types.go

@@ -18,13 +18,19 @@ package v1beta1
 
 import (
 	"github.com/fluxcd/pkg/apis/meta"
-	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // HelmChartKind is the string representation of a HelmChart.
 const HelmChartKind = "HelmChart"
 
+const (
+	DependenciesBuildCondition string = "DependenciesBuild"
+	ValuesFilesMergedCondition string = "ValuesFilesMerged"
+	ChartPackagedCondition     string = "ChartPackaged"
+	ChartReconciled            string = "ChartReconciled"
+)
+
 // HelmChartSpec defines the desired state of a Helm chart.
 type HelmChartSpec struct {
 	// The name or path the Helm chart is available at in the SourceRef.
@@ -122,52 +128,20 @@ const (
 	ChartPackageSucceededReason string = "ChartPackageSucceeded"
 )
 
-// HelmChartProgressing resets the conditions of the HelmChart to meta.Condition
-// of type meta.ReadyCondition with status 'Unknown' and meta.ProgressingReason
-// reason and message. It returns the modified HelmChart.
-func HelmChartProgressing(chart HelmChart) HelmChart {
-	chart.Status.ObservedGeneration = chart.Generation
-	chart.Status.URL = ""
-	chart.Status.Conditions = []metav1.Condition{}
-	meta.SetResourceCondition(&chart, meta.ReadyCondition, metav1.ConditionUnknown, meta.ProgressingReason, "reconciliation in progress")
-	return chart
-}
-
-// HelmChartReady sets the given Artifact and URL on the HelmChart and sets the
-// meta.ReadyCondition to 'True', with the given reason and message. It returns
-// the modified HelmChart.
-func HelmChartReady(chart HelmChart, artifact Artifact, url, reason, message string) HelmChart {
-	chart.Status.Artifact = &artifact
-	chart.Status.URL = url
-	meta.SetResourceCondition(&chart, meta.ReadyCondition, metav1.ConditionTrue, reason, message)
-	return chart
-}
-
-// HelmChartNotReady sets the meta.ReadyCondition on the given HelmChart to
-// 'False', with the given reason and message. It returns the modified
-// HelmChart.
-func HelmChartNotReady(chart HelmChart, reason, message string) HelmChart {
-	meta.SetResourceCondition(&chart, meta.ReadyCondition, metav1.ConditionFalse, reason, message)
-	return chart
-}
-
-// HelmChartReadyMessage returns the message of the meta.ReadyCondition with
-// status 'True', or an empty string.
-func HelmChartReadyMessage(chart HelmChart) string {
-	if c := apimeta.FindStatusCondition(chart.Status.Conditions, meta.ReadyCondition); c != nil {
-		if c.Status == metav1.ConditionTrue {
-			return c.Message
-		}
-	}
-	return ""
-}
-
 // GetArtifact returns the latest artifact from the source if present in the
 // status sub-resource.
 func (in *HelmChart) GetArtifact() *Artifact {
 	return in.Status.Artifact
 }
 
+func (in HelmChart) GetConditions() []metav1.Condition {
+	return in.Status.Conditions
+}
+
+func (in *HelmChart) SetConditions(conditions []metav1.Condition) {
+	in.Status.Conditions = conditions
+}
+
 // GetStatusConditions returns a pointer to the Status.Conditions slice
 func (in *HelmChart) GetStatusConditions() *[]metav1.Condition {
 	return &in.Status.Conditions

api/v1beta1/source.go

@@ -18,6 +18,7 @@ package v1beta1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 )
 
 const (
@@ -29,9 +30,15 @@ const (
 // Source interface must be supported by all API types.
 // +k8s:deepcopy-gen=false
 type Source interface {
+	metav1.Object
+	runtime.Object
+
 	// GetArtifact returns the latest artifact from the source if present in the
 	// status sub-resource.
 	GetArtifact() *Artifact
+	// GetConditions returns the conditions of the source if present in the
+	// status sub-resource.
+	GetConditions() []metav1.Condition
 	// GetInterval returns the interval at which the source is updated.
 	GetInterval() metav1.Duration
 }

api/v1beta1/zz_generated.deepcopy.go

@@ -147,10 +147,16 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 			// We have now observed this generation
 			patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
 
-			if readyCondition := conditions.Get(obj, meta.ReadyCondition); readyCondition.Status == metav1.ConditionFalse {
-				// As we are no longer reconciling, and the end-state
+			readyCondition := conditions.Get(obj, meta.ReadyCondition)
+			switch readyCondition.Status {
+			case metav1.ConditionFalse:
+				// As we are no longer reconciling and the end-state
 				// is not ready, the reconciliation has stalled
 				conditions.MarkTrue(obj, meta.StalledCondition, readyCondition.Reason, readyCondition.Message)
+			case metav1.ConditionTrue:
+				// As we are no longer reconciling and the end-state
+				// is ready, the reconciliation is no longer stalled
+				conditions.Delete(obj, meta.StalledCondition)
 			}
 		}
 
@@ -357,7 +363,7 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.
 	// The artifact is up-to-date
 	if obj.GetArtifact().HasRevision(artifact.Revision) {
 		logr.FromContext(ctx).Info("Artifact is up-to-date")
-		conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision)
+		conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Compressed source to artifact with revision %s", artifact.Revision)
 		return ctrl.Result{RequeueAfter: obj.GetInterval().Duration}, nil
 	}
 
@@ -392,7 +398,7 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.
 
 	// Record it on the object
 	obj.Status.Artifact = artifact.DeepCopy()
-	conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision)
+	conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Compressed source to artifact with revision %s", artifact.Revision)
 	r.Events.Eventf(ctx, obj, map[string]string{
 		"revision": obj.GetArtifact().Revision,
 	}, events.EventSeverityInfo, sourcev1.GitOperationSucceedReason, conditions.Get(obj, sourcev1.ArtifactAvailableCondition).Message)

controllers/bucket_controller.go

@@ -41,7 +41,7 @@ import (
 
 const (
 	bucketInterval = 1 * time.Second
-	bucketTimeout = 30 * time.Second
+	bucketTimeout  = 30 * time.Second
 )
 
 func TestBucketReconciler_Reconcile(t *testing.T) {

controllers/bucket_controller_test.go

@@ -154,10 +154,16 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			// We have now observed this generation
 			patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
 
-			if readyCondition := conditions.Get(obj, meta.ReadyCondition); readyCondition.Status == metav1.ConditionFalse {
-				// As we are no longer reconciling, and the end-state
+			readyCondition := conditions.Get(obj, meta.ReadyCondition)
+			switch readyCondition.Status {
+			case metav1.ConditionFalse:
+				// As we are no longer reconciling and the end-state
 				// is not ready, the reconciliation has stalled
 				conditions.MarkTrue(obj, meta.StalledCondition, readyCondition.Reason, readyCondition.Message)
+			case metav1.ConditionTrue:
+				// As we are no longer reconciling and the end-state
+				// is ready, the reconciliation is no longer stalled
+				conditions.Delete(obj, meta.StalledCondition)
 			}
 		}
 
@@ -329,7 +335,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
 
 	// Create potential new artifact
 	*artifact = r.Storage.NewArtifactFor(obj.Kind, obj, revision, fmt.Sprintf("%s.tar.gz", commit.Hash()))
-	conditions.MarkTrue(obj, sourcev1.SourceAvailableCondition, sourcev1.GitOperationSucceedReason, "Checked out revision %s from %s", revision, obj.Spec.URL)
+	conditions.MarkTrue(obj, sourcev1.SourceAvailableCondition, sourcev1.GitOperationSucceedReason, "Checked out revision %s", revision)
 
 	return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil
 }
@@ -391,7 +397,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
 	// Record it on the object
 	obj.Status.Artifact = artifact.DeepCopy()
 	obj.Status.IncludedArtifacts = includes
-	conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision)
+	conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Compressed source to artifact with revision %s", artifact.Revision)
 	r.Events.Eventf(ctx, obj, map[string]string{
 		"revision": obj.GetArtifact().Revision,
 	}, events.EventSeverityInfo, sourcev1.GitOperationSucceedReason, conditions.Get(obj, sourcev1.ArtifactAvailableCondition).Message)

controllers/gitrepository_controller.go

@@ -160,7 +160,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			protocol: "http",
 			want:     ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, "SuccessfulCheckout", "Checked out revision master/<commit> from <url>"),
+				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, sourcev1.GitOperationSucceedReason, "Checked out revision master/<commit>"),
 			},
 		},
 		{
@@ -184,7 +184,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, "SuccessfulCheckout", "Checked out revision master/<commit> from <url>"),
+				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, sourcev1.GitOperationSucceedReason, "Checked out revision master/<commit>"),
 			},
 		},
 		{
@@ -208,7 +208,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, "SuccessfulCheckout", "Checked out revision master/<commit> from <url>"),
+				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, sourcev1.GitOperationSucceedReason, "Checked out revision master/<commit>"),
 			},
 		},
 		{
@@ -281,7 +281,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, "SuccessfulCheckout", "Checked out revision master/<commit> from <url>"),
+				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, sourcev1.GitOperationSucceedReason, "Checked out revision master/<commit>"),
 			},
 		},
 		{
@@ -305,7 +305,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, "SuccessfulCheckout", "Checked out revision master/<commit> from <url>"),
+				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, sourcev1.GitOperationSucceedReason, "Checked out revision master/<commit>"),
 			},
 		},
 		{
@@ -319,7 +319,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "AuthenticationFailed", "Failed to get auth secret /non-existing: secrets \"non-existing\" not found"),
+				*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "AuthenticationFailed", "Failed to get secret /non-existing: secrets \"non-existing\" not found"),
 			},
 		},
 	}
@@ -593,7 +593,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision main/revision"),
+				*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Compressed source to artifact with revision main/revision"),
 			},
 		},
 		{
@@ -625,7 +625,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Archived artifact revision main/revision"),
+				*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Compressed source to artifact with revision main/revision"),
 			},
 		},
 	}
@@ -1054,6 +1054,12 @@ func TestMain(m *testing.M) {
 
 	eventsHelper = controller.MakeEvents(newTestEnv, "test", nil)
 	metricsHelper = controller.MustMakeMetrics(newTestEnv)
+	getters := getter.Providers{
+		getter.Provider{
+			Schemes: []string{"http", "https"},
+			New:     getter.NewHTTPGetter,
+		},
+	}
 
 	if err := (&GitRepositoryReconciler{
 		Client:  newTestEnv,
@@ -1068,12 +1074,7 @@ func TestMain(m *testing.M) {
 		Client:  newTestEnv,
 		Events:  eventsHelper,
 		Metrics: metricsHelper,
-		Getters: getter.Providers{
-			getter.Provider{
-				Schemes: []string{"http", "https"},
-				New:     getter.NewHTTPGetter,
-			},
-		},
+		Getters: getters,
 		Storage: storage,
 	}).SetupWithManager(newTestEnv); err != nil {
 		panic(fmt.Sprintf("Failed to start HelmRepositoryReconciler: %v", err))
@@ -1088,6 +1089,16 @@ func TestMain(m *testing.M) {
 		panic(fmt.Sprintf("Failed to start BucketReconciler: %v", err))
 	}
 
+	if err := (&HelmChartReconciler{
+		Client:  newTestEnv,
+		Events:  eventsHelper,
+		Metrics: metricsHelper,
+		Getters: getters,
+		Storage: storage,
+	}).SetupWithManager(newTestEnv); err != nil {
+		panic(fmt.Sprintf("Failed to start HelmChartReconciler: %v", err))
+	}
+
 	go func() {
 		fmt.Println("Starting the test environment manager")
 		if err := newTestEnv.StartManager(ctx); err != nil {