From 7db8837ca28449b1cf4090df5adec5b683b5f814 Mon Sep 17 00:00:00 2001 From: Ashley Dumaine <5779804+AshleyDumaine@users.noreply.github.com> Date: Mon, 8 Jul 2024 17:14:41 -0400 Subject: [PATCH] [fix] add support for filling in controlPlaneEndpoint if only network info is specified (#395) * only create the nodebalancer and config if not already set, switch to getNB instead of listNB * clean up unused listNB function --- clients/clients.go | 3 +- cloud/services/loadbalancers.go | 104 +++++++---------- cloud/services/loadbalancers_test.go | 109 +++++++++--------- controller/linodecluster_controller.go | 46 +++++--- controller/linodecluster_controller_test.go | 67 ++++++++--- mock/client.go | 74 ++++++++---- .../wrappers/linodeclient/linodeclient.gen.go | 70 +++++++---- 7 files changed, 273 insertions(+), 200 deletions(-) diff --git a/clients/clients.go b/clients/clients.go index e596d40ff..85608264a 100644 --- a/clients/clients.go +++ b/clients/clients.go @@ -48,8 +48,9 @@ type LinodeVPCClient interface { // LinodeNodeBalancerClient defines the methods that interact with Linode's Node Balancer service. type LinodeNodeBalancerClient interface { - ListNodeBalancers(ctx context.Context, opts *linodego.ListOptions) ([]linodego.NodeBalancer, error) CreateNodeBalancer(ctx context.Context, opts linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error) + GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error) + GetNodeBalancerConfig(ctx context.Context, nodebalancerID int, configID int) (*linodego.NodeBalancerConfig, error) CreateNodeBalancerConfig(ctx context.Context, nodebalancerID int, opts linodego.NodeBalancerConfigCreateOptions) (*linodego.NodeBalancerConfig, error) DeleteNodeBalancerNode(ctx context.Context, nodebalancerID int, configID int, nodeID int) error DeleteNodeBalancer(ctx context.Context, nodebalancerID int) error diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go index 7500e9ff3..ee2cf81f9 100644 --- a/cloud/services/loadbalancers.go +++ b/cloud/services/loadbalancers.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net/http" - "slices" "github.com/go-logr/logr" "github.com/linode/linodego" @@ -20,85 +19,69 @@ const ( DefaultKonnectivityLBPort = 8132 ) -// CreateNodeBalancer creates a new NodeBalancer if one doesn't exist -func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { - var linodeNB *linodego.NodeBalancer - - NBLabel := clusterScope.LinodeCluster.Name - clusterUID := string(clusterScope.LinodeCluster.UID) - tags := []string{string(clusterScope.LinodeCluster.UID)} - listFilter := util.Filter{ - ID: clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, - Label: NBLabel, - Tags: tags, - } - filter, err := listFilter.String() - if err != nil { - return nil, err - } - linodeNBs, err := clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, filter)) - if err != nil { - logger.Info("Failed to list NodeBalancers", "error", err.Error()) - - return nil, err - } - if len(linodeNBs) == 1 { - logger.Info(fmt.Sprintf("NodeBalancer %s already exists", *linodeNBs[0].Label)) - if !slices.Contains(linodeNBs[0].Tags, clusterUID) { - err = errors.New("NodeBalancer conflict") - logger.Error(err, fmt.Sprintf("NodeBalancer %s is not associated with cluster UID %s. Owner cluster is %s", *linodeNBs[0].Label, clusterUID, linodeNBs[0].Tags[0])) +// EnsureNodeBalancer creates a new NodeBalancer if one doesn't exist or returns the existing NodeBalancer +func EnsureNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { + nbID := clusterScope.LinodeCluster.Spec.Network.NodeBalancerID + if nbID != nil && *nbID != 0 { + res, err := clusterScope.LinodeClient.GetNodeBalancer(ctx, *nbID) + if err != nil { + logger.Info("Failed to get NodeBalancer", "error", err.Error()) return nil, err } - - return &linodeNBs[0], nil + return res, nil } logger.Info(fmt.Sprintf("Creating NodeBalancer %s", clusterScope.LinodeCluster.Name)) createConfig := linodego.NodeBalancerCreateOptions{ Label: util.Pointer(clusterScope.LinodeCluster.Name), Region: clusterScope.LinodeCluster.Spec.Region, - Tags: tags, + Tags: []string{string(clusterScope.LinodeCluster.UID)}, } - linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig) - if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil { - return nil, err - } - if linodeNB != nil { - logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) - } - - return linodeNB, nil + return clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig) } -// CreateNodeBalancerConfigs creates NodeBalancer configs if it does not exist -func CreateNodeBalancerConfigs( +// EnsureNodeBalancerConfigs creates NodeBalancer configs if it does not exist or returns the existing NodeBalancerConfig +func EnsureNodeBalancerConfigs( ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger, ) ([]*linodego.NodeBalancerConfig, error) { nbConfigs := []*linodego.NodeBalancerConfig{} + var apiserverLinodeNBConfig *linodego.NodeBalancerConfig + var err error apiLBPort := DefaultApiserverLBPort if clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 { apiLBPort = clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort } - apiserverCreateConfig := linodego.NodeBalancerConfigCreateOptions{ - Port: apiLBPort, - Protocol: linodego.ProtocolTCP, - Algorithm: linodego.AlgorithmRoundRobin, - Check: linodego.CheckConnection, - } - apiserverLinodeNBConfig, err := clusterScope.LinodeClient.CreateNodeBalancerConfig( - ctx, - *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, - apiserverCreateConfig, - ) - if err != nil { - logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) - return nil, err + if clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID != nil { + apiserverLinodeNBConfig, err = clusterScope.LinodeClient.GetNodeBalancerConfig( + ctx, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID) + if err != nil { + logger.Info("Failed to get Linode NodeBalancer config", "error", err.Error()) + return nil, err + } + } else { + apiserverLinodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig( + ctx, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + linodego.NodeBalancerConfigCreateOptions{ + Port: apiLBPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + }, + ) + if err != nil { + logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) + return nil, err + } } + nbConfigs = append(nbConfigs, apiserverLinodeNBConfig) // return if additional ports should not be configured @@ -180,11 +163,6 @@ func AddNodeToNB( return err } - // return if additional ports should not be configured - if len(machineScope.LinodeCluster.Spec.Network.AdditionalPorts) == 0 { - return nil - } - for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts { _, err = machineScope.LinodeClient.CreateNodeBalancerNode( ctx, @@ -234,10 +212,6 @@ func DeleteNodeFromNB( return err } - if len(machineScope.LinodeCluster.Spec.Network.AdditionalPorts) == 0 { - return nil - } - for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts { err = machineScope.LinodeClient.DeleteNodeBalancerNode( ctx, diff --git a/cloud/services/loadbalancers_test.go b/cloud/services/loadbalancers_test.go index 27507c3e1..8da8ac06b 100644 --- a/cloud/services/loadbalancers_test.go +++ b/cloud/services/loadbalancers_test.go @@ -19,7 +19,7 @@ import ( "github.com/linode/cluster-api-provider-linode/mock" ) -func TestCreateNodeBalancer(t *testing.T) { +func TestEnsureNodeBalancer(t *testing.T) { t.Parallel() tests := []struct { name string @@ -50,8 +50,7 @@ func TestCreateNodeBalancer(t *testing.T) { }, }, expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{}, nil) - mockClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{ + mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{ ID: 1234, }, nil) }, @@ -60,7 +59,7 @@ func TestCreateNodeBalancer(t *testing.T) { }, }, { - name: "Success - List NodeBalancers returns one nodebalancer and we return that", + name: "Success - Get NodeBalancers returns one nodebalancer and we return that", clusterScope: &scope.ClusterScope{ LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -75,12 +74,10 @@ func TestCreateNodeBalancer(t *testing.T) { }, }, expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{ - { - ID: 1234, - Label: ptr.To("test"), - Tags: []string{"test-uid"}, - }, + mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{ + ID: 1234, + Label: ptr.To("test"), + Tags: []string{"test-uid"}, }, nil) }, expectedNodeBalancer: &linodego.NodeBalancer{ @@ -90,38 +87,7 @@ func TestCreateNodeBalancer(t *testing.T) { }, }, { - name: "Error - List NodeBalancers returns one nodebalancer but there is a nodebalancer conflict", - clusterScope: &scope.ClusterScope{ - LinodeCluster: &infrav1alpha2.LinodeCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeClusterSpec{ - Network: infrav1alpha2.NetworkSpec{ - NodeBalancerID: ptr.To(1234), - }, - }, - }, - }, - expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{ - { - ID: 1234, - Label: ptr.To("test"), - Tags: []string{"test"}, - }, - }, nil) - }, - expectedNodeBalancer: &linodego.NodeBalancer{ - ID: 1234, - Label: ptr.To("test"), - Tags: []string{"test"}, - }, - expectedError: fmt.Errorf("NodeBalancer conflict"), - }, - { - name: "Error - List NodeBalancers returns an error", + name: "Error - Get NodeBalancer returns an error", clusterScope: &scope.ClusterScope{ LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -136,9 +102,9 @@ func TestCreateNodeBalancer(t *testing.T) { }, }, expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to list NodeBalancers")) + mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to get NodeBalancer")) }, - expectedError: fmt.Errorf("Unable to list NodeBalancers"), + expectedError: fmt.Errorf("Unable to get NodeBalancer"), }, { name: "Error - Create NodeBalancer returns an error", @@ -148,15 +114,10 @@ func TestCreateNodeBalancer(t *testing.T) { Name: "test-cluster", UID: "test-uid", }, - Spec: infrav1alpha2.LinodeClusterSpec{ - Network: infrav1alpha2.NetworkSpec{ - NodeBalancerID: ptr.To(1234), - }, - }, + Spec: infrav1alpha2.LinodeClusterSpec{}, }, }, expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{}, nil) mockClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to create NodeBalancer")) }, expectedError: fmt.Errorf("Unable to create NodeBalancer"), @@ -176,7 +137,7 @@ func TestCreateNodeBalancer(t *testing.T) { testcase.expects(MockLinodeClient) - got, err := CreateNodeBalancer(context.Background(), testcase.clusterScope, logr.Discard()) + got, err := EnsureNodeBalancer(context.Background(), testcase.clusterScope, logr.Discard()) if testcase.expectedError != nil { assert.ErrorContains(t, err, testcase.expectedError.Error()) } else { @@ -187,7 +148,7 @@ func TestCreateNodeBalancer(t *testing.T) { } } -func TestCreateNodeBalancerConfigs(t *testing.T) { +func TestEnsureNodeBalancerConfigs(t *testing.T) { t.Parallel() tests := []struct { @@ -232,6 +193,48 @@ func TestCreateNodeBalancerConfigs(t *testing.T) { }, nil) }, }, + { + name: "Success - Get NodeBalancerConfig", + clusterScope: &scope.ClusterScope{ + LinodeClient: nil, + LinodeCluster: &infrav1alpha2.LinodeCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeClusterSpec{ + Network: infrav1alpha2.NetworkSpec{ + NodeBalancerID: ptr.To(1234), + ApiserverNodeBalancerConfigID: ptr.To(2), + }, + ControlPlaneEndpoint: clusterv1.APIEndpoint{ + Host: "", + Port: 0, + }, + }, + }, + }, + expectedConfigs: []*linodego.NodeBalancerConfig{ + { + Port: DefaultApiserverLBPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + NodeBalancerID: 1234, + ID: 2, + }, + }, + expects: func(mockClient *mock.MockLinodeClient) { + mockClient.EXPECT().GetNodeBalancerConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancerConfig{ + ID: 2, + Port: DefaultApiserverLBPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + NodeBalancerID: 1234, + }, nil) + }, + }, { name: "Success - Create NodeBalancerConfig using assigned LB ports", clusterScope: &scope.ClusterScope{ @@ -396,7 +399,7 @@ func TestCreateNodeBalancerConfigs(t *testing.T) { testcase.expects(MockLinodeClient) - got, err := CreateNodeBalancerConfigs(context.Background(), testcase.clusterScope, logr.Discard()) + got, err := EnsureNodeBalancerConfigs(context.Background(), testcase.clusterScope, logr.Discard()) if testcase.expectedError != nil { assert.ErrorContains(t, err, testcase.expectedError.Error()) } else { diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 62fb30884..17e6eb8d3 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -142,10 +142,10 @@ func (r *LinodeClusterReconciler) reconcile( } // Create - if (clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "") || (clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "dns") { + if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { if err := r.reconcileCreate(ctx, logger, clusterScope); err != nil { if !reconciler.HasConditionSeverity(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.ConditionSeverityError) { - logger.Info("re-queuing cluster/nb creation") + logger.Info("re-queuing cluster/load-balancer creation") return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil } return res, err @@ -175,36 +175,36 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger lo return err } + // handle creation for the loadbalancer for the control plane if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "dns" { - domainName := clusterScope.LinodeCluster.ObjectMeta.Name + "-" + clusterScope.LinodeCluster.Spec.Network.DNSUniqueIdentifier + "." + clusterScope.LinodeCluster.Spec.Network.DNSRootDomain - apiLBPort := services.DefaultApiserverLBPort - if clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 { - apiLBPort = clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort - } - clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{ - Host: domainName, - Port: int32(apiLBPort), + r.handleDNS(clusterScope) + } else if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "NodeBalancer" || clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "" { + if err := r.handleNBCreate(ctx, logger, clusterScope); err != nil { + return err } - return nil } - linodeNB, err := services.CreateNodeBalancer(ctx, clusterScope, logger) + + return nil +} + +func (r *LinodeClusterReconciler) handleNBCreate(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { + linodeNB, err := services.EnsureNodeBalancer(ctx, clusterScope, logger) if err != nil { - logger.Error(err, "failed to create nodebalancer") + logger.Error(err, "failed to ensure nodebalancer") setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) return err } - if linodeNB == nil { err = fmt.Errorf("nodeBalancer created was nil") setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) return err } - clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = &linodeNB.ID - configs, err := services.CreateNodeBalancerConfigs(ctx, clusterScope, logger) + // create the configs for the nodeabalancer if not already specified + configs, err := services.EnsureNodeBalancerConfigs(ctx, clusterScope, logger) if err != nil { - logger.Error(err, "failed to create nodebalancer config") + logger.Error(err, "failed to ensure nodebalancer configs") setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) return err } @@ -228,6 +228,18 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger lo return nil } +func (r *LinodeClusterReconciler) handleDNS(clusterScope *scope.ClusterScope) { + domainName := clusterScope.LinodeCluster.ObjectMeta.Name + "-" + clusterScope.LinodeCluster.Spec.Network.DNSUniqueIdentifier + "." + clusterScope.LinodeCluster.Spec.Network.DNSRootDomain + apiLBPort := services.DefaultApiserverLBPort + if clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 { + apiLBPort = clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort + } + clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{ + Host: domainName, + Port: int32(apiLBPort), + } +} + func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { logger.Info("deleting cluster") if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil { diff --git a/controller/linodecluster_controller_test.go b/controller/linodecluster_controller_test.go index 08f8db7b2..daf4d50bd 100644 --- a/controller/linodecluster_controller_test.go +++ b/controller/linodecluster_controller_test.go @@ -33,6 +33,7 @@ import ( infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2" "github.com/linode/cluster-api-provider-linode/cloud/scope" "github.com/linode/cluster-api-provider-linode/mock" + "github.com/linode/cluster-api-provider-linode/util" rec "github.com/linode/cluster-api-provider-linode/util/reconciler" . "github.com/linode/cluster-api-provider-linode/mock/mocktest" @@ -42,6 +43,7 @@ import ( var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecycle"), func() { nodebalancerID := 1 + nbConfigID := util.Pointer(3) controlPlaneEndpointHost := "10.0.0.1" controlPlaneEndpointPort := 6443 clusterName := "cluster-lifecycle" @@ -93,24 +95,22 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path( Call("cluster is not created because there was an error creating nb", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient - getNB := mck.LinodeClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, nil) mck.LinodeClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()). - After(getNB). - Return(nil, errors.New("create NB error")) + Return(nil, errors.New("failed to ensure nodebalancer")) }), OneOf( Path(Result("create requeues", func(ctx context.Context, mck Mock) { res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).NotTo(HaveOccurred()) Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) - Expect(mck.Logs()).To(ContainSubstring("re-queuing cluster/nb creation")) + Expect(mck.Logs()).To(ContainSubstring("re-queuing cluster/load-balancer creation")) })), Path(Result("create nb error - timeout error", func(ctx context.Context, mck Mock) { tempTimeout := reconciler.ReconcileTimeout reconciler.ReconcileTimeout = time.Nanosecond _, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("create NB error")) + Expect(err.Error()).To(ContainSubstring("failed to ensure nodebalancer")) reconciler.ReconcileTimeout = tempTimeout })), ), @@ -118,9 +118,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path( Call("cluster is not created because nb was nil", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient - getNB := mck.LinodeClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, nil) mck.LinodeClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()). - After(getNB). Return(nil, nil) }), OneOf( @@ -128,7 +126,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).NotTo(HaveOccurred()) Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) - Expect(mck.Logs()).To(ContainSubstring("re-queuing cluster/nb creation")) + Expect(mck.Logs()).To(ContainSubstring("re-queuing cluster/load-balancer creation")) })), Path(Result("create nb error - timeout error", func(ctx context.Context, mck Mock) { tempTimeout := reconciler.ReconcileTimeout @@ -143,30 +141,62 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path( Call("cluster is not created because nb config was nil", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient - getNB := mck.LinodeClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, nil) - mck.LinodeClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()). - After(getNB). + mck.LinodeClient.EXPECT().CreateNodeBalancerConfig(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, errors.New("nodeBalancer config created was nil")) + }), + OneOf( + Path(Result("create requeues", func(ctx context.Context, mck Mock) { + mck.LinodeClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()). + Return(&linodego.NodeBalancer{ + ID: nodebalancerID, + IPv4: &controlPlaneEndpointHost, + }, nil) + res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) + Expect(err).NotTo(HaveOccurred()) + Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) + Expect(mck.Logs()).To(ContainSubstring("re-queuing cluster/load-balancer creation")) + })), + Path(Result("create nb error - timeout error", func(ctx context.Context, mck Mock) { + mck.LinodeClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()). + Return(&linodego.NodeBalancer{ + ID: nodebalancerID, + IPv4: &controlPlaneEndpointHost, + }, nil) + + tempTimeout := reconciler.ReconcileTimeout + reconciler.ReconcileTimeout = time.Nanosecond + _, err := reconciler.reconcile(ctx, cScope, mck.Logger()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("nodeBalancer config created was nil")) + reconciler.ReconcileTimeout = tempTimeout + })), + ), + ), + Path( + Call("cluster is not created because there was an error getting nb config", func(ctx context.Context, mck Mock) { + cScope.LinodeClient = mck.LinodeClient + cScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nbConfigID + mck.LinodeClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()). Return(&linodego.NodeBalancer{ ID: nodebalancerID, IPv4: &controlPlaneEndpointHost, }, nil) - mck.LinodeClient.EXPECT().CreateNodeBalancerConfig(gomock.Any(), gomock.Any(), gomock.Any()). - After(getNB). - Return(nil, errors.New("nodeBalancer config created was nil")) + mck.LinodeClient.EXPECT().GetNodeBalancerConfig(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, errors.New("failed to get nodebalancer config")) }), OneOf( Path(Result("create requeues", func(ctx context.Context, mck Mock) { res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).NotTo(HaveOccurred()) Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) - Expect(mck.Logs()).To(ContainSubstring("re-queuing cluster/nb creation")) + Expect(mck.Logs()).To(ContainSubstring("re-queuing cluster/load-balancer creation")) })), Path(Result("create nb error - timeout error", func(ctx context.Context, mck Mock) { tempTimeout := reconciler.ReconcileTimeout reconciler.ReconcileTimeout = time.Nanosecond _, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("nodeBalancer config created was nil")) + Expect(err.Error()).To(ContainSubstring("failed to get nodebalancer config")) reconciler.ReconcileTimeout = tempTimeout })), ), @@ -187,9 +217,8 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path( Call("cluster is created", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient - getNB := mck.LinodeClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, nil) - mck.LinodeClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()). - After(getNB). + cScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nil + getNB := mck.LinodeClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()). Return(&linodego.NodeBalancer{ ID: nodebalancerID, IPv4: &controlPlaneEndpointHost, diff --git a/mock/client.go b/mock/client.go index 848049687..7e35c64fc 100644 --- a/mock/client.go +++ b/mock/client.go @@ -352,6 +352,36 @@ func (mr *MockLinodeClientMockRecorder) GetInstanceIPAddresses(ctx, linodeID any return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetInstanceIPAddresses", reflect.TypeOf((*MockLinodeClient)(nil).GetInstanceIPAddresses), ctx, linodeID) } +// GetNodeBalancer mocks base method. +func (m *MockLinodeClient) GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNodeBalancer", ctx, nodebalancerID) + ret0, _ := ret[0].(*linodego.NodeBalancer) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNodeBalancer indicates an expected call of GetNodeBalancer. +func (mr *MockLinodeClientMockRecorder) GetNodeBalancer(ctx, nodebalancerID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNodeBalancer", reflect.TypeOf((*MockLinodeClient)(nil).GetNodeBalancer), ctx, nodebalancerID) +} + +// GetNodeBalancerConfig mocks base method. +func (m *MockLinodeClient) GetNodeBalancerConfig(ctx context.Context, nodebalancerID, configID int) (*linodego.NodeBalancerConfig, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNodeBalancerConfig", ctx, nodebalancerID, configID) + ret0, _ := ret[0].(*linodego.NodeBalancerConfig) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNodeBalancerConfig indicates an expected call of GetNodeBalancerConfig. +func (mr *MockLinodeClientMockRecorder) GetNodeBalancerConfig(ctx, nodebalancerID, configID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNodeBalancerConfig", reflect.TypeOf((*MockLinodeClient)(nil).GetNodeBalancerConfig), ctx, nodebalancerID, configID) +} + // GetObjectStorageBucket mocks base method. func (m *MockLinodeClient) GetObjectStorageBucket(ctx context.Context, cluster, label string) (*linodego.ObjectStorageBucket, error) { m.ctrl.T.Helper() @@ -487,21 +517,6 @@ func (mr *MockLinodeClientMockRecorder) ListInstances(ctx, opts any) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListInstances", reflect.TypeOf((*MockLinodeClient)(nil).ListInstances), ctx, opts) } -// ListNodeBalancers mocks base method. -func (m *MockLinodeClient) ListNodeBalancers(ctx context.Context, opts *linodego.ListOptions) ([]linodego.NodeBalancer, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListNodeBalancers", ctx, opts) - ret0, _ := ret[0].([]linodego.NodeBalancer) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ListNodeBalancers indicates an expected call of ListNodeBalancers. -func (mr *MockLinodeClientMockRecorder) ListNodeBalancers(ctx, opts any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodeBalancers", reflect.TypeOf((*MockLinodeClient)(nil).ListNodeBalancers), ctx, opts) -} - // ListStackscripts mocks base method. func (m *MockLinodeClient) ListStackscripts(ctx context.Context, opts *linodego.ListOptions) ([]linodego.Stackscript, error) { m.ctrl.T.Helper() @@ -1014,19 +1029,34 @@ func (mr *MockLinodeNodeBalancerClientMockRecorder) DeleteNodeBalancerNode(ctx, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteNodeBalancerNode", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).DeleteNodeBalancerNode), ctx, nodebalancerID, configID, nodeID) } -// ListNodeBalancers mocks base method. -func (m *MockLinodeNodeBalancerClient) ListNodeBalancers(ctx context.Context, opts *linodego.ListOptions) ([]linodego.NodeBalancer, error) { +// GetNodeBalancer mocks base method. +func (m *MockLinodeNodeBalancerClient) GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNodeBalancer", ctx, nodebalancerID) + ret0, _ := ret[0].(*linodego.NodeBalancer) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNodeBalancer indicates an expected call of GetNodeBalancer. +func (mr *MockLinodeNodeBalancerClientMockRecorder) GetNodeBalancer(ctx, nodebalancerID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNodeBalancer", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).GetNodeBalancer), ctx, nodebalancerID) +} + +// GetNodeBalancerConfig mocks base method. +func (m *MockLinodeNodeBalancerClient) GetNodeBalancerConfig(ctx context.Context, nodebalancerID, configID int) (*linodego.NodeBalancerConfig, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListNodeBalancers", ctx, opts) - ret0, _ := ret[0].([]linodego.NodeBalancer) + ret := m.ctrl.Call(m, "GetNodeBalancerConfig", ctx, nodebalancerID, configID) + ret0, _ := ret[0].(*linodego.NodeBalancerConfig) ret1, _ := ret[1].(error) return ret0, ret1 } -// ListNodeBalancers indicates an expected call of ListNodeBalancers. -func (mr *MockLinodeNodeBalancerClientMockRecorder) ListNodeBalancers(ctx, opts any) *gomock.Call { +// GetNodeBalancerConfig indicates an expected call of GetNodeBalancerConfig. +func (mr *MockLinodeNodeBalancerClientMockRecorder) GetNodeBalancerConfig(ctx, nodebalancerID, configID any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodeBalancers", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).ListNodeBalancers), ctx, opts) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNodeBalancerConfig", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).GetNodeBalancerConfig), ctx, nodebalancerID, configID) } // MockLinodeObjectStorageClient is a mock of LinodeObjectStorageClient interface. diff --git a/observability/wrappers/linodeclient/linodeclient.gen.go b/observability/wrappers/linodeclient/linodeclient.gen.go index fe7f1291f..17f283240 100644 --- a/observability/wrappers/linodeclient/linodeclient.gen.go +++ b/observability/wrappers/linodeclient/linodeclient.gen.go @@ -521,6 +521,53 @@ func (_d LinodeClientWithTracing) GetInstanceIPAddresses(ctx context.Context, li return _d.LinodeClient.GetInstanceIPAddresses(ctx, linodeID) } +// GetNodeBalancer implements clients.LinodeClient +func (_d LinodeClientWithTracing) GetNodeBalancer(ctx context.Context, nodebalancerID int) (np1 *linodego.NodeBalancer, err error) { + ctx, _span := tracing.Start(ctx, "clients.LinodeClient.GetNodeBalancer") + defer func() { + if _d._spanDecorator != nil { + _d._spanDecorator(_span, map[string]interface{}{ + "ctx": ctx, + "nodebalancerID": nodebalancerID}, map[string]interface{}{ + "np1": np1, + "err": err}) + } else if err != nil { + _span.RecordError(err) + _span.SetAttributes( + attribute.String("event", "error"), + attribute.String("message", err.Error()), + ) + } + + _span.End() + }() + return _d.LinodeClient.GetNodeBalancer(ctx, nodebalancerID) +} + +// GetNodeBalancerConfig implements clients.LinodeClient +func (_d LinodeClientWithTracing) GetNodeBalancerConfig(ctx context.Context, nodebalancerID int, configID int) (np1 *linodego.NodeBalancerConfig, err error) { + ctx, _span := tracing.Start(ctx, "clients.LinodeClient.GetNodeBalancerConfig") + defer func() { + if _d._spanDecorator != nil { + _d._spanDecorator(_span, map[string]interface{}{ + "ctx": ctx, + "nodebalancerID": nodebalancerID, + "configID": configID}, map[string]interface{}{ + "np1": np1, + "err": err}) + } else if err != nil { + _span.RecordError(err) + _span.SetAttributes( + attribute.String("event", "error"), + attribute.String("message", err.Error()), + ) + } + + _span.End() + }() + return _d.LinodeClient.GetNodeBalancerConfig(ctx, nodebalancerID, configID) +} + // GetObjectStorageBucket implements clients.LinodeClient func (_d LinodeClientWithTracing) GetObjectStorageBucket(ctx context.Context, cluster string, label string) (op1 *linodego.ObjectStorageBucket, err error) { ctx, _span := tracing.Start(ctx, "clients.LinodeClient.GetObjectStorageBucket") @@ -731,29 +778,6 @@ func (_d LinodeClientWithTracing) ListInstances(ctx context.Context, opts *linod return _d.LinodeClient.ListInstances(ctx, opts) } -// ListNodeBalancers implements clients.LinodeClient -func (_d LinodeClientWithTracing) ListNodeBalancers(ctx context.Context, opts *linodego.ListOptions) (na1 []linodego.NodeBalancer, err error) { - ctx, _span := tracing.Start(ctx, "clients.LinodeClient.ListNodeBalancers") - defer func() { - if _d._spanDecorator != nil { - _d._spanDecorator(_span, map[string]interface{}{ - "ctx": ctx, - "opts": opts}, map[string]interface{}{ - "na1": na1, - "err": err}) - } else if err != nil { - _span.RecordError(err) - _span.SetAttributes( - attribute.String("event", "error"), - attribute.String("message", err.Error()), - ) - } - - _span.End() - }() - return _d.LinodeClient.ListNodeBalancers(ctx, opts) -} - // ListStackscripts implements clients.LinodeClient func (_d LinodeClientWithTracing) ListStackscripts(ctx context.Context, opts *linodego.ListOptions) (sa1 []linodego.Stackscript, err error) { ctx, _span := tracing.Start(ctx, "clients.LinodeClient.ListStackscripts")