From f7af9bb88ae03cb4d3d3e9937035a0a7afad82fb Mon Sep 17 00:00:00 2001 From: Mikhail Fedosin Date: Mon, 8 May 2023 15:05:35 +0200 Subject: [PATCH] feat: support provider downgrades Previously, the operator allowed only installation of new provider versions. Downgrading to a previous version would result in operator failure. This PR introduces support for seamless provider downgrades in addition to upgrades. --- .../genericprovider_controller_test.go | 145 ++++++++++++++++-- internal/controller/phases.go | 9 +- internal/envtest/environment.go | 27 ++++ 3 files changed, 169 insertions(+), 12 deletions(-) diff --git a/internal/controller/genericprovider_controller_test.go b/internal/controller/genericprovider_controller_test.go index 3f2288e12..6b2c9703b 100644 --- a/internal/controller/genericprovider_controller_test.go +++ b/internal/controller/genericprovider_controller_test.go @@ -71,6 +71,8 @@ spec: requests: cpu: 200m ` + + testCurrentVersion = "v0.4.2" ) func insertDummyConfig(provider genericprovider.GenericProvider) { @@ -85,10 +87,10 @@ func insertDummyConfig(provider genericprovider.GenericProvider) { provider.SetSpec(spec) } -func dummyConfigMap(ns string) *corev1.ConfigMap { +func dummyConfigMap(ns, name string) *corev1.ConfigMap { return &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "v0.4.2", + Name: name, Namespace: ns, Labels: map[string]string{ "test": "dummy-config", @@ -118,7 +120,7 @@ func TestReconcilerPreflightConditions(t *testing.T) { }, Spec: operatorv1.CoreProviderSpec{ ProviderSpec: operatorv1.ProviderSpec{ - Version: "v0.4.2", + Version: testCurrentVersion, }, }, }, @@ -136,7 +138,7 @@ func TestReconcilerPreflightConditions(t *testing.T) { }, Spec: operatorv1.CoreProviderSpec{ ProviderSpec: operatorv1.ProviderSpec{ - Version: "v0.4.2", + Version: testCurrentVersion, }, }, Status: operatorv1.CoreProviderStatus{ @@ -158,7 +160,7 @@ func TestReconcilerPreflightConditions(t *testing.T) { }, Spec: operatorv1.ControlPlaneProviderSpec{ ProviderSpec: operatorv1.ProviderSpec{ - Version: "v0.4.2", + Version: testCurrentVersion, }, }, }, @@ -171,10 +173,10 @@ func TestReconcilerPreflightConditions(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - t.Log("creating namespace", tc.namespace) - namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: tc.namespace}} - g.Expect(env.CreateAndWait(ctx, namespace)).To(Succeed()) - g.Expect(env.CreateAndWait(ctx, dummyConfigMap(tc.namespace))).To(Succeed()) + t.Log("Ensure namespace exists", tc.namespace) + g.Expect(env.EnsureNamespaceExists(ctx, tc.namespace)).To(Succeed()) + + g.Expect(env.CreateAndWait(ctx, dummyConfigMap(tc.namespace, testCurrentVersion))).To(Succeed()) for _, p := range tc.providers { insertDummyConfig(p) @@ -206,6 +208,131 @@ func TestReconcilerPreflightConditions(t *testing.T) { for _, p := range tc.providers { objs = append(objs, p.GetObject()) } + + objs = append(objs, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: testCurrentVersion, + Namespace: tc.namespace, + }, + }) + + g.Expect(env.CleanupAndWait(ctx, objs...)).To(Succeed()) + }) + } +} + +func TestUpgradeDowngradeProvider(t *testing.T) { + testCases := []struct { + name string + namespace string + newVersion string + }{ + { + name: "upgrade provider version", + namespace: "test-core-provider", + newVersion: "v0.4.3", + }, + { + name: "downgrade provider version", + namespace: "test-core-provider", + newVersion: "v0.4.1", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + provider := &genericprovider.CoreProviderWrapper{ + CoreProvider: &operatorv1.CoreProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-api", + }, + Spec: operatorv1.CoreProviderSpec{ + ProviderSpec: operatorv1.ProviderSpec{ + Version: testCurrentVersion, + }, + }, + }, + } + + t.Log("Ensure namespace exists", tc.namespace) + g.Expect(env.EnsureNamespaceExists(ctx, tc.namespace)).To(Succeed()) + + g.Expect(env.CreateAndWait(ctx, dummyConfigMap(tc.namespace, testCurrentVersion))).To(Succeed()) + + insertDummyConfig(provider) + provider.SetNamespace(tc.namespace) + t.Log("creating test provider", provider.GetName()) + g.Expect(env.CreateAndWait(ctx, provider.GetObject())).To(Succeed()) + + g.Eventually(func() bool { + if err := env.Get(ctx, client.ObjectKeyFromObject(provider.GetObject()), provider.GetObject()); err != nil { + return false + } + + if provider.GetStatus().InstalledVersion == nil || *provider.GetStatus().InstalledVersion != testCurrentVersion { + return false + } + + for _, cond := range provider.GetStatus().Conditions { + if cond.Type == operatorv1.PreflightCheckCondition { + t.Log(t.Name(), provider.GetName(), cond) + if cond.Status == corev1.ConditionTrue { + return true + } + } + } + + return false + }, timeout).Should(BeEquivalentTo(true)) + + // creating another configmap with another version + 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) + g.Expect(env.Client.Update(ctx, provider.GetObject())).To(Succeed()) + + g.Eventually(func() bool { + if err := env.Get(ctx, client.ObjectKeyFromObject(provider.GetObject()), provider.GetObject()); err != nil { + return false + } + + if provider.GetStatus().InstalledVersion == nil || *provider.GetStatus().InstalledVersion != tc.newVersion { + return false + } + + for _, cond := range provider.GetStatus().Conditions { + if cond.Type == operatorv1.PreflightCheckCondition { + t.Log(t.Name(), provider.GetName(), cond) + if cond.Status == corev1.ConditionTrue { + return true + } + } + } + + return false + }, timeout).Should(BeEquivalentTo(true)) + + // Clean up + objs := []client.Object{provider.GetObject()} + objs = append(objs, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: testCurrentVersion, + Namespace: tc.namespace, + }, + }) + + objs = append(objs, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.newVersion, + Namespace: tc.namespace, + }, + }) + g.Expect(env.CleanupAndWait(ctx, objs...)).To(Succeed()) }) } diff --git a/internal/controller/phases.go b/internal/controller/phases.go index e7fa6cb50..ab92db532 100644 --- a/internal/controller/phases.go +++ b/internal/controller/phases.go @@ -351,17 +351,20 @@ func (s *phaseReconciler) updateRequiresPreDeletion() (bool, error) { return false, nil } - nextVersion, err := versionutil.ParseSemantic(s.components.Version()) + currentVersion, err := versionutil.ParseSemantic(*installedVersion) if err != nil { return false, err } - currentVersion, err := versionutil.ParseSemantic(*installedVersion) + res, err := currentVersion.Compare(s.components.Version()) if err != nil { return false, err } - return currentVersion.LessThan(nextVersion), nil + // we need to delete installed components if versions are different + needPreDelete := res != 0 + + return needPreDelete, nil } // install installs the provider components using clusterctl library. diff --git a/internal/envtest/environment.go b/internal/envtest/environment.go index 28908f0b0..27ac5c94c 100644 --- a/internal/envtest/environment.go +++ b/internal/envtest/environment.go @@ -293,6 +293,33 @@ func (e *Environment) CreateNamespace(ctx context.Context, generateName string) return ns, nil } +func (e *Environment) EnsureNamespaceExists(ctx context.Context, namespace string) error { + // Check if the namespace exists + ns := &corev1.Namespace{} + + err := e.Client.Get(ctx, client.ObjectKey{Name: namespace}, ns) + if err == nil { + return nil + } + + if !apierrors.IsNotFound(err) { + return fmt.Errorf("unexpected error during namespace checking: %w", err) + } + + // Create the namespace if it doesn't exist + newNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + + if err := e.Client.Create(ctx, newNamespace); err != nil { + return fmt.Errorf("unable to create namespace %s: %w", namespace, err) + } + + return nil +} + func getFilePathToClusterctlCRDs(root string) string { if clusterctlCRDPath := os.Getenv("CLUSTERCTL_CRD_PATH"); clusterctlCRDPath != "" { return clusterctlCRDPath