Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
release: rely on Helm storage for rollbacks
Browse files Browse the repository at this point in the history 
This commit removes our own state keeping around rollbacks except
for the condition we set marking a rollback has happend, but
instead relies on comparison of a dry-run with the latest failed
release. If this results in a diff, a new upgrade will be attempted.

The reason we move to this approach is that it is not possible to
take all variables safely into account and determine if we should
perform a new upgrade attempt based on our own bookkeeping, with
a long list of edge-case bugs as a result.

Relying on the real truth is always better than a reflection of it,
which in the end may not be the actual truth at all.
  • Loading branch information
@hiddeco
hiddeco committed Jan 24, 2020
1 parent f8ced7a commit c695af2
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 145 deletions.
9 changes: 0 additions & 9 deletions pkg/apis/helm.fluxcd.io/v1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,20 +260,11 @@ 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
Revision string `json:"revision,omitempty"`

// PrevRevision would define what Git hash or Chart version had previously
// been deployed.
// +optional
PrevRevision string `json:"prevRevision,omitempty"`

// Conditions contains observations of the resource's state, e.g.,
// has the chart which it refers to been fetched.
// +optional
Expand Down
20 changes: 9 additions & 11 deletions pkg/helm/v2/history.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,22 @@ import (
"github.com/pkg/errors"

helmv2 "k8s.io/helm/pkg/helm"
"k8s.io/helm/pkg/proto/hapi/release"

"github.com/fluxcd/helm-operator/pkg/helm"
)

func (h *HelmV2) History(releaseName string, opts helm.HistoryOptions) ([]*helm.Release, error) {
res, err := h.client.ReleaseHistory(releaseName, helmv2.WithMaxHistory(int32(opts.Max)))
max := helmv2.WithMaxHistory(256)
if opts.Max != 0 {
max = helmv2.WithMaxHistory(int32(opts.Max))
}
res, err := h.client.ReleaseHistory(releaseName, max)
if err != nil {
return nil, errors.Wrapf(statusMessageErr(err), "failed to retrieve history for [%s]", releaseName)
}
return getReleaseHistory(res.Releases), nil
}

func getReleaseHistory(rls []*release.Release) []*helm.Release {
history := make([]*helm.Release, len(rls))
for i := len(rls) - 1; i >= 0; i-- {
r := rls[i]
history = append(history, releaseToGenericRelease(r))
var rels []*helm.Release
for _, r := range res.Releases {
rels = append(rels, releaseToGenericRelease(r))
}
return history
return rels, nil
}
17 changes: 3 additions & 14 deletions pkg/helm/v3/history.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"github.com/pkg/errors"

"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/releaseutil"

"github.com/fluxcd/helm-operator/pkg/helm"
Expand All @@ -28,21 +27,11 @@ func (h *HelmV3) History(releaseName string, opts helm.HistoryOptions) ([]*helm.

releaseutil.Reverse(hist, releaseutil.SortByRevision)

var rels []*release.Release
var rels []*helm.Release
for i := 0; i < min(len(hist), client.Max); i++ {
rels = append(rels, hist[i])
rels = append(rels, releaseToGenericRelease(hist[i]))
}

return getReleaseHistory(hist), nil
}

func getReleaseHistory(rls []*release.Release) []*helm.Release {
history := make([]*helm.Release, len(rls))
for i := len(rls) - 1; i >= 0; i-- {
r := rls[i]
history = append(history, releaseToGenericRelease(r))
}
return history
return rels, nil
}

func min(x, y int) int {
Expand Down
93 changes: 52 additions & 41 deletions pkg/release/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ func (r *Release) Sync(client helm.Client, hr *v1.HelmRelease) (rHr *v1.HelmRele
logger.Log("error", ErrComposingValues.Error(), "err", err.Error())
return hr, ErrComposingValues
}
defer status.SetValuesChecksum(r.helmReleaseClient.HelmReleases(hr.Namespace), hr, composedValues.Checksum())

if ok, err := shouldSync(logger, client, hr, curRel, chartPath, revision, composedValues, r.config.LogDiffs); !ok {
if err != nil {
Expand Down Expand Up @@ -256,9 +255,12 @@ func (r *Release) Sync(client helm.Client, hr *v1.HelmRelease) (rHr *v1.HelmRele
if performRollback {
logger.Log("info", "rolling back failed Helm release")
rel, err = client.Rollback(hr.GetReleaseName(), helm.RollbackOptions{
Namespace: hr.GetTargetNamespace(),
Timeout: hr.GetTimeout(),
Force: hr.Spec.ForceUpgrade,
Namespace: hr.GetTargetNamespace(),
Timeout: hr.Spec.Rollback.GetTimeout(),
Wait: hr.Spec.Rollback.Wait,
DisableHooks: hr.Spec.Rollback.DisableHooks,
Recreate: hr.Spec.Rollback.Recreate,
Force: hr.Spec.Rollback.Force,
})
if err != nil {
_ = status.SetCondition(r.helmReleaseClient.HelmReleases(hr.Namespace), hr, status.NewCondition(
Expand All @@ -268,7 +270,6 @@ func (r *Release) Sync(client helm.Client, hr *v1.HelmRelease) (rHr *v1.HelmRele
}
_ = status.SetCondition(r.helmReleaseClient.HelmReleases(hr.Namespace), hr, status.NewCondition(
v1.HelmReleaseRolledBack, corev1.ConditionTrue, ReasonSuccess, "Helm rollback succeeded"))
status.SetPrevReleaseRevision(r.helmReleaseClient.HelmReleases(hr.Namespace), hr, revision)
logger.Log("info", "Helm rollback succeeded")

// We should still report failure.
Expand Down Expand Up @@ -299,82 +300,92 @@ func (r *Release) Uninstall(client helm.Client, hr *v1.HelmRelease) {

// shouldSync determines if the given `v1.HelmRelease` should be synced
// with Helm. The cheapest checks which do not require a dry-run are
// consulted first (e.g. is this our first sync, has the release been
// rolled back, have we already seen this revision of the resource);
// before running the dry-run release to determine if any undefined
// mutations have occurred.
// consulted first (e.g. is this our first sync, have we already seen
// this revision of the resource); before running the dry-run release to
// determine if any undefined mutations have occurred.
func shouldSync(logger log.Logger, client helm.Client, hr *v1.HelmRelease, curRel *helm.Release,
chartPath, revision string, values helm.Values, logDiffs bool) (bool, error) {
chartPath, revision string, values helm.Values, logDiffs bool) (bool, error) {

// Without valid YAML we will not get anywhere, return early.
b, err := values.YAML()
if err != nil {
return false, ErrComposingValues
}

// If there is no existing release, we should simply sync.
if curRel == nil {
logger.Log("info", "no existing release", "action", "install")
// If there is no existing release, we should simply sync.
return true, nil
}

// If the release is not managed by our resource, we skip to avoid conflicts.
if ok, resourceID := managedByHelmRelease(curRel, *hr); !ok {
logger.Log("warning", "release appears to be managed by "+resourceID, "action", "skip")
return false, nil
}

// If the current state of the release does not allow us to safely upgrade, we skip.
if s := curRel.Info.Status; !s.AllowsUpgrade() {
logger.Log("warning", "unable to sync release with status "+s.String(), "action", "skip")
return false, nil
}

if status.HasRolledBack(*hr, revision) {
if hr.Status.ValuesChecksum != values.Checksum() {
// The release has been rolled back but the values have
// changed. We should attempt a new sync to see if the
// change resolved the issue that triggered the rollback.
logger.Log("info", "values appear to have changed since rollback", "action", "upgrade")
return true, nil
}
logger.Log("warning", "release has been rolled back", "action", "skip")
return false, nil
}

// If we have not processed this generation of the release, we should sync.
if !status.HasSynced(*hr) {
logger.Log("info", "release has not yet been processed", "action", "upgrade")

// The generation of this `v1.HelmRelease` has not been
// processed, we should simply sync.
return true, nil
}

b, err := values.YAML()
if err != nil {
// Without valid YAML values we are unable to sync.
return false, ErrComposingValues
}

// Next, we perform a dry-run upgrade and compare the result against the
// latest release _or_ the latest failed release in case of a rollback.
// If this results in one or more diffs we should sync.
logger.Log("info", "performing dry-run upgrade to see if release has diverged")

// Perform a dry-run upgrade so that we can compare what we ought
// to be running matches what is defined in the `v1.HelmRelease`.
desRel, err := client.UpgradeFromPath(chartPath, hr.GetReleaseName(), b, helm.UpgradeOptions{DryRun: true})
if err != nil {
return false, err
}

curValues, desValues := curRel.Values, desRel.Values
curChart, desChart := curRel.Chart, desRel.Chart
var vDiff, cDiff string
switch {
case status.HasRolledBack(*hr):
logger.Log("info", "release has been rolled back, comparing dry-run output with latest failed release")
rels, err := client.History(hr.GetReleaseName(), helm.HistoryOptions{Namespace: hr.GetTargetNamespace()})
if err != nil {
return false, err
}
for _, r := range rels {
if r.Info.Status == helm.StatusFailed {
vDiff, cDiff = compareRelease(r, desRel)
break
}
}
default:
vDiff, cDiff = compareRelease(curRel, desRel)
}

// Compare values to detect mutations.
vDiff := cmp.Diff(curValues, desValues)
if vDiff != "" && logDiffs {
logger.Log("info", "values have diverged", "diff", vDiff)
}

// Compare chart to detect mutations.
cDiff := cmp.Diff(curChart, desChart)
if cDiff != "" && logDiffs {
logger.Log("info", "chart has diverged", "diff", cDiff)
}

if cDiff != "" || vDiff != "" {
logger.Log("info", "dry-run differed", "action", "upgrade")
} else {
logger.Log("info", "no changes", "action", "skip")
}

return vDiff != "" || cDiff != "", nil
}

// compareRelease compares the values and charts of the two given
// releases and returns the diff sets.
func compareRelease(j *helm.Release, k *helm.Release) (string, string) {
return cmp.Diff(j.Values, k.Values), cmp.Diff(j.Chart, k.Chart)
}

// releaseLogger returns a logger in the context of the given
// HelmRelease (that being, with metadata included).
func releaseLogger(logger log.Logger, client helm.Client, hr *v1.HelmRelease) log.Logger {
Expand Down
71 changes: 1 addition & 70 deletions pkg/status/status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ func SetReleaseRevision(client v1client.HelmReleaseInterface, hr *helmfluxv1.Hel
}

cHr := hr.DeepCopy()
cHr.Status.PrevRevision = cHr.Status.Revision
cHr.Status.Revision = revision

_, err = client.UpdateStatus(cHr)
Expand All @@ -146,63 +145,6 @@ func SetReleaseRevision(client v1client.HelmReleaseInterface, hr *helmfluxv1.Hel
return err
}

// SetReleaseRevision updates the previous revision in the status of the
// HelmRelease to the given revision, its main purpose is to be able to
// record the revision of a failed release.
func SetPrevReleaseRevision(client v1client.HelmReleaseInterface, hr *helmfluxv1.HelmRelease, revision string) error {

firstTry := true
err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
if !firstTry {
var getErr error
hr, getErr = client.Get(hr.Name, metav1.GetOptions{})
if getErr != nil {
return getErr
}
}

if revision == "" || hr.Status.PrevRevision == revision {
return
}

cHr := hr.DeepCopy()
cHr.Status.PrevRevision = revision

_, err = client.UpdateStatus(cHr)
firstTry = false
return
})
return err
}

// SetValuesChecksum updates the values checksum of the HelmRelease to
// the given checksum.
func SetValuesChecksum(client v1client.HelmReleaseInterface, hr *helmfluxv1.HelmRelease, valuesChecksum string) error {

firstTry := true
err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
if !firstTry {
var getErr error
hr, getErr = client.Get(hr.Name, metav1.GetOptions{})
if getErr != nil {
return getErr
}
}

if valuesChecksum == "" || hr.Status.ValuesChecksum == valuesChecksum {
return
}

cHr := hr.DeepCopy()
cHr.Status.ValuesChecksum = valuesChecksum

_, err = client.UpdateStatus(cHr)
firstTry = false
return
})
return err
}

// SetObservedGeneration updates the observed generation status of the
// HelmRelease to the given generation.
func SetObservedGeneration(client v1client.HelmReleaseInterface, hr *helmfluxv1.HelmRelease, generation int64) error {
Expand Down Expand Up @@ -238,7 +180,7 @@ func HasSynced(hr helmfluxv1.HelmRelease) bool {

// HasRolledBack returns if the current generation of the HelmRelease
// has been rolled back.
func HasRolledBack(hr helmfluxv1.HelmRelease, revision string) bool {
func HasRolledBack(hr helmfluxv1.HelmRelease) bool {
if !HasSynced(hr) {
return false
}
Expand All @@ -248,16 +190,5 @@ func HasRolledBack(hr helmfluxv1.HelmRelease, revision string) bool {
return false
}

chartFetched := GetCondition(hr.Status, helmfluxv1.HelmReleaseChartFetched)
if chartFetched != nil {
// NB: as two successful state updates can happen right after
// each other, on which we both want to act, we _must_ compare
// the update timestamps as the transition timestamp will only
// change on a status shift.
if chartFetched.Status == v1.ConditionTrue && rolledBack.LastUpdateTime.Before(&chartFetched.LastUpdateTime) {
return hr.Status.PrevRevision == revision
}
}

return rolledBack.Status == v1.ConditionTrue
}

0 comments on commit c695af2

Please sign in to comment.