Merge pull request #396 from Danil-Grigorev/test-capi-provider-operat…

…ions

🌱 Add tests for capi provider edit operations

api/v1alpha1/capiprovider_wrapper.go

@@ -78,3 +78,12 @@ func (b *CAPIProvider) SetVariables(v map[string]string) {
 func (b *CAPIProvider) SetPhase(p Phase) {
 	b.Status.Phase = p
 }
+
+// ProviderName is a name for the managed CAPI provider resource.
+func (b *CAPIProvider) ProviderName() string {
+	if b.Spec.Name != "" {
+		return b.Spec.Name
+	}
+
+	return b.Name
+}

internal/controllers/capiprovider_controller.go

@@ -68,11 +68,11 @@ func (r *CAPIProviderReconciler) reconcileNormal(ctx context.Context, capiProvid
 }
 
 func (r *CAPIProviderReconciler) sync(ctx context.Context, capiProvider *turtlesv1.CAPIProvider) (_ ctrl.Result, err error) {
-	s := sync.List{
+	s := sync.NewList(
 		sync.NewProviderSync(r.Client, capiProvider),
 		sync.NewSecretSync(r.Client, capiProvider),
 		sync.NewSecretMapperSync(ctx, r.Client, capiProvider),
-	}
+	)
 
 	if err := s.Sync(ctx); client.IgnoreNotFound(err) != nil {
 		return ctrl.Result{}, err

internal/controllers/capiprovider_controller_test.go

@@ -0,0 +1,159 @@
+/*
+Copyright © 2023 - 2024 SUSE LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controllers
+
+import (
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	turtlesv1 "github.com/rancher-sandbox/rancher-turtles/api/v1alpha1"
+	"github.com/rancher-sandbox/rancher-turtles/internal/sync"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha2"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	. "sigs.k8s.io/controller-runtime/pkg/envtest/komega"
+)
+
+func objectFromKey(key client.ObjectKey, obj client.Object) client.Object {
+	obj.SetName(key.Name)
+	obj.SetNamespace(key.Namespace)
+	return obj
+}
+
+var _ = Describe("Reconcile CAPIProvider", func() {
+	var (
+		ns *corev1.Namespace
+	)
+
+	BeforeEach(func() {
+		var err error
+
+		ns, err = testEnv.CreateNamespace(ctx, "capiprovider")
+		Expect(err).ToNot(HaveOccurred())
+		_ = ns
+
+		r := &CAPIProviderReconciler{
+			Client: testEnv.GetClient(),
+			Scheme: testEnv.GetScheme(),
+		}
+
+		Expect(r.SetupWithManager(ctx, testEnv.Manager)).ToNot(HaveOccurred())
+	})
+
+	It("Should create infrastructure docker provider and secret", func() {
+		provider := &turtlesv1.CAPIProvider{ObjectMeta: metav1.ObjectMeta{
+			Name:      "docker",
+			Namespace: ns.Name,
+		}, Spec: turtlesv1.CAPIProviderSpec{
+			Type: turtlesv1.Infrastructure,
+		}}
+		Expect(cl.Create(ctx, provider)).ToNot(HaveOccurred())
+
+		dockerProvider := objectFromKey(client.ObjectKeyFromObject(provider), &operatorv1.InfrastructureProvider{})
+		dockerSecret := objectFromKey(client.ObjectKeyFromObject(provider), &corev1.Secret{})
+		Eventually(Object(dockerProvider)).ShouldNot(BeNil())
+		Eventually(Object(dockerSecret)).Should(HaveField("Data", Equal(map[string][]byte{
+			"CLUSTER_TOPOLOGY":         []byte("true"),
+			"EXP_CLUSTER_RESOURCE_SET": []byte("true"),
+			"EXP_MACHINE_POOL":         []byte("true"),
+		})))
+	})
+
+	It("Should update infrastructure docker provider version and secret content from CAPI Provider change", func() {
+		provider := &turtlesv1.CAPIProvider{ObjectMeta: metav1.ObjectMeta{
+			Name:      "docker",
+			Namespace: ns.Name,
+		}, Spec: turtlesv1.CAPIProviderSpec{
+			Type: turtlesv1.Infrastructure,
+		}}
+		Expect(cl.Create(ctx, provider)).ToNot(HaveOccurred())
+
+		dockerProvider := objectFromKey(client.ObjectKeyFromObject(provider), &operatorv1.InfrastructureProvider{})
+		dockerSecret := objectFromKey(client.ObjectKeyFromObject(provider), &corev1.Secret{})
+		Eventually(Object(dockerProvider)).ShouldNot(BeNil())
+		Eventually(Object(dockerSecret)).ShouldNot(BeNil())
+
+		Eventually(Update(provider, func() {
+			provider.Spec.Version = "v1.2.3"
+			provider.Spec.Variables = map[string]string{
+				"other": "var",
+			}
+		})).Should(Succeed())
+
+		Eventually(Object(dockerProvider)).Should(HaveField("Spec.Version", Equal("v1.2.3")))
+		Eventually(Object(dockerSecret)).Should(HaveField("Data", Equal(map[string][]byte{
+			"other":                    []byte("var"),
+			"CLUSTER_TOPOLOGY":         []byte("true"),
+			"EXP_CLUSTER_RESOURCE_SET": []byte("true"),
+			"EXP_MACHINE_POOL":         []byte("true"),
+		})))
+	})
+
+	It("Should update infrastructure digitalocean provider features and convert rancher credentials secret on CAPI Provider change", func() {
+		provider := &turtlesv1.CAPIProvider{ObjectMeta: metav1.ObjectMeta{
+			Name:      "digitalocean",
+			Namespace: ns.Name,
+		}, Spec: turtlesv1.CAPIProviderSpec{
+			Type: turtlesv1.Infrastructure,
+		}}
+		Expect(cl.Create(ctx, provider)).ToNot(HaveOccurred())
+
+		doSecret := objectFromKey(client.ObjectKeyFromObject(provider), &corev1.Secret{})
+		Eventually(testEnv.GetAs(provider, &operatorv1.InfrastructureProvider{})).ShouldNot(BeNil())
+		Eventually(testEnv.GetAs(provider, doSecret)).ShouldNot(BeNil())
+
+		Expect(cl.Create(ctx, &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: sync.RancherCredentialsNamespace,
+			},
+		})).To(Succeed())
+
+		Expect(cl.Create(ctx, &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:         "cc",
+				GenerateName: "cc-",
+				Namespace:    sync.RancherCredentialsNamespace,
+				Annotations: map[string]string{
+					sync.NameAnnotation:       "test-rancher-secret",
+					sync.DriverNameAnnotation: "digitalocean",
+				},
+			},
+			StringData: map[string]string{
+				"digitaloceancredentialConfig-accessToken": "token",
+			},
+		})).To(Succeed())
+
+		Eventually(Update(provider, func() {
+			provider.Spec.Features = &turtlesv1.Features{MachinePool: true}
+			provider.Spec.Credentials = &turtlesv1.Credentials{
+				RancherCloudCredential: "test-rancher-secret",
+			}
+		})).Should(Succeed())
+
+		Eventually(Object(doSecret), 10*time.Second).Should(HaveField("Data", Equal(map[string][]byte{
+			"EXP_MACHINE_POOL":          []byte("true"),
+			"CLUSTER_TOPOLOGY":          []byte("false"),
+			"EXP_CLUSTER_RESOURCE_SET":  []byte("false"),
+			"DIGITALOCEAN_ACCESS_TOKEN": []byte("token"),
+			"DO_B64ENCODED_CREDENTIALS": []byte("dG9rZW4="),
+		})))
+
+	})
+})

internal/controllers/import_controller_test.go

@@ -129,14 +129,16 @@ var _ = Describe("reconcile CAPI Cluster", func() {
 	It("should reconcile a CAPI cluster when control plane not ready", func() {
 		Expect(cl.Create(ctx, capiCluster)).To(Succeed())
 
-		res, err := r.Reconcile(ctx, reconcile.Request{
-			NamespacedName: types.NamespacedName{
-				Namespace: capiCluster.Namespace,
-				Name:      capiCluster.Name,
-			},
+		Eventually(func(g Gomega) {
+			res, err := r.Reconcile(ctx, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Namespace: capiCluster.Namespace,
+					Name:      capiCluster.Name,
+				},
+			})
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(res.RequeueAfter).To(Equal(defaultRequeueDuration))
 		})
-		Expect(err).ToNot(HaveOccurred())
-		Expect(res.RequeueAfter).To(Equal(defaultRequeueDuration))
 	})
 
 	It("should reconcile a CAPI cluster when rancher cluster doesn't exist", func() {

internal/controllers/import_controller_v3.go

@@ -97,12 +97,10 @@ func (r *CAPIImportManagementV3Reconciler) SetupWithManager(ctx context.Context,
 	}
 
 	ns := &corev1.Namespace{}
-	err = c.Watch(
+	if err = c.Watch(
 		source.Kind(mgr.GetCache(), ns),
 		handler.EnqueueRequestsFromMapFunc(namespaceToCapiClusters(ctx, capiPredicates, r.Client)),
-	)
-
-	if err != nil {
+	); err != nil {
 		return fmt.Errorf("adding watch for namespaces: %w", err)
 	}
 

internal/sync/core.go

@@ -22,6 +22,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/client-go/util/retry"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -65,7 +66,9 @@ func (s *DefaultSynchronizer) Apply(ctx context.Context, reterr *error) {
 
 	setOwnerReference(s.Source, s.Destination)
 
-	if err := Patch(ctx, s.client, s.Destination); err != nil {
+	if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		return Patch(ctx, s.client, s.Destination)
+	}); err != nil {
 		*reterr = kerrors.NewAggregate([]error{*reterr, err})
 		log.Error(*reterr, fmt.Sprintf("Unable to patch object: %s", *reterr))
 	}

internal/sync/interface.go

@@ -18,6 +18,7 @@ package sync
 
 import (
 	"context"
+	"slices"
 
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -36,15 +37,18 @@ type Sync interface {
 // List contains a list of syncers to apply the syncing logic.
 type List []Sync
 
+// NewList creates a new list of only initialized Sync handlers.
+func NewList(syncHandlers ...Sync) List {
+	return slices.DeleteFunc(syncHandlers, func(s Sync) bool {
+		return s == nil
+	})
+}
+
 // Sync applies synchronization logic on all syncers in the list.
 func (s List) Sync(ctx context.Context) error {
 	errors := []error{}
 
 	for _, syncer := range s {
-		if syncer == nil {
-			continue
-		}
-
 		errors = append(errors, syncer.Get(ctx), syncer.Sync(ctx))
 	}
 
@@ -56,10 +60,6 @@ func (s List) Apply(ctx context.Context, reterr *error) {
 	errors := []error{*reterr}
 
 	for _, syncer := range s {
-		if syncer == nil {
-			continue
-		}
-
 		var err error
 
 		syncer.Apply(ctx, &err)

internal/sync/interface_test.go

@@ -60,15 +60,15 @@ var _ = Describe("resource Sync interface", func() {
 		}{
 			{
 				name:     "Nil syncronizer is accepted",
-				list:     sync.List{nil},
+				list:     sync.NewList(nil),
 				expected: false,
 			}, {
 				name:     "All syncronizers is executed",
-				list:     sync.List{&MockSynchronizer{}, &MockSynchronizer{}},
+				list:     sync.NewList(&MockSynchronizer{}, &MockSynchronizer{}),
 				expected: false,
 			}, {
 				name:     "Syncronizer errors are returned",
-				list:     sync.List{&MockSynchronizer{getErr: errors.New("Fail first get"), syncronizerr: errors.New("Fail sync")}, &MockSynchronizer{getErr: errors.New("Fail second get")}},
+				list:     sync.NewList(&MockSynchronizer{getErr: errors.New("Fail first get"), syncronizerr: errors.New("Fail sync")}, &MockSynchronizer{getErr: errors.New("Fail second get")}),
 				err:      "Fail first get, Fail sync, Fail second get",
 				expected: true,
 			},
@@ -92,15 +92,15 @@ var _ = Describe("resource Sync interface", func() {
 		}{
 			{
 				name:     "Nil syncronizer is accepted",
-				list:     sync.List{nil},
+				list:     sync.NewList(nil),
 				expected: false,
 			}, {
 				name:     "All syncronizers is executed",
-				list:     sync.List{&MockSynchronizer{}, &MockSynchronizer{}},
+				list:     sync.NewList(&MockSynchronizer{}, &MockSynchronizer{}),
 				expected: false,
 			}, {
 				name:     "Syncronizer errors are returned",
-				list:     sync.List{&MockSynchronizer{applyErr: errors.New("Fail apply")}, &MockSynchronizer{applyErr: errors.New("Fail second apply")}},
+				list:     sync.NewList(&MockSynchronizer{applyErr: errors.New("Fail apply")}, &MockSynchronizer{applyErr: errors.New("Fail second apply")}),
 				err:      "Fail apply, Fail second apply",
 				expected: true,
 			},

internal/sync/provider_sync.go

@@ -101,6 +101,7 @@ func (s *ProviderSync) Sync(_ context.Context) error {
 // CAPIProvider <- Status.
 func (s *ProviderSync) SyncObjects() {
 	s.Destination.SetSpec(s.Source.GetSpec())
+	s.rolloutInfrastructure()
 	s.Source.SetStatus(s.Destination.GetStatus())
 	s.syncStatus()
 }
@@ -115,14 +116,14 @@ func (s *ProviderSync) syncStatus() {
 		s.Source.SetPhase(turtlesv1.Provisioning)
 	}
 
-	s.rolloutInfrastructure()
+	conditions.MarkTrue(s.Source, turtlesv1.LastAppliedConfigurationTime)
 }
 
 func (s *ProviderSync) rolloutInfrastructure() {
-	now := metav1.NewTime(time.Now().UTC().Truncate(time.Second))
+	now := time.Now()
 	lastApplied := conditions.Get(s.Source, turtlesv1.LastAppliedConfigurationTime)
 
-	if lastApplied != nil && lastApplied.LastTransitionTime.Add(time.Minute).Before(now.Time) {
+	if lastApplied != nil && lastApplied.LastTransitionTime.Add(time.Minute).After(now) {
 		return
 	}
 
@@ -136,6 +137,4 @@ func (s *ProviderSync) rolloutInfrastructure() {
 
 	annotations[AppliedSpecHashAnnotation] = ""
 	s.Destination.SetAnnotations(annotations)
-
-	conditions.MarkTrue(s.Source, turtlesv1.LastAppliedConfigurationTime)
 }