From b34007120db2e6b525e5d634bf27a0b73ba3ec6f Mon Sep 17 00:00:00 2001 From: Amol Deodhar Date: Tue, 23 Apr 2024 08:01:38 -0400 Subject: [PATCH] adapt linodecluster controller tests to new mocktest changes --- cloud/scope/cluster.go | 4 +- controller/linodecluster_controller.go | 1 + controller/linodecluster_controller_test.go | 364 ++++++++------------ 3 files changed, 154 insertions(+), 215 deletions(-) diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 0573bf89c..7769df967 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -72,7 +72,7 @@ func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopePara } return &ClusterScope{ - client: params.Client, + Client: params.Client, Cluster: params.Cluster, LinodeClient: linodeClient, LinodeCluster: params.LinodeCluster, @@ -82,7 +82,7 @@ func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopePara // ClusterScope defines the basic context for an actuator to operate upon. type ClusterScope struct { - client K8sClient + Client K8sClient PatchHelper *patch.Helper LinodeClient LinodeNodeBalancerClient Cluster *clusterv1.Cluster diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 18813b743..1b9763c3e 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -192,6 +192,7 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil { logger.Info("NodeBalancer ID is missing, nothing to do") controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) + r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, "NodeBalancerIDMissing", "NodeBalancer ID is missing, nothing to do") return nil } diff --git a/controller/linodecluster_controller_test.go b/controller/linodecluster_controller_test.go index 2bc495268..c71f3e4a6 100644 --- a/controller/linodecluster_controller_test.go +++ b/controller/linodecluster_controller_test.go @@ -17,24 +17,32 @@ package controller import ( - corev1 "k8s.io/api/core/v1" + "context" + "errors" + + "go.uber.org/mock/gomock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/go-logr/logr" infrav1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/mock" + . "github.com/linode/cluster-api-provider-linode/mock/mocktest" + "github.com/linode/linodego" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -var _ = Describe("lifecycle", Ordered, Label("cluster", "lifecycle"), func() { - var reconciler *LinodeClusterReconciler +var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecycle"), func() { nodebalancerID := 1 controlPlaneEndpointHost := "10.0.0.1" - clusterName := "lifecycle" + controlPlaneEndpointPort := 6443 + clusterName := "cluster-lifecycle" clusterNameSpace := "default" ownerRef := metav1.OwnerReference{ Name: clusterName, @@ -49,177 +57,108 @@ var _ = Describe("lifecycle", Ordered, Label("cluster", "lifecycle"), func() { OwnerReferences: ownerRefs, } - caplCluster := clusterv1.Cluster{ - ObjectMeta: metadata, - Spec: clusterv1.ClusterSpec{ - InfrastructureRef: &corev1.ObjectReference{ - Kind: "LinodeCluster", - Name: clusterName, - Namespace: clusterNameSpace, - }, - ControlPlaneRef: &corev1.ObjectReference{ - Kind: "KubeadmControlPlane", - Name: "lifecycle-control-plane", - Namespace: clusterNameSpace, - }, - }, - } - linodeCluster := infrav1.LinodeCluster{ ObjectMeta: metadata, Spec: infrav1.LinodeClusterSpec{ Region: "us-ord", - Network: infrav1.NetworkSpec{ - NodeBalancerID: &nodebalancerID, - }, - ControlPlaneEndpoint: clusterv1.APIEndpoint{ - Host: controlPlaneEndpointHost, - }, }, } - BeforeEach(func() { - reconciler = &LinodeClusterReconciler{ - Client: k8sClient, - LinodeApiKey: "test-key", - } - }) - - It("should provision a control plane endpoint", func(ctx SpecContext) { - clusterKey := client.ObjectKeyFromObject(&linodeCluster) - Expect(k8sClient.Create(ctx, &caplCluster)).To(Succeed()) - Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed()) - _, err := reconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: clusterKey, - }) - Expect(err).NotTo(HaveOccurred()) - - By("checking ready conditions") - Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) - Expect(linodeCluster.Status.Ready).To(BeTrue()) - Expect(linodeCluster.Status.Conditions).To(HaveLen(1)) - Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition)) - - By("checking nb id") - Expect(linodeCluster.Spec.Network.NodeBalancerID).To(Equal(&nodebalancerID)) - - By("checking controlPlaneEndpoint host") - Expect(linodeCluster.Spec.ControlPlaneEndpoint.Host).To(Equal(controlPlaneEndpointHost)) - }) -}) - -var _ = Describe("no-capl-cluster", Ordered, Label("cluster", "no-capl-cluster"), func() { - var reconciler *LinodeClusterReconciler - nodebalancerID := 1 - controlPlaneEndpointHost := "10.0.0.1" - clusterName := "no-capl-cluster" - clusterNameSpace := "default" - metadata := metav1.ObjectMeta{ - Name: clusterName, - Namespace: clusterNameSpace, + ctlrSuite := NewControllerTestSuite(GinkgoT(), mock.MockLinodeNodeBalancerClient{}) + reconciler := LinodeClusterReconciler{ + Recorder: ctlrSuite.Recorder(), } - linodeCluster := infrav1.LinodeCluster{ - ObjectMeta: metadata, - Spec: infrav1.LinodeClusterSpec{ - Region: "us-ord", - Network: infrav1.NetworkSpec{ - NodeBalancerID: &nodebalancerID, - }, - ControlPlaneEndpoint: clusterv1.APIEndpoint{ - Host: controlPlaneEndpointHost, - }, - }, + cScope := &scope.ClusterScope{ + LinodeCluster: &linodeCluster, } - BeforeEach(func() { - reconciler = &LinodeClusterReconciler{ - Client: k8sClient, - LinodeApiKey: "test-key", - } - }) - - It("should fail reconciliation if no capl cluster exists", func(ctx SpecContext) { - clusterKey := client.ObjectKeyFromObject(&linodeCluster) + BeforeAll(func(ctx SpecContext) { + cScope.Client = k8sClient Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed()) - _, err := reconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: clusterKey, - }) - Expect(err).NotTo(HaveOccurred()) - - By("checking ready conditions") - Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) - Expect(linodeCluster.Status.Ready).To(BeFalseBecause("failed to get Cluster/no-capl-cluster: clusters.cluster.x-k8s.io \"no-capl-cluster\" not found")) }) -}) - -var _ = Describe("no-owner-ref", Ordered, Label("cluster", "no-owner-ref"), func() { - var reconciler *LinodeClusterReconciler - nodebalancerID := 1 - controlPlaneEndpointHost := "10.0.0.1" - clusterName := "no-owner-ref" - clusterNameSpace := "default" - metadata := metav1.ObjectMeta{ - Name: clusterName, - Namespace: clusterNameSpace, - } - - caplCluster := clusterv1.Cluster{ - ObjectMeta: metadata, - Spec: clusterv1.ClusterSpec{ - InfrastructureRef: &corev1.ObjectReference{ - Kind: "LinodeCluster", - Name: clusterName, - Namespace: clusterNameSpace, - }, - ControlPlaneRef: &corev1.ObjectReference{ - Kind: "KubeadmControlPlane", - Name: "lifecycle-control-plane", - Namespace: clusterNameSpace, - }, - }, - } - linodeCluster := infrav1.LinodeCluster{ - ObjectMeta: metadata, - Spec: infrav1.LinodeClusterSpec{ - Region: "us-ord", - Network: infrav1.NetworkSpec{ - NodeBalancerID: &nodebalancerID, - }, - ControlPlaneEndpoint: clusterv1.APIEndpoint{ - Host: controlPlaneEndpointHost, - }, - }, - } - - BeforeEach(func() { - reconciler = &LinodeClusterReconciler{ - Client: k8sClient, - LinodeApiKey: "test-key", - } - }) + BeforeEach(func(ctx SpecContext) { + clusterKey := client.ObjectKey{Name: "cluster-lifecycle", Namespace: "default"} + Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) - It("linode cluster should remain NotReady", func(ctx SpecContext) { - clusterKey := client.ObjectKeyFromObject(&linodeCluster) - Expect(k8sClient.Create(ctx, &caplCluster)).To(Succeed()) - Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed()) - _, err := reconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: clusterKey, - }) + // Create patch helper with latest state of resource. + // This is only needed when relying on envtest's k8sClient. + patchHelper, err := patch.NewHelper(&linodeCluster, k8sClient) Expect(err).NotTo(HaveOccurred()) - - By("checking ready conditions") - Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) - Expect(linodeCluster.Status.Ready).To(BeFalse()) - Expect(linodeCluster.Status.FailureMessage).To(BeNil()) - Expect(linodeCluster.Status.FailureReason).To(BeNil()) + cScope.PatchHelper = patchHelper }) + + ctlrSuite.Run(Paths( + Either( + Call("cluster is created", func(ctx context.Context, m Mock) { + cScope.LinodeClient = m.NodeBalancerClient + getNB := m.NodeBalancerClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, nil) + m.NodeBalancerClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()). + After(getNB). + Return(&linodego.NodeBalancer{ + ID: nodebalancerID, + IPv4: &controlPlaneEndpointHost, + }, nil) + m.NodeBalancerClient.EXPECT().CreateNodeBalancerConfig(gomock.Any(), gomock.Any(), gomock.Any()).After(getNB).Return(&linodego.NodeBalancerConfig{ + Port: controlPlaneEndpointPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + NodeBalancerID: nodebalancerID, + }, nil) + }), + Path( + Call("cluster is not created because there is no nb", func(ctx context.Context, m Mock) { + cScope.LinodeClient = m.NodeBalancerClient + getNB := m.NodeBalancerClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, nil) + m.NodeBalancerClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()). + After(getNB). + Return(nil, errors.New("create NB error")) + }), + Result("error", func(ctx context.Context, m Mock) { + _, err := reconciler.reconcile(ctx, cScope, logr.Logger{}) + Expect(err.Error()).To(ContainSubstring("create NB error")) + }), + ), + Path( + Call("cluster is not created because there is no capl cluster", func(ctx context.Context, m Mock) { + cScope.LinodeClient = m.NodeBalancerClient + }), + Result("error", func(ctx context.Context, m Mock) { + reconciler.Client = k8sClient + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: client.ObjectKeyFromObject(cScope.LinodeCluster), + }) + Expect(err).NotTo(HaveOccurred()) + Expect(linodeCluster.Status.Ready).To(BeFalseBecause("failed to get Cluster/no-capl-cluster: clusters.cluster.x-k8s.io \"no-capl-cluster\" not found")) + }), + ), + ), + Result("resource status is updated and NB is created", func(ctx context.Context, m Mock) { + _, err := reconciler.reconcile(ctx, cScope, logr.Logger{}) + Expect(err).NotTo(HaveOccurred()) + + By("checking ready conditions") + clusterKey := client.ObjectKeyFromObject(&linodeCluster) + Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) + Expect(linodeCluster.Status.Ready).To(BeTrue()) + Expect(linodeCluster.Status.Conditions).To(HaveLen(1)) + Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition)) + + By("checking NB id") + Expect(linodeCluster.Spec.Network.NodeBalancerID).To(Equal(&nodebalancerID)) + + By("checking controlPlaneEndpoint/NB host and port") + Expect(linodeCluster.Spec.ControlPlaneEndpoint.Host).To(Equal(controlPlaneEndpointHost)) + Expect(linodeCluster.Spec.ControlPlaneEndpoint.Port).To(Equal(int32(controlPlaneEndpointPort))) + }), + )) }) -var _ = Describe("no-ctrl-plane-endpt", Ordered, Label("cluster", "no-ctrl-plane-endpt"), func() { - var reconciler *LinodeClusterReconciler - clusterName := "no-ctrl-plane-endpt" +var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"), func() { + nodebalancerID := 1 + clusterName := "cluster-delete" clusterNameSpace := "default" ownerRef := metav1.OwnerReference{ Name: clusterName, @@ -234,69 +173,68 @@ var _ = Describe("no-ctrl-plane-endpt", Ordered, Label("cluster", "no-ctrl-plane OwnerReferences: ownerRefs, } - caplCluster := clusterv1.Cluster{ - ObjectMeta: metadata, - Spec: clusterv1.ClusterSpec{ - InfrastructureRef: &corev1.ObjectReference{ - Kind: "LinodeCluster", - Name: clusterName, - Namespace: clusterNameSpace, - }, - ControlPlaneRef: &corev1.ObjectReference{ - Kind: "KubeadmControlPlane", - Name: "lifecycle-control-plane", - Namespace: clusterNameSpace, - }, - }, - } - linodeCluster := infrav1.LinodeCluster{ ObjectMeta: metadata, Spec: infrav1.LinodeClusterSpec{ Region: "us-ord", + Network: infrav1.NetworkSpec{ + NodeBalancerID: &nodebalancerID, + }, }, } - // Create a recorder with a buffered channel for consuming event strings. - recorder := record.NewFakeRecorder(10) - - BeforeEach(func() { - reconciler = &LinodeClusterReconciler{ - Client: k8sClient, - Recorder: recorder, - LinodeApiKey: "test-key", - } - }) - - AfterEach(func() { - // Flush the channel if any events were not consumed. - for len(recorder.Events) > 0 { - <-recorder.Events - } - }) - - It("should fail creating cluster", func(ctx SpecContext) { - clusterKey := client.ObjectKeyFromObject(&linodeCluster) - Expect(k8sClient.Create(ctx, &caplCluster)).To(Succeed()) - Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed()) - _, err := reconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: clusterKey, - }) - Expect(err).To(HaveOccurred()) - - By("checking ready conditions") - Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) - Expect(linodeCluster.Status.Ready).To(BeFalse()) - Expect(linodeCluster.Status.Conditions).To(HaveLen(1)) - Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition)) - - By("checking controlPlaneEndpoint host") - Expect(linodeCluster.Spec.ControlPlaneEndpoint.Host).To(Equal("")) + ctlrSuite := NewControllerTestSuite( + GinkgoT(), + mock.MockLinodeNodeBalancerClient{}, + mock.MockK8sClient{}, + ) + reconciler := LinodeClusterReconciler{ + Recorder: ctlrSuite.Recorder(), + } - By("checking nb id to be nil") - Expect(linodeCluster.Spec.Network.NodeBalancerID).To(BeNil()) + cScope := &scope.ClusterScope{ + LinodeCluster: &linodeCluster, + } - By("recording the expected events") - Expect(<-recorder.Events).To(ContainSubstring("Warning CreateError")) - }) + ctlrSuite.Run(Paths( + Either( + Call("cluster is deleted", func(ctx context.Context, m Mock) { + cScope.LinodeClient = m.NodeBalancerClient + cScope.Client = m.K8sClient + m.NodeBalancerClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(nil) + }), + Path( + Call("nothing to do because NB ID is nil", func(ctx context.Context, m Mock) { + cScope.Client = m.K8sClient + cScope.LinodeClient = m.NodeBalancerClient + cScope.LinodeCluster.Spec.Network.NodeBalancerID = nil + }), + Result("nothing to do because NB ID is nil", func(ctx context.Context, m Mock) { + reconciler.Client = m.K8sClient + err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope) + Expect(err).NotTo(HaveOccurred()) + Expect(ctlrSuite.Events()).To(ContainSubstring("Warning NodeBalancerIDMissing NodeBalancer ID is missing, nothing to do")) + }), + ), + Path( + Call("cluster not deleted because the nb can't be deleted", func(ctx context.Context, m Mock) { + cScope.LinodeClient = m.NodeBalancerClient + cScope.Client = m.K8sClient + cScope.LinodeCluster.Spec.Network.NodeBalancerID = &nodebalancerID + m.NodeBalancerClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(errors.New("delete NB error")) + }), + Result("cluster not deleted because the nb can't be deleted", func(ctx context.Context, m Mock) { + reconciler.Client = m.K8sClient + err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("delete NB error")) + }), + ), + ), + Result("cluster deleted", func(ctx context.Context, m Mock) { + reconciler.Client = m.K8sClient + err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope) + Expect(err).NotTo(HaveOccurred()) + }), + )) })