Skip to content

Commit

Permalink
fix: Avoid failure on unchanged provider version
Browse files Browse the repository at this point in the history
In the current implementation, when a provider version remains the same
during subsequent reconciliations, the operator encounters an error with
the message "failed getting clusterctl Provider version," leading to a
degraded status.

This PR addresses this issue by adding a check to determine whether
the provider version has changed or not.
  • Loading branch information
Fedosin committed May 15, 2023
1 parent faab547 commit f620843
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 9 deletions.
22 changes: 21 additions & 1 deletion internal/controller/genericprovider_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ func TestUpgradeDowngradeProvider(t *testing.T) {
namespace string
newVersion string
}{
{
name: "same provider version",
namespace: "test-core-provider",
newVersion: "v0.4.2",
},
{
name: "upgrade provider version",
namespace: "test-core-provider",
Expand Down Expand Up @@ -288,12 +293,23 @@ func TestUpgradeDowngradeProvider(t *testing.T) {
}, timeout).Should(BeEquivalentTo(true))

// creating another configmap with another version
g.Expect(env.CreateAndWait(ctx, dummyConfigMap(tc.namespace, tc.newVersion))).To(Succeed())
if tc.newVersion != testCurrentVersion {
g.Expect(env.CreateAndWait(ctx, dummyConfigMap(tc.namespace, tc.newVersion))).To(Succeed())
}

// Change provider version
providerSpec := provider.GetSpec()
providerSpec.Version = tc.newVersion
provider.SetSpec(providerSpec)

// Set label (needed to start a reconciliation of the provider)
labels := provider.GetLabels()
if labels == nil {
labels = map[string]string{}
}
labels["provider-version"] = tc.newVersion
provider.SetLabels(labels)

g.Expect(env.Client.Update(ctx, provider.GetObject())).To(Succeed())

g.Eventually(func() bool {
Expand All @@ -305,6 +321,10 @@ func TestUpgradeDowngradeProvider(t *testing.T) {
return false
}

if provider.GetLabels()["provider-version"] != tc.newVersion {
return false
}

for _, cond := range provider.GetStatus().Conditions {
if cond.Type == operatorv1.PreflightCheckCondition {
t.Log(t.Name(), provider.GetName(), cond)
Expand Down
31 changes: 23 additions & 8 deletions internal/controller/phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,21 +334,26 @@ func (p *phaseReconciler) fetch(ctx context.Context) (reconcile.Result, error) {
func (p *phaseReconciler) preInstall(ctx context.Context) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)

needPreDelete, err := p.updateRequiresPreDeletion()
if err != nil || !needPreDelete {
return reconcile.Result{}, wrapPhaseError(err, "failed getting clusterctl Provider", operatorv1.ProviderInstalledCondition)
needPreDelete, err := p.versionChanged()
if err != nil {
return reconcile.Result{}, wrapPhaseError(err, "failed getting clusterctl Provider version", operatorv1.ProviderInstalledCondition)
}

// we need to delete existing components only if their version changes.
if !needPreDelete {
return reconcile.Result{}, nil
}

log.Info("Upgrade detected, deleting existing components")

return p.delete(ctx)
}

// updateRequiresPreDeletion try to get installed version from provider status and decide if it's an upgrade.
func (s *phaseReconciler) updateRequiresPreDeletion() (bool, error) {
// versionChanged try to get installed version from provider status and decide if it has changed.
func (s *phaseReconciler) versionChanged() (bool, error) {
installedVersion := s.provider.GetStatus().InstalledVersion
if installedVersion == nil {
return false, nil
return true, nil
}

currentVersion, err := versionutil.ParseSemantic(*installedVersion)
Expand All @@ -362,15 +367,25 @@ func (s *phaseReconciler) updateRequiresPreDeletion() (bool, error) {
}

// we need to delete installed components if versions are different
needPreDelete := res != 0
versionChanged := res != 0

return needPreDelete, nil
return versionChanged, nil
}

// install installs the provider components using clusterctl library.
func (p *phaseReconciler) install(ctx context.Context) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)

versionChanged, err := p.versionChanged()
if err != nil {
return reconcile.Result{}, wrapPhaseError(err, "failed getting clusterctl Provider version", operatorv1.ProviderInstalledCondition)
}

// skip installation if the version hasn't changed.
if !versionChanged {
return reconcile.Result{}, nil
}

clusterClient := p.newClusterClient()

log.Info("Installing provider")
Expand Down

0 comments on commit f620843

Please sign in to comment.