WIP: Refactor HelmChartReconciler

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

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
 }

controllers/bucket_controller.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_test.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/gitrepository_controller.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_test.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 {