Small sync fixes Helm operator

integrations/helm/chartsync/chartsync.go

@@ -175,7 +175,7 @@ func (chs *ChartChangeSync) Run(stopCh <-chan struct{}, errc chan error, wg *syn
 						// Then why .. did you say .. it had changed? It may have been removed. Add it back and let it signal again.
 						chs.logger.Log("warning", "mirrored git repo disappeared after signalling change", "repo", mirror)
 						for _, fhr := range resources {
-							chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, "git mirror missing; starting mirroring again")
+							chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, "git mirror missing; starting mirroring again")
 							chs.maybeMirror(fhr)
 						}
 						continue
@@ -187,7 +187,7 @@ func (chs *ChartChangeSync) Run(stopCh <-chan struct{}, errc chan error, wg *syn
 						chs.logger.Log("info", "repo not ready yet, while attempting chart sync", "repo", mirror, "status", string(status))
 						for _, fhr := range resources {
 							// TODO(michael) log if there's a problem with the following?
-							chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, err.Error())
+							chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, err.Error())
 						}
 						continue
 					}
@@ -204,7 +204,7 @@ func (chs *ChartChangeSync) Run(stopCh <-chan struct{}, errc chan error, wg *syn
 						refHead, err := repo.Revision(ctx, ref)
 						cancel()
 						if err != nil {
-							chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonGitNotReady, "problem cloning from local git mirror: "+err.Error())
+							chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonGitNotReady, "problem cloning from local git mirror: "+err.Error())
 							chs.logger.Log("warning", "could not get revision for ref while checking for changes", "resource", fhr.ResourceID().String(), "repo", mirror, "ref", ref, "err", err)
 							continue
 						}
@@ -220,7 +220,7 @@ func (chs *ChartChangeSync) Run(stopCh <-chan struct{}, errc chan error, wg *syn
 							commits, err := repo.CommitsBetween(ctx, cloneForChart.head, refHead, path)
 							cancel()
 							if err != nil {
-								chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonGitNotReady, "problem cloning from local git mirror: "+err.Error())
+								chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonGitNotReady, "problem cloning from local git mirror: "+err.Error())
 								chs.logger.Log("warning", "could not get revision for ref while checking for changes", "resource", fhr.ResourceID().String(), "repo", mirror, "ref", ref, "err", err)
 								continue
 							}
@@ -232,7 +232,7 @@ func (chs *ChartChangeSync) Run(stopCh <-chan struct{}, errc chan error, wg *syn
 							newClone, err := repo.Export(ctx, refHead)
 							cancel()
 							if err != nil {
-								chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonGitNotReady, "problem cloning from local git mirror: "+err.Error())
+								chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonGitNotReady, "problem cloning from local git mirror: "+err.Error())
 								chs.logger.Log("warning", "could not clone from mirror while checking for changes", "resource", fhr.ResourceID().String(), "repo", mirror, "ref", ref, "err", err)
 								continue
 							}
@@ -331,24 +331,24 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) {
 			repo, ok := chs.mirrors.Get(mirrorName(chartSource))
 			if !ok {
 				chs.maybeMirror(fhr)
-				chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, "git repo "+chartSource.GitURL+" not mirrored yet")
+				chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, "git repo "+chartSource.GitURL+" not mirrored yet")
 				chs.logger.Log("info", "chart repo not cloned yet", "resource", fhr.ResourceID().String())
 			} else {
 				status, err := repo.Status()
 				if status != git.RepoReady {
-					chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, "git repo not mirrored yet: "+err.Error())
+					chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionUnknown, ReasonGitNotReady, "git repo not mirrored yet: "+err.Error())
 					chs.logger.Log("info", "chart repo not ready yet", "resource", fhr.ResourceID().String(), "status", string(status), "err", err)
 				}
 			}
 			return
 		}
-		chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionTrue, ReasonCloned, "successfully cloned git repo")
+		chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionTrue, ReasonCloned, "successfully cloned git repo")
 		chartPath = filepath.Join(chartClone.export.Dir(), chartSource.Path)
 		chartRevision = chartClone.head
 
 		if chs.config.UpdateDeps && !fhr.Spec.ChartSource.GitChartSource.SkipDepUpdate {
 			if err := updateDependencies(chartPath, ""); err != nil {
-				chs.setCondition(&fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionFalse, ReasonDependencyFailed, err.Error())
+				chs.setCondition(fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionFalse, ReasonDependencyFailed, err.Error())
 				chs.logger.Log("warning", "failed to update chart dependencies", "resource", fhr.ResourceID().String(), "err", err)
 				return
 			}
@@ -357,23 +357,23 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) {
 		chartSource := fhr.Spec.ChartSource.RepoChartSource
 		path, err := ensureChartFetched(chs.config.ChartCache, chartSource)
 		if err != nil {
-			chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonDownloadFailed, "chart download failed: "+err.Error())
+			chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionFalse, ReasonDownloadFailed, "chart download failed: "+err.Error())
 			chs.logger.Log("info", "chart download failed", "resource", fhr.ResourceID().String(), "err", err)
 			return
 		}
-		chs.setCondition(&fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionTrue, ReasonDownloaded, "chart fetched: "+filepath.Base(path))
+		chs.setCondition(fhr, fluxv1beta1.HelmReleaseChartFetched, v1.ConditionTrue, ReasonDownloaded, "chart fetched: "+filepath.Base(path))
 		chartPath = path
 		chartRevision = chartSource.Version
 	}
 
 	if rel == nil {
 		_, err := chs.release.Install(chartPath, releaseName, fhr, release.InstallAction, opts, &chs.kubeClient)
 		if err != nil {
-			chs.setCondition(&fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionFalse, ReasonInstallFailed, err.Error())
+			chs.setCondition(fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionFalse, ReasonInstallFailed, err.Error())
 			chs.logger.Log("warning", "failed to install chart", "resource", fhr.ResourceID().String(), "err", err)
 			return
 		}
-		chs.setCondition(&fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionTrue, ReasonSuccess, "helm install succeeded")
+		chs.setCondition(fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionTrue, ReasonSuccess, "helm install succeeded")
 		if err = status.UpdateReleaseRevision(chs.ifClient.FluxV1beta1().HelmReleases(fhr.Namespace), fhr, chartRevision); err != nil {
 			chs.logger.Log("warning", "could not update the release revision", "namespace", fhr.Namespace, "resource", fhr.Name, "err", err)
 		}
@@ -397,11 +397,11 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) {
 		}
 		_, err = chs.release.Install(chartPath, releaseName, fhr, release.UpgradeAction, opts, &chs.kubeClient)
 		if err != nil {
-			chs.setCondition(&fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionFalse, ReasonUpgradeFailed, err.Error())
+			chs.setCondition(fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionFalse, ReasonUpgradeFailed, err.Error())
 			chs.logger.Log("warning", "failed to upgrade chart", "resource", fhr.ResourceID().String(), "err", err)
 			return
 		}
-		chs.setCondition(&fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionTrue, ReasonSuccess, "helm upgrade succeeded")
+		chs.setCondition(fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionTrue, ReasonSuccess, "helm upgrade succeeded")
 		if err = status.UpdateReleaseRevision(chs.ifClient.FluxV1beta1().HelmReleases(fhr.Namespace), fhr, chartRevision); err != nil {
 			chs.logger.Log("warning", "could not update the release revision", "resource", fhr.ResourceID().String(), "err", err)
 		}
@@ -463,7 +463,7 @@ func (chs *ChartChangeSync) getCustomResourcesForMirror(mirror string) ([]fluxv1
 // information. New information is something that adds or changes the
 // status, reason or message (i.e., anything but the transition time)
 // for one of the types of condition.
-func (chs *ChartChangeSync) setCondition(fhr *fluxv1beta1.HelmRelease, typ fluxv1beta1.HelmReleaseConditionType, st v1.ConditionStatus, reason, message string) error {
+func (chs *ChartChangeSync) setCondition(fhr fluxv1beta1.HelmRelease, typ fluxv1beta1.HelmReleaseConditionType, st v1.ConditionStatus, reason, message string) error {
 	for _, c := range fhr.Status.Conditions {
 		if c.Type == typ && c.Status == st && c.Message == message && c.Reason == reason {
 			return nil

integrations/helm/operator/operator.go

@@ -282,8 +282,19 @@ func (c *Controller) enqueueUpdateJob(old, new interface{}) {
 		return
 	}
 
-	log := []string{"info", "enqueuing release upgrade"}
-	if diff := cmp.Diff(oldFhr.Spec, newFhr.Spec); diff != "" && c.logDiffs {
+	diff := cmp.Diff(oldFhr.Spec, newFhr.Spec)
+
+	// Filter out any update notifications that are due to status
+	// updates, as the dry-run that determines if we should upgrade
+	// is expensive, but _without_ filtering out updates that are
+	// from the periodic refresh, as we still want to detect (and
+	// undo) mutations to Helm charts.
+	if sDiff := cmp.Diff(oldFhr.Status, newFhr.Status); diff == "" && sDiff != "" {
+		return
+	}
+
+	log := []string{"info", "enqueuing release"}
+	if diff != "" && c.logDiffs {
 		log = append(log, "diff", diff)
 	}
 	log = append(log, "resource", newFhr.ResourceID().String())

integrations/helm/status/conditions.go

@@ -3,6 +3,7 @@ package status
 import (
 	"github.com/weaveworks/flux/integrations/apis/flux.weave.works/v1beta1"
 	v1beta1client "github.com/weaveworks/flux/integrations/client/clientset/versioned/typed/flux.weave.works/v1beta1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // We can't rely on having UpdateStatus, or strategic merge patching
@@ -27,13 +28,16 @@ updates:
 	status.Conditions = newConditions
 }
 
-// UpdateConditions applies the updates to the HelmRelease given, and
-// updates the resource in the cluster.
-func UpdateConditions(client v1beta1client.HelmReleaseInterface, fhr *v1beta1.HelmRelease, updates ...v1beta1.HelmReleaseCondition) error {
-	fhrCopy := fhr.DeepCopy()
-
-	UpdateConditionsPatch(&fhrCopy.Status, updates...)
-	_, err := client.UpdateStatus(fhrCopy)
+// UpdateConditions retrieves a new copy of the HelmRelease given,
+// applies the updates to this copy, and updates the resource in the
+// cluster.
+func UpdateConditions(client v1beta1client.HelmReleaseInterface, fhr v1beta1.HelmRelease, updates ...v1beta1.HelmReleaseCondition) error {
+	cFhr, err := client.Get(fhr.Name, v1.GetOptions{})
+	if err != nil {
+		return err
+	}
 
+	UpdateConditionsPatch(&cFhr.Status, updates...)
+	_, err = client.UpdateStatus(cFhr)
 	return err
 }

integrations/helm/status/status.go

@@ -102,18 +102,24 @@ bail:
 }
 
 func UpdateReleaseStatus(client v1beta1client.HelmReleaseInterface, fhr v1beta1.HelmRelease, releaseName, releaseStatus string) error {
-	fhr.Status.ReleaseName = releaseName
-	fhr.Status.ReleaseStatus = releaseStatus
-
-	_, err := client.UpdateStatus(&fhr)
+	cFhr, err := client.Get(fhr.Name, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
 
+	cFhr.Status.ReleaseName = releaseName
+	cFhr.Status.ReleaseStatus = releaseStatus
+	_, err = client.UpdateStatus(cFhr)
 	return err
 }
 
 func UpdateReleaseRevision(client v1beta1client.HelmReleaseInterface, fhr v1beta1.HelmRelease, revision string) error {
-	fhr.Status.Revision = revision
-
-	_, err := client.UpdateStatus(&fhr)
+	cFhr, err := client.Get(fhr.Name, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
 
+	cFhr.Status.Revision = revision
+	_, err = client.UpdateStatus(cFhr)
 	return err
 }