Merge pull request #21 from fluxcd/log-context

Use same logger during reconcile operations

Author: hiddeco

api/v1alpha1/helmchart_types.go

@@ -104,7 +104,7 @@ func HelmChartNotReady(chart HelmChart, reason, message string) HelmChart {
 
 func HelmChartReadyMessage(chart HelmChart) string {
 	for _, condition := range chart.Status.Conditions {
-		if condition.Type == ReadyCondition {
+		if condition.Type == ReadyCondition && condition.Status == corev1.ConditionTrue {
 			return condition.Message
 		}
 	}

api/v1alpha1/helmrepository_types.go

@@ -100,7 +100,7 @@ func HelmRepositoryNotReady(repository HelmRepository, reason, message string) H
 
 func HelmRepositoryReadyMessage(repository HelmRepository) string {
 	for _, condition := range repository.Status.Conditions {
-		if condition.Type == ReadyCondition {
+		if condition.Type == ReadyCondition && condition.Status == corev1.ConditionTrue {
 			return condition.Message
 		}
 	}

controllers/gitrepository_controller.go

@@ -72,12 +72,14 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro
 	}
 
 	// try to remove old artifacts
-	r.gc(repo)
+	if err := r.gc(repo); err != nil {
+		log.Error(err, "artifacts GC failed")
+	}
 
 	// try git clone
 	syncedRepo, err := r.sync(ctx, *repo.DeepCopy())
 	if err != nil {
-		log.Info("Git repository sync failed", "error", err.Error())
+		log.Error(err, "Git repository sync failed")
 	}
 
 	// update status
@@ -322,10 +324,9 @@ func (r *GitRepositoryReconciler) shouldResetStatus(repository sourcev1.GitRepos
 	}
 }
 
-func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository) {
+func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository) error {
 	if repository.Status.Artifact != nil {
-		if err := r.Storage.RemoveAllButCurrent(*repository.Status.Artifact); err != nil {
-			r.Log.Info("Artifacts GC failed", "error", err)
-		}
+		return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact)
 	}
+	return nil
 }

controllers/helmchart_controller.go

@@ -72,7 +72,9 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 
 	// try to remove old artifacts
-	r.gc(chart)
+	if err := r.gc(chart); err != nil {
+		log.Error(err, "artifacts GC failed")
+	}
 
 	// get referenced chart repository
 	repository, err := r.chartRepository(ctx, chart)
@@ -87,12 +89,14 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	// set ownership reference so chart is garbage collected on
 	// repository removal
-	r.setOwnerRef(ctx, &chart, repository)
+	if err := r.setOwnerRef(ctx, &chart, repository); err != nil {
+		log.Error(err, "failed to set owner reference")
+	}
 
 	// try to pull chart
 	pulledChart, err := r.sync(repository, *chart.DeepCopy())
 	if err != nil {
-		log.Info("Helm chart sync failed", "error", err.Error())
+		log.Error(err, "Helm chart sync failed")
 	}
 
 	// update status
@@ -278,21 +282,18 @@ func (r *HelmChartReconciler) shouldResetStatus(chart sourcev1.HelmChart) (bool,
 	}
 }
 
-func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart) {
+func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart) error {
 	if chart.Status.Artifact != nil {
-		if err := r.Storage.RemoveAllButCurrent(*chart.Status.Artifact); err != nil {
-			r.Log.Info("Artifacts GC failed", "error", err)
-		}
+		return r.Storage.RemoveAllButCurrent(*chart.Status.Artifact)
 	}
+	return nil
 }
 
-func (r *HelmChartReconciler) setOwnerRef(ctx context.Context, chart *sourcev1.HelmChart, repository sourcev1.HelmRepository) {
-	if metav1.IsControlledBy(chart.GetObjectMeta(), repository.GetObjectMeta()) {
-		return
-	}
-	chart.SetOwnerReferences(append(chart.GetOwnerReferences(),
-		*metav1.NewControllerRef(repository.GetObjectMeta(), repository.GroupVersionKind())))
-	if err := r.Update(ctx, chart); err != nil {
-		r.Log.Error(err, fmt.Sprintf("failed to set owner reference to HelmRepository '%s'", repository.Name))
+func (r *HelmChartReconciler) setOwnerRef(ctx context.Context, chart *sourcev1.HelmChart, repository sourcev1.HelmRepository) error {
+	if !metav1.IsControlledBy(chart.GetObjectMeta(), repository.GetObjectMeta()) {
+		chart.SetOwnerReferences(append(chart.GetOwnerReferences(),
+			*metav1.NewControllerRef(repository.GetObjectMeta(), repository.GroupVersionKind())))
+		return r.Update(ctx, chart)
 	}
+	return nil
 }

controllers/helmrepository_controller.go

@@ -74,12 +74,14 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 	}
 
 	// try to remove old artifacts
-	r.gc(repository)
+	if err := r.gc(repository); err != nil {
+		log.Error(err, "artifacts GC failed")
+	}
 
 	// try to download index
 	syncedRepo, err := r.sync(*repository.DeepCopy())
 	if err != nil {
-		log.Info("Helm repository sync failed", "error", err.Error())
+		log.Error(err, "Helm repository sync failed")
 	}
 
 	// update status
@@ -221,10 +223,9 @@ func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRep
 	}
 }
 
-func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository) {
+func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository) error {
 	if repository.Status.Artifact != nil {
-		if err := r.Storage.RemoveAllButCurrent(*repository.Status.Artifact); err != nil {
-			r.Log.Info("Artifacts GC failed", "error", err)
-		}
+		return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact)
 	}
+	return nil
 }