Store and compare SHA256 checksum of merged values

integrations/apis/flux.weave.works/v1beta1/types.go

@@ -4,7 +4,7 @@ import (
 	"strings"
 
 	"github.com/ghodss/yaml"
-	v1 "k8s.io/api/core/v1"
+	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/helm/pkg/chartutil"
 
@@ -149,6 +149,22 @@ func (r HelmRelease) GetTimeout() int64 {
 	return *r.Spec.Timeout
 }
 
+// GetValuesFromSource maintains backwards compatibility with
+// ValueFileSecrets by merging them into the ValuesFrom array.
+func (r HelmRelease) GetValuesFromSource() []ValuesFromSource {
+	valuesFrom := r.Spec.ValuesFrom
+	// Maintain backwards compatibility with ValueFileSecrets
+	if r.Spec.ValueFileSecrets != nil {
+		var secretKeyRefs []ValuesFromSource
+		for _, ref := range r.Spec.ValueFileSecrets {
+			s := &v1.SecretKeySelector{LocalObjectReference: ref}
+			secretKeyRefs = append(secretKeyRefs, ValuesFromSource{SecretKeyRef: s})
+		}
+		valuesFrom = append(secretKeyRefs, valuesFrom...)
+	}
+	return valuesFrom
+}
+
 type HelmReleaseStatus struct {
 	// ReleaseName is the name as either supplied or generated.
 	// +optional
@@ -162,6 +178,10 @@ type HelmReleaseStatus struct {
 	// the controller.
 	ObservedGeneration int64 `json:"observedGeneration"`
 
+	// ValuesChecksum holds the SHA256 checksum of the last applied
+	// values.
+	ValuesChecksum string `json:"valuesChecksum"`
+
 	// Revision would define what Git hash or Chart version has currently
 	// been deployed.
 	// +optional

integrations/helm/chartsync/chartsync.go

@@ -280,6 +280,41 @@ func (chs *ChartChangeSync) maybeMirror(fhr fluxv1beta1.HelmRelease) {
 	}
 }
 
+func (chs *ChartChangeSync) CompareValuesChecksum(fhr fluxv1beta1.HelmRelease) bool {
+	return chs.compareValuesChecksum(fhr)
+}
+
+func (chs *ChartChangeSync) compareValuesChecksum(fhr fluxv1beta1.HelmRelease) bool {
+	chartPath, ok := "", false
+	if fhr.Spec.ChartSource.GitChartSource != nil {
+		// We need to hold the lock until have compared the values,
+		// so that the clone doesn't get swapped out from under us.
+		chs.clonesMu.Lock()
+		defer chs.clonesMu.Unlock()
+		chartPath, _, ok = chs.getGitChartSource(fhr)
+		if !ok {
+			return false
+		}
+	} else if fhr.Spec.ChartSource.RepoChartSource != nil {
+		chartPath, _, ok = chs.getRepoChartSource(fhr)
+		if !ok {
+			return false
+		}
+	}
+
+	values, err := release.Values(chs.kubeClient.CoreV1(), fhr.Namespace, chartPath, fhr.GetValuesFromSource(), fhr.Spec.Values)
+	if err != nil {
+		return false
+	}
+
+	strValues, err := values.YAML()
+	if err != nil {
+		return false
+	}
+
+	return fhr.Status.ValuesChecksum == release.ValuesChecksum([]byte(strValues))
+}
+
 // ReconcileReleaseDef asks the ChartChangeSync to examine the release
 // associated with a HelmRelease, and install or upgrade the
 // release if the chart it refers to has changed.
@@ -316,15 +351,15 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) {
 		if !ok {
 			return
 		}
-	} else if fhr.Spec.ChartSource.RepoChartSource != nil { // TODO(michael): make this dispatch more natural, or factor it out
+	} else if fhr.Spec.ChartSource.RepoChartSource != nil {
 		chartPath, chartRevision, ok = chs.getRepoChartSource(fhr)
 		if !ok {
 			return
 		}
 	}
 
 	if rel == nil {
-		_, err := chs.release.Install(chartPath, releaseName, fhr, release.InstallAction, opts, &chs.kubeClient)
+		_, checksum, 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.logger.Log("warning", "failed to install chart", "resource", fhr.ResourceID().String(), "err", err)
@@ -334,6 +369,9 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) {
 		if err = status.SetReleaseRevision(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)
 		}
+		if err = status.SetValuesChecksum(chs.ifClient.FluxV1beta1().HelmReleases(fhr.Namespace), fhr, checksum); err != nil {
+			chs.logger.Log("warning", "could not update the values checksum", "namespace", fhr.Namespace, "resource", fhr.Name, "err", err)
+		}
 		return
 	}
 
@@ -352,16 +390,22 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) {
 			chs.logger.Log("warning", "HelmRelease spec has diverged since we calculated if we should upgrade, skipping upgrade", "resource", fhr.ResourceID().String())
 			return
 		}
-		_, err = chs.release.Install(chartPath, releaseName, fhr, release.UpgradeAction, opts, &chs.kubeClient)
+		_, checksum, 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())
+			if err = status.SetValuesChecksum(chs.ifClient.FluxV1beta1().HelmReleases(fhr.Namespace), fhr, checksum); err != nil {
+				chs.logger.Log("warning", "could not update the values checksum", "namespace", fhr.Namespace, "resource", fhr.Name, "err", err)
+			}
 			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")
 		if err = status.SetReleaseRevision(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)
 		}
+		if err = status.SetValuesChecksum(chs.ifClient.FluxV1beta1().HelmReleases(fhr.Namespace), fhr, checksum); err != nil {
+			chs.logger.Log("warning", "could not update the values checksum", "namespace", fhr.Namespace, "resource", fhr.Name, "err", err)
+		}
 		return
 	}
 }
@@ -585,7 +629,7 @@ func (chs *ChartChangeSync) shouldUpgrade(chartsRepo string, currRel *hapi_relea
 	// Get the desired release state
 	opts := release.InstallOptions{DryRun: true}
 	tempRelName := string(fhr.UID)
-	desRel, err := chs.release.Install(chartsRepo, tempRelName, fhr, release.InstallAction, opts, &chs.kubeClient)
+	desRel, _, err := chs.release.Install(chartsRepo, tempRelName, fhr, release.InstallAction, opts, &chs.kubeClient)
 	if err != nil {
 		return false, err
 	}

integrations/helm/operator/operator.go

@@ -311,8 +311,10 @@ func (c *Controller) enqueueUpdateJob(old, new interface{}) {
 	}
 
 	// Skip if the current HelmRelease generation has been rolled
-	// back, as otherwise we will end up in a loop of failure.
-	if status.HasRolledBack(newFhr) {
+	// back, as otherwise we will end up in a loop of failure, but
+	// continue if the checksum of the values differs, as the failure
+	// may have been the result of the values contents.
+	if newFhr.Spec.Rollback.Enable && status.HasRolledBack(newFhr) && c.sync.CompareValuesChecksum(newFhr) {
 		c.logger.Log("warning", "release has been rolled back, skipping", "resource", newFhr.ResourceID().String())
 		return
 	}

integrations/helm/release/release.go

@@ -2,6 +2,8 @@ package release
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"io/ioutil"
 	"net/url"
@@ -14,7 +16,6 @@ import (
 	"github.com/ghodss/yaml"
 	"github.com/go-kit/kit/log"
 	"github.com/spf13/pflag"
-	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -171,43 +172,33 @@ func (r *Release) canDelete(name string) (bool, error) {
 // TODO(michael): cloneDir is only relevant if installing from git;
 // either split this procedure into two varieties, or make it more
 // general and calculate the path to the chart in the caller.
-func (r *Release) Install(chartPath, releaseName string, fhr flux_v1beta1.HelmRelease, action Action, opts InstallOptions, kubeClient *kubernetes.Clientset) (*hapi_release.Release, error) {
+func (r *Release) Install(chartPath, releaseName string, fhr flux_v1beta1.HelmRelease, action Action, opts InstallOptions, kubeClient *kubernetes.Clientset) (*hapi_release.Release, string, error) {
 	if chartPath == "" {
-		return nil, fmt.Errorf("empty path to chart supplied for resource %q", fhr.ResourceID().String())
+		return nil, "", fmt.Errorf("empty path to chart supplied for resource %q", fhr.ResourceID().String())
 	}
 	_, err := os.Stat(chartPath)
 	switch {
 	case os.IsNotExist(err):
-		return nil, fmt.Errorf("no file or dir at path to chart: %s", chartPath)
+		return nil, "", fmt.Errorf("no file or dir at path to chart: %s", chartPath)
 	case err != nil:
-		return nil, fmt.Errorf("error statting path given for chart %s: %s", chartPath, err.Error())
+		return nil, "", fmt.Errorf("error statting path given for chart %s: %s", chartPath, err.Error())
 	}
 
 	r.logger.Log("info", fmt.Sprintf("processing release %s (as %s)", GetReleaseName(fhr), releaseName),
 		"action", fmt.Sprintf("%v", action),
 		"options", fmt.Sprintf("%+v", opts),
 		"timeout", fmt.Sprintf("%vs", fhr.GetTimeout()))
 
-	valuesFrom := fhr.Spec.ValuesFrom
-	// Maintain backwards compatibility with ValueFileSecrets
-	if fhr.Spec.ValueFileSecrets != nil {
-		var secretKeyRefs []flux_v1beta1.ValuesFromSource
-		for _, ref := range fhr.Spec.ValueFileSecrets {
-			s := &v1.SecretKeySelector{LocalObjectReference: ref}
-			secretKeyRefs = append(secretKeyRefs, flux_v1beta1.ValuesFromSource{SecretKeyRef: s})
-		}
-		valuesFrom = append(secretKeyRefs, valuesFrom...)
-	}
-	vals, err := values(kubeClient.CoreV1(), fhr.Namespace, chartPath, valuesFrom, fhr.Spec.Values)
+	vals, err := Values(kubeClient.CoreV1(), fhr.Namespace, chartPath, fhr.GetValuesFromSource(), fhr.Spec.Values)
 	if err != nil {
 		r.logger.Log("error", fmt.Sprintf("Failed to compose values for Chart release [%s]: %v", fhr.Spec.ReleaseName, err))
-		return nil, err
+		return nil, "", err
 	}
 
 	strVals, err := vals.YAML()
 	if err != nil {
 		r.logger.Log("error", fmt.Sprintf("Problem with supplied customizations for Chart release [%s]: %v", fhr.Spec.ReleaseName, err))
-		return nil, err
+		return nil, "", err
 	}
 	rawVals := []byte(strVals)
 
@@ -232,15 +223,15 @@ func (r *Release) Install(chartPath, releaseName string, fhr flux_v1beta1.HelmRe
 				_, err = r.HelmClient.DeleteRelease(releaseName, k8shelm.DeletePurge(true))
 				if err != nil {
 					r.logger.Log("error", fmt.Sprintf("Release deletion error: %#v", err))
-					return nil, err
+					return nil, ValuesChecksum(rawVals), err
 				}
 			}
-			return nil, err
+			return nil, ValuesChecksum(rawVals), err
 		}
 		if !opts.DryRun {
 			r.annotateResources(res.Release, fhr)
 		}
-		return res.Release, err
+		return res.Release, ValuesChecksum(rawVals), err
 	case UpgradeAction:
 		res, err := r.HelmClient.UpdateRelease(
 			releaseName,
@@ -254,16 +245,16 @@ func (r *Release) Install(chartPath, releaseName string, fhr flux_v1beta1.HelmRe
 
 		if err != nil {
 			r.logger.Log("error", fmt.Sprintf("Chart upgrade release failed: %s: %#v", fhr.Spec.ReleaseName, err))
-			return nil, err
+			return nil, ValuesChecksum(rawVals), err
 		}
 		if !opts.DryRun {
 			r.annotateResources(res.Release, fhr)
 		}
-		return res.Release, err
+		return res.Release, ValuesChecksum(rawVals), err
 	default:
 		err = fmt.Errorf("Valid install options: CREATE, UPDATE. Provided: %s", action)
 		r.logger.Log("error", err.Error())
-		return nil, err
+		return nil, "", err
 	}
 }
 
@@ -344,14 +335,9 @@ func (r *Release) annotateResources(release *hapi_release.Release, fhr flux_v1be
 	}
 }
 
-// fhrResourceID constructs a flux.ResourceID for a HelmRelease resource.
-func fhrResourceID(fhr flux_v1beta1.HelmRelease) flux.ResourceID {
-	return flux.MakeResourceID(fhr.Namespace, "HelmRelease", fhr.Name)
-}
-
-// values tries to resolve all given value file sources and merges
+// Values tries to resolve all given value file sources and merges
 // them into one Values struct. It returns the merged Values.
-func values(corev1 k8sclientv1.CoreV1Interface, ns string, chartPath string, valuesFromSource []flux_v1beta1.ValuesFromSource, values chartutil.Values) (chartutil.Values, error) {
+func Values(corev1 k8sclientv1.CoreV1Interface, ns string, chartPath string, valuesFromSource []flux_v1beta1.ValuesFromSource, values chartutil.Values) (chartutil.Values, error) {
 	result := chartutil.Values{}
 
 	for _, v := range valuesFromSource {
@@ -455,6 +441,19 @@ func values(corev1 k8sclientv1.CoreV1Interface, ns string, chartPath string, val
 	return result, nil
 }
 
+// ValuesChecksum calculates the SHA256 checksum of the given raw
+// values.
+func ValuesChecksum(rawValues []byte) string {
+	hasher := sha256.New()
+	hasher.Write(rawValues)
+	return hex.EncodeToString(hasher.Sum(nil))
+}
+
+// fhrResourceID constructs a flux.ResourceID for a HelmRelease resource.
+func fhrResourceID(fhr flux_v1beta1.HelmRelease) flux.ResourceID {
+	return flux.MakeResourceID(fhr.Namespace, "HelmRelease", fhr.Name)
+}
+
 // Merges source and destination `chartutils.Values`, preferring values from the source Values
 // This is slightly adapted from https://github.com/helm/helm/blob/2332b480c9cb70a0d8a85247992d6155fbe82416/cmd/helm/install.go#L359
 func mergeValues(dest, src chartutil.Values) chartutil.Values {

integrations/helm/status/status.go

@@ -124,6 +124,24 @@ func SetReleaseRevision(client v1beta1client.HelmReleaseInterface, hr v1beta1.He
 	return err
 }
 
+// SetValuesChecksum updates the values checksum of the HelmRelease to
+// the given checksum.
+func SetValuesChecksum(client v1beta1client.HelmReleaseInterface, hr v1beta1.HelmRelease, valuesChecksum string) error {
+	cHr, err := client.Get(hr.Name, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
+
+	if cHr.Status.ValuesChecksum == valuesChecksum {
+		return nil
+	}
+
+	cHr.Status.ValuesChecksum = valuesChecksum
+
+	_, err = client.UpdateStatus(cHr)
+	return err
+}
+
 // SetObservedGeneration updates the observed generation status of the
 // HelmRelease to the given generation.
 func SetObservedGeneration(client v1beta1client.HelmReleaseInterface, hr v1beta1.HelmRelease, generation int64) error {