From 9e626484aade7437d7e9452213d42ad5f1d0af21 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 20 May 2019 17:06:59 +0200 Subject: [PATCH] Store and compare SHA256 checksum of merged values --- .../apis/flux.weave.works/v1beta1/types.go | 22 ++++++- integrations/helm/chartsync/chartsync.go | 57 +++++++++++++---- integrations/helm/operator/operator.go | 6 +- integrations/helm/release/release.go | 62 +++++++++---------- integrations/helm/status/status.go | 18 ++++++ 5 files changed, 120 insertions(+), 45 deletions(-) diff --git a/integrations/apis/flux.weave.works/v1beta1/types.go b/integrations/apis/flux.weave.works/v1beta1/types.go index e74456850..e6a532c4a 100644 --- a/integrations/apis/flux.weave.works/v1beta1/types.go +++ b/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 diff --git a/integrations/helm/chartsync/chartsync.go b/integrations/helm/chartsync/chartsync.go index dc68f81b3..0bff1d2ec 100644 --- a/integrations/helm/chartsync/chartsync.go +++ b/integrations/helm/chartsync/chartsync.go @@ -280,17 +280,43 @@ func (chs *ChartChangeSync) maybeMirror(fhr fluxv1beta1.HelmRelease) { } } +// CompareValuesChecksum recalculates the checksum of the values +// and compares it to the last recorded checksum. +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. func (chs *ChartChangeSync) ReconcileReleaseDef(fhr fluxv1beta1.HelmRelease) { - chs.reconcileReleaseDef(fhr) -} - -// reconcileReleaseDef looks up the helm release associated with a -// HelmRelease resource, and either installs, upgrades, or does -// nothing, depending on the state (or absence) of the release. -func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) { defer chs.updateObservedGeneration(fhr) releaseName := release.GetReleaseName(fhr) @@ -316,7 +342,7 @@ 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 @@ -324,7 +350,7 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) { } 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 +360,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,9 +381,12 @@ 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 } @@ -362,6 +394,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", "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 +620,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 } diff --git a/integrations/helm/operator/operator.go b/integrations/helm/operator/operator.go index a02886fbc..656aff3cf 100644 --- a/integrations/helm/operator/operator.go +++ b/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 } diff --git a/integrations/helm/release/release.go b/integrations/helm/release/release.go index fd19ececb..3fbe65977 100644 --- a/integrations/helm/release/release.go +++ b/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,16 +172,16 @@ 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), @@ -188,28 +189,19 @@ func (r *Release) Install(chartPath, releaseName string, fhr flux_v1beta1.HelmRe "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) + checksum := ValuesChecksum(rawVals) switch action { case InstallAction: @@ -232,15 +224,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, "", err } } - return nil, err + return nil, checksum, err } if !opts.DryRun { r.annotateResources(res.Release, fhr) } - return res.Release, err + return res.Release, checksum, err case UpgradeAction: res, err := r.HelmClient.UpdateRelease( releaseName, @@ -254,16 +246,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, checksum, err } if !opts.DryRun { r.annotateResources(res.Release, fhr) } - return res.Release, err + return res.Release, checksum, 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 +336,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 +442,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 { diff --git a/integrations/helm/status/status.go b/integrations/helm/status/status.go index 7fb9c242c..dd3df3635 100644 --- a/integrations/helm/status/status.go +++ b/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 valuesChecksum == "" || 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 {