Skip to content

Commit 73b36a6

Browse files
joelanfordjoelanford
authored
pkg/reconciler: force upgrade if release is failed or superseded (#81)
1 parent 128321d commit 73b36a6

File tree

3 files changed

+78
-23
lines changed

3 files changed

+78
-23
lines changed

‎Makefile

+2-1
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ all: test lint build
3131
# Run tests
3232
.PHONY: test
3333
export KUBEBUILDER_ASSETS := $(TOOLS_BIN_DIR)
34+
TESTPKG ?= ./...
3435
CR_VERSION=$(shell go list -m sigs.k8s.io/controller-runtime | cut -d" " -f2 | sed 's/^v//')
3536
test:
36-
fetch envtest $(CR_VERSION) && go test -race -covermode atomic -coverprofile cover.out ./...
37+
fetch envtest $(CR_VERSION) && go test -race -covermode atomic -coverprofile cover.out $(TESTPKG)
3738

3839
# Build manager binary
3940
.PHONY: build

‎pkg/reconciler/reconciler.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ func (r *Reconciler) handleDeletion(ctx context.Context, actionClient helmclient
582582
}
583583

584584
func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj metav1.Object, vals map[string]interface{}) (*release.Release, helmReleaseState, error) {
585-
deployedRelease, err := client.Get(obj.GetName())
585+
currentRelease, err := client.Get(obj.GetName())
586586
if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
587587
return nil, stateError, err
588588
}
@@ -603,12 +603,14 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta
603603
})
604604
specRelease, err := client.Upgrade(obj.GetName(), obj.GetNamespace(), r.chrt, vals, opts...)
605605
if err != nil {
606-
return deployedRelease, stateError, err
606+
return currentRelease, stateError, err
607607
}
608-
if specRelease.Manifest != deployedRelease.Manifest {
609-
return deployedRelease, stateNeedsUpgrade, nil
608+
if specRelease.Manifest != currentRelease.Manifest ||
609+
currentRelease.Info.Status == release.StatusFailed ||
610+
currentRelease.Info.Status == release.StatusSuperseded {
611+
return currentRelease, stateNeedsUpgrade, nil
610612
}
611-
return deployedRelease, stateUnchanged, nil
613+
return currentRelease, stateUnchanged, nil
612614
}
613615

614616
func (r *Reconciler) doInstall(actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, vals map[string]interface{}, log logr.Logger) (*release.Release, error) {

‎pkg/reconciler/reconciler_test.go

+69-17
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/go-logr/logr/testing"
2929
. "github.com/onsi/ginkgo"
3030
. "github.com/onsi/gomega"
31+
"helm.sh/helm/v3/pkg/action"
3132
"helm.sh/helm/v3/pkg/chart"
3233
"helm.sh/helm/v3/pkg/chartutil"
3334
"helm.sh/helm/v3/pkg/release"
@@ -450,7 +451,7 @@ var _ = Describe("Reconciler", func() {
450451
Expect(mgr.GetClient().Create(ctx, obj)).To(Succeed())
451452
})
452453

453-
When("requested CR release is not installed", func() {
454+
When("requested CR release is not present", func() {
454455
When("action client getter is not working", func() {
455456
It("returns an error getting the action client", func() {
456457
acgErr := errors.New("broken action client getter: error getting action client")
@@ -682,9 +683,9 @@ var _ = Describe("Reconciler", func() {
682683
})
683684
})
684685
})
685-
When("requested CR release is installed", func() {
686+
When("requested CR release is present", func() {
686687
var (
687-
installedRelease *release.Release
688+
currentRelease *release.Release
688689
)
689690
BeforeEach(func() {
690691
// Reconcile once to get the release installed and finalizers added
@@ -693,7 +694,7 @@ var _ = Describe("Reconciler", func() {
693694
Expect(res).To(Equal(reconcile.Result{}))
694695
Expect(err).To(BeNil())
695696

696-
installedRelease, err = ac.Get(obj.GetName())
697+
currentRelease, err = ac.Get(obj.GetName())
697698
Expect(err).To(BeNil())
698699
})
699700
When("action client getter is not working", func() {
@@ -801,27 +802,78 @@ var _ = Describe("Reconciler", func() {
801802
Expect(c.Reason).To(Equal(conditions.ReasonErrorGettingValues))
802803
Expect(c.Message).To(ContainSubstring("error parsing index"))
803804

804-
Expect(objStat.Status.DeployedRelease.Name).To(Equal(installedRelease.Name))
805-
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal(installedRelease.Manifest))
805+
Expect(objStat.Status.DeployedRelease.Name).To(Equal(currentRelease.Name))
806+
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal(currentRelease.Manifest))
806807
})
807808

808809
By("verifying the uninstall finalizer is not present on the CR", func() {
809810
Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeTrue())
810811
})
811812
})
812813
})
813-
When("all preconditions are met", func() {
814+
When("requested CR release is not deployed", func() {
815+
var actionConf *action.Configuration
816+
BeforeEach(func() {
817+
By("getting the current release and config", func() {
818+
var err error
819+
acg := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), nil)
820+
actionConf, err = acg.ActionConfigFor(obj)
821+
Expect(err).To(BeNil())
822+
})
823+
})
824+
When("state is Failed", func() {
825+
BeforeEach(func() {
826+
currentRelease.Info.Status = release.StatusFailed
827+
Expect(actionConf.Releases.Update(currentRelease)).To(Succeed())
828+
})
829+
It("upgrades the release", func() {
830+
By("successfully reconciling a request", func() {
831+
res, err := r.Reconcile(ctx, req)
832+
Expect(res).To(Equal(reconcile.Result{}))
833+
Expect(err).To(BeNil())
834+
})
835+
By("verifying the release", func() {
836+
rel, err := ac.Get(obj.GetName())
837+
Expect(err).To(BeNil())
838+
Expect(rel).NotTo(BeNil())
839+
Expect(rel.Version).To(Equal(2))
840+
verifyRelease(ctx, mgr.GetAPIReader(), obj.GetNamespace(), rel)
841+
})
842+
})
843+
})
844+
When("state is Superseded", func() {
845+
BeforeEach(func() {
846+
currentRelease.Info.Status = release.StatusSuperseded
847+
Expect(actionConf.Releases.Update(currentRelease)).To(Succeed())
848+
})
849+
It("upgrades the release", func() {
850+
By("successfully reconciling a request", func() {
851+
res, err := r.Reconcile(ctx, req)
852+
Expect(res).To(Equal(reconcile.Result{}))
853+
Expect(err).To(BeNil())
854+
})
855+
By("verifying the release", func() {
856+
rel, err := ac.Get(obj.GetName())
857+
Expect(err).To(BeNil())
858+
Expect(rel).NotTo(BeNil())
859+
Expect(rel.Version).To(Equal(2))
860+
verifyRelease(ctx, mgr.GetAPIReader(), obj.GetNamespace(), rel)
861+
})
862+
})
863+
})
864+
})
865+
When("state is Deployed", func() {
814866
When("upgrade fails", func() {
815867
BeforeEach(func() {
816868
ac := helmfake.NewActionClient()
817869
ac.HandleGet = func() (*release.Release, error) {
818-
return &release.Release{Name: "test", Version: 1, Manifest: "version: 1"}, nil
870+
return &release.Release{Name: "test", Version: 1, Manifest: "manifest: 1"}, nil
819871
}
820872
firstRun := true
821873
ac.HandleUpgrade = func() (*release.Release, error) {
822874
if firstRun {
823875
firstRun = false
824-
return &release.Release{Name: "test", Version: 1, Manifest: "version: 2"}, nil
876+
return &release.Release{Name: "test", Version: 1, Manifest: "manifest: 2"}, nil
825877
}
826878
return nil, errors.New("upgrade failed: foobar")
827879
}
@@ -846,7 +898,7 @@ var _ = Describe("Reconciler", func() {
846898
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeDeployed)).To(BeTrue())
847899
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeReleaseFailed)).To(BeTrue())
848900
Expect(objStat.Status.DeployedRelease.Name).To(Equal("test"))
849-
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal("version: 1"))
901+
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal("manifest: 1"))
850902

851903
c := objStat.Status.Conditions.GetCondition(conditions.TypeReleaseFailed)
852904
Expect(c).NotTo(BeNil())
@@ -921,10 +973,10 @@ var _ = Describe("Reconciler", func() {
921973
BeforeEach(func() {
922974
ac := helmfake.NewActionClient()
923975
ac.HandleGet = func() (*release.Release, error) {
924-
return &release.Release{Name: "test", Version: 1, Manifest: "version: 1"}, nil
976+
return &release.Release{Name: "test", Version: 1, Manifest: "manifest: 1", Info: &release.Info{Status: release.StatusDeployed}}, nil
925977
}
926978
ac.HandleUpgrade = func() (*release.Release, error) {
927-
return &release.Release{Name: "test", Version: 1, Manifest: "version: 1"}, nil
979+
return &release.Release{Name: "test", Version: 2, Manifest: "manifest: 1", Info: &release.Info{Status: release.StatusDeployed}}, nil
928980
}
929981
ac.HandleReconcile = func() error {
930982
return errors.New("reconciliation failed: foobar")
@@ -950,7 +1002,7 @@ var _ = Describe("Reconciler", func() {
9501002
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeDeployed)).To(BeTrue())
9511003
Expect(objStat.Status.Conditions.IsFalseFor(conditions.TypeReleaseFailed)).To(BeTrue())
9521004
Expect(objStat.Status.DeployedRelease.Name).To(Equal("test"))
953-
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal("version: 1"))
1005+
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal("manifest: 1"))
9541006

9551007
c := objStat.Status.Conditions.GetCondition(conditions.TypeIrreconcilable)
9561008
Expect(c).NotTo(BeNil())
@@ -970,7 +1022,7 @@ var _ = Describe("Reconciler", func() {
9701022
err error
9711023
)
9721024
By("changing the release resources", func() {
973-
for _, resource := range manifestToObjects(installedRelease.Manifest) {
1025+
for _, resource := range manifestToObjects(currentRelease.Manifest) {
9741026
key := client.ObjectKeyFromObject(resource)
9751027

9761028
u := &unstructured.Unstructured{}
@@ -1032,7 +1084,7 @@ var _ = Describe("Reconciler", func() {
10321084
BeforeEach(func() {
10331085
ac := helmfake.NewActionClient()
10341086
ac.HandleGet = func() (*release.Release, error) {
1035-
return &release.Release{Name: "test", Version: 1, Manifest: "version: 1"}, nil
1087+
return &release.Release{Name: "test", Version: 1, Manifest: "manifest: 1"}, nil
10361088
}
10371089
ac.HandleUninstall = func() (*release.UninstallReleaseResponse, error) {
10381090
return nil, errors.New("uninstall failed: foobar")
@@ -1062,7 +1114,7 @@ var _ = Describe("Reconciler", func() {
10621114
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeDeployed)).To(BeTrue())
10631115
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeReleaseFailed)).To(BeTrue())
10641116
Expect(objStat.Status.DeployedRelease.Name).To(Equal("test"))
1065-
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal("version: 1"))
1117+
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal("manifest: 1"))
10661118

10671119
c := objStat.Status.Conditions.GetCondition(conditions.TypeReleaseFailed)
10681120
Expect(c).NotTo(BeNil())
@@ -1093,7 +1145,7 @@ var _ = Describe("Reconciler", func() {
10931145
})
10941146

10951147
By("verifying the release is uninstalled", func() {
1096-
verifyNoRelease(ctx, mgr.GetClient(), obj.GetNamespace(), obj.GetName(), installedRelease)
1148+
verifyNoRelease(ctx, mgr.GetClient(), obj.GetNamespace(), obj.GetName(), currentRelease)
10971149
})
10981150

10991151
By("ensuring the finalizer is removed and the CR is deleted", func() {

0 commit comments

Comments
 (0)