release: record rolled back chart revision (#213)

release: record rolled back chart revision

pkg/apis/helm.fluxcd.io/v1/types.go

@@ -18,7 +18,7 @@ import (
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
-// FluxHelmRelease represents custom resource associated with a Helm Chart
+// HelmRelease represents custom resource associated with a Helm Chart
 type HelmRelease struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata"`
@@ -241,6 +241,11 @@ type HelmReleaseStatus struct {
 	// +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
@@ -302,7 +307,7 @@ func (in *HelmValues) DeepCopyInto(out *HelmValues) {
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
-// HelmReleaseList is a list of FluxHelmRelease resources
+// HelmReleaseList is a list of HelmRelease resources
 type HelmReleaseList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata"`

pkg/operator/operator.go

@@ -2,7 +2,6 @@ package operator
 
 import (
 	"fmt"
-	"github.com/fluxcd/helm-operator/pkg/release"
 	"sync"
 	"time"
 
@@ -24,7 +23,7 @@ import (
 	hrv1 "github.com/fluxcd/helm-operator/pkg/client/informers/externalversions/helm.fluxcd.io/v1"
 	iflister "github.com/fluxcd/helm-operator/pkg/client/listers/helm.fluxcd.io/v1"
 	"github.com/fluxcd/helm-operator/pkg/helm"
-	"github.com/fluxcd/helm-operator/pkg/status"
+	"github.com/fluxcd/helm-operator/pkg/release"
 )
 
 const (
@@ -101,8 +100,7 @@ func New(
 	// ----- EVENT HANDLERS for HelmRelease resources change ---------
 	hrInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(new interface{}) {
-			hr, ok := checkCustomResourceType(controller.logger, new)
-			if ok && !status.HasRolledBack(hr) {
+			if _, ok := checkCustomResourceType(controller.logger, new); ok {
 				controller.enqueueJob(new)
 			}
 		},

pkg/release/release.go

@@ -181,7 +181,7 @@ func (r *Release) Sync(client helm.Client, hr *v1.HelmRelease) (rHr *v1.HelmRele
 	}
 	defer status.SetValuesChecksum(r.helmReleaseClient.HelmReleases(hr.Namespace), hr, composedValues.Checksum())
 
-	if ok, err := shouldSync(logger, client, hr, curRel, chartPath, composedValues, r.config.LogDiffs); !ok {
+	if ok, err := shouldSync(logger, client, hr, curRel, chartPath, revision, composedValues, r.config.LogDiffs); !ok {
 		if err != nil {
 			_ = status.SetCondition(r.helmReleaseClient.HelmReleases(hr.Namespace), hr, status.NewCondition(
 				v1.HelmReleaseReleased, corev1.ConditionFalse, failReason, err.Error()))
@@ -270,6 +270,7 @@ 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.
@@ -305,7 +306,7 @@ func (r *Release) Uninstall(client helm.Client, hr *v1.HelmRelease) {
 // 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 string, values helm.Values, logDiffs bool) (bool, error) {
+	chartPath, revision string, values helm.Values, logDiffs bool) (bool, error) {
 
 	if curRel == nil {
 		logger.Log("info", "no existing release", "action", "install")
@@ -323,7 +324,7 @@ func shouldSync(logger log.Logger, client helm.Client, hr *v1.HelmRelease, curRe
 		return false, nil
 	}
 
-	if status.HasRolledBack(*hr) {
+	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

pkg/status/status.go

@@ -117,8 +117,8 @@ func SetReleaseStatus(client v1client.HelmReleaseInterface, hr *helmfluxv1.HelmR
 	return err
 }
 
-// SetReleaseRevision updates the status of the HelmRelease to the
-// given revision.
+// SetReleaseRevision updates the revision in the status of the HelmRelease
+// to the given revision, and sets the current revision as the previous one.
 func SetReleaseRevision(client v1client.HelmReleaseInterface, hr *helmfluxv1.HelmRelease, revision string) error {
 
 	firstTry := true
@@ -136,6 +136,7 @@ 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)
@@ -145,6 +146,35 @@ 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 {
@@ -208,7 +238,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) bool {
+func HasRolledBack(hr helmfluxv1.HelmRelease, revision string) bool {
 	if !HasSynced(hr) {
 		return false
 	}
@@ -224,14 +254,8 @@ func HasRolledBack(hr helmfluxv1.HelmRelease) bool {
 		// 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.
-		// TODO(hidde): in case of a pod restart the last update time
-		// will actually refresh because of the pruned cache;
-		// triggering a rollout that should not happen. A solution for
-		// this may be to look at the revision we also record, as this
-		// will tell us if we already attempted a release for this
-		// revision (but failed to do so).
 		if chartFetched.Status == v1.ConditionTrue && rolledBack.LastUpdateTime.Before(&chartFetched.LastUpdateTime) {
-			return false
+			return hr.Status.PrevRevision == revision
 		}
 	}