diff --git a/clients/clients.go b/clients/clients.go index 93ff0d2a9..bba1801c1 100644 --- a/clients/clients.go +++ b/clients/clients.go @@ -64,6 +64,7 @@ type LinodeVPCClient interface { type LinodeNodeBalancerClient interface { CreateNodeBalancer(ctx context.Context, opts linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error) GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error) + ListNodeBalancerNodes(ctx context.Context, nodebalancerID int, configID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerNode, 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 diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go index 058844493..c3408af70 100644 --- a/cloud/services/loadbalancers.go +++ b/cloud/services/loadbalancers.go @@ -5,11 +5,13 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/go-logr/logr" "github.com/linode/linodego" "sigs.k8s.io/cluster-api/api/v1beta1" + "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/util" ) @@ -112,61 +114,73 @@ func EnsureNodeBalancerConfigs( } // AddNodesToNB adds backend Nodes on the Node Balancer configuration -func AddNodesToNB(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { +func AddNodesToNB(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope, eachMachine v1alpha2.LinodeMachine) error { apiserverLBPort := DefaultApiserverLBPort if clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 { apiserverLBPort = clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort } if clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID == nil { - err := errors.New("nil NodeBalancer Config ID") - logger.Error(err, "config ID for NodeBalancer is nil") + return errors.New("nil NodeBalancer Config ID") + } + nodeBalancerNodes, err := clusterScope.LinodeClient.ListNodeBalancerNodes( + ctx, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, + &linodego.ListOptions{}, + ) + if err != nil { return err } + internalIPFound := false + for _, IPs := range eachMachine.Status.Addresses { + if IPs.Type != v1beta1.MachineInternalIP || !strings.Contains(IPs.Address, "192.168") { + continue + } + internalIPFound = true - for _, eachMachine := range clusterScope.LinodeMachines.Items { - internalIPFound := false - for _, IPs := range eachMachine.Status.Addresses { - if IPs.Type != v1beta1.MachineInternalIP { - continue - } - internalIPFound = true - _, err := clusterScope.LinodeClient.CreateNodeBalancerNode( - ctx, - *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, - *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, - linodego.NodeBalancerNodeCreateOptions{ - Label: clusterScope.Cluster.Name, - Address: fmt.Sprintf("%s:%d", IPs.Address, apiserverLBPort), - Mode: linodego.ModeAccept, - }, - ) - if err != nil { - logger.Error(err, "Failed to update Node Balancer") - return err - } + // Set the port number and NB config ID for standard ports + portsToBeAdded := make([]map[string]int, 0) + standardPort := map[string]int{"configID": *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, "port": apiserverLBPort} + portsToBeAdded = append(portsToBeAdded, standardPort) + + // Set the port number and NB config ID for any additional ports + for _, portConfig := range clusterScope.LinodeCluster.Spec.Network.AdditionalPorts { + portsToBeAdded = append(portsToBeAdded, map[string]int{"configID": *portConfig.NodeBalancerConfigID, "port": portConfig.Port}) + } + + logger.Info("abir", "portsToBeAdded", portsToBeAdded) - for _, portConfig := range clusterScope.LinodeCluster.Spec.Network.AdditionalPorts { - _, err = clusterScope.LinodeClient.CreateNodeBalancerNode( + // Cycle through all ports to be added + for _, ports := range portsToBeAdded { + ipPortComboExists := false + for _, nodes := range nodeBalancerNodes { + // Create the node if the IP:Port combination does not exist + if nodes.Address == fmt.Sprintf("%s:%d", IPs.Address, ports["port"]) { + ipPortComboExists = true + break + } + } + if !ipPortComboExists { + _, err := clusterScope.LinodeClient.CreateNodeBalancerNode( ctx, *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, - *portConfig.NodeBalancerConfigID, + ports["configID"], linodego.NodeBalancerNodeCreateOptions{ Label: clusterScope.Cluster.Name, - Address: fmt.Sprintf("%s:%d", IPs.Address, portConfig.Port), + Address: fmt.Sprintf("%s:%d", IPs.Address, ports["port"]), Mode: linodego.ModeAccept, }, ) if err != nil { - logger.Error(err, "Failed to update Node Balancer") return err } } } - if !internalIPFound { - return errors.New("no private IP address") - } + } + if !internalIPFound { + return errors.New("no private IP address") } return nil diff --git a/cloud/services/loadbalancers_test.go b/cloud/services/loadbalancers_test.go index ddb277a0a..e22089aac 100644 --- a/cloud/services/loadbalancers_test.go +++ b/cloud/services/loadbalancers_test.go @@ -488,7 +488,9 @@ func TestAddNodeToNBConditions(t *testing.T) { }, }, expectedError: fmt.Errorf("no private IP address"), - expects: func(mockClient *mock.MockLinodeClient) {}, + expects: func(mockClient *mock.MockLinodeClient) { + mockClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil) + }, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() }, @@ -509,9 +511,11 @@ func TestAddNodeToNBConditions(t *testing.T) { testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := AddNodesToNB(context.Background(), logr.Discard(), testcase.clusterScope) - if testcase.expectedError != nil { - assert.ErrorContains(t, err, testcase.expectedError.Error()) + for _, eachMachine := range testcase.clusterScope.LinodeMachines.Items { + err := AddNodesToNB(context.Background(), logr.Discard(), testcase.clusterScope, eachMachine) + if testcase.expectedError != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } } }) } @@ -527,53 +531,6 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { expects func(*mock.MockLinodeClient) expectK8sClient func(*mock.MockK8sClient) }{ - { - name: "If the machine is not a control plane node, do nothing", - 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), - ApiserverNodeBalancerConfigID: ptr.To(5678), - AdditionalPorts: []infrav1alpha2.LinodeNBPortConfig{ - { - Port: DefaultKonnectivityLBPort, - NodeBalancerConfigID: ptr.To(1234), - }, - }, - }, - }, - }, - Cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - UID: "test-uid", - }, - }, - LinodeMachines: infrav1alpha2.LinodeMachineList{ - Items: []infrav1alpha2.LinodeMachine{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - InstanceID: ptr.To(123), - }, - }, - }, - }, - }, - expects: func(*mock.MockLinodeClient) {}, - expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { - mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() - }, - }, { name: "Success - If the machine is a control plane node, add the node to the NodeBalancer", clusterScope: &scope.ClusterScope{ @@ -617,6 +574,7 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { }, }, expects: func(mockClient *mock.MockLinodeClient) { + mockClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil) mockClient.EXPECT().CreateNodeBalancerNode(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(&linodego.NodeBalancerNode{}, nil) }, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { @@ -667,9 +625,10 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { }, }, }, - expectedError: fmt.Errorf("could not create node balancer node"), + expectedError: nil, expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().CreateNodeBalancerNode(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("could not create node balancer node")) + mockClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil) + mockClient.EXPECT().CreateNodeBalancerNode(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() }, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() @@ -691,9 +650,11 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := AddNodesToNB(context.Background(), logr.Discard(), testcase.clusterScope) - if testcase.expectedError != nil { - assert.ErrorContains(t, err, testcase.expectedError.Error()) + for _, eachMachine := range testcase.clusterScope.LinodeMachines.Items { + err := AddNodesToNB(context.Background(), logr.Discard(), testcase.clusterScope, eachMachine) + if testcase.expectedError != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } } }) } @@ -710,39 +671,6 @@ func TestDeleteNodeFromNB(t *testing.T) { expectK8sClient func(*mock.MockK8sClient) }{ // TODO: Add test cases. - { - name: "If the machine is not a control plane node, do nothing", - 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), - ApiserverNodeBalancerConfigID: ptr.To(5678), - AdditionalPorts: []infrav1alpha2.LinodeNBPortConfig{ - { - Port: DefaultKonnectivityLBPort, - NodeBalancerConfigID: ptr.To(1234), - }, - }, - }, - }, - }, - Cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - UID: "test-uid", - }, - }, - }, - expects: func(*mock.MockLinodeClient) {}, - expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { - mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() - }, - }, { name: "NodeBalancer is already deleted", clusterScope: &scope.ClusterScope{ diff --git a/controller/linodecluster_controller_helpers.go b/controller/linodecluster_controller_helpers.go index b2c647a29..fbfe1c491 100644 --- a/controller/linodecluster_controller_helpers.go +++ b/controller/linodecluster_controller_helpers.go @@ -12,8 +12,10 @@ import ( func (r *LinodeClusterReconciler) addMachineToLB(ctx context.Context, clusterScope *scope.ClusterScope) error { logger := logr.FromContextOrDiscard(ctx) if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType != "dns" { - if err := services.AddNodesToNB(ctx, logger, clusterScope); err != nil { - return err + for _, eachMachine := range clusterScope.LinodeMachines.Items { + if err := services.AddNodesToNB(ctx, logger, clusterScope, eachMachine); err != nil { + return err + } } } else { if err := services.EnsureDNSEntries(ctx, clusterScope, "create"); err != nil { diff --git a/controller/linodecluster_controller_test.go b/controller/linodecluster_controller_test.go index b3e5653ef..1b6c63fb8 100644 --- a/controller/linodecluster_controller_test.go +++ b/controller/linodecluster_controller_test.go @@ -227,6 +227,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Call("cluster is created", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient cScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nil + mck.LinodeClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil).AnyTimes() getNB := mck.LinodeClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()). Return(&linodego.NodeBalancer{ ID: nodebalancerID, diff --git a/mock/client.go b/mock/client.go index 3e20c26e5..7d0127be9 100644 --- a/mock/client.go +++ b/mock/client.go @@ -680,6 +680,21 @@ func (mr *MockLinodeClientMockRecorder) ListInstances(ctx, opts any) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListInstances", reflect.TypeOf((*MockLinodeClient)(nil).ListInstances), ctx, opts) } +// ListNodeBalancerNodes mocks base method. +func (m *MockLinodeClient) ListNodeBalancerNodes(ctx context.Context, nodebalancerID, configID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerNode, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListNodeBalancerNodes", ctx, nodebalancerID, configID, opts) + ret0, _ := ret[0].([]linodego.NodeBalancerNode) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListNodeBalancerNodes indicates an expected call of ListNodeBalancerNodes. +func (mr *MockLinodeClientMockRecorder) ListNodeBalancerNodes(ctx, nodebalancerID, configID, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodeBalancerNodes", reflect.TypeOf((*MockLinodeClient)(nil).ListNodeBalancerNodes), ctx, nodebalancerID, configID, opts) +} + // ListPlacementGroups mocks base method. func (m *MockLinodeClient) ListPlacementGroups(ctx context.Context, options *linodego.ListOptions) ([]linodego.PlacementGroup, error) { m.ctrl.T.Helper() @@ -1501,6 +1516,21 @@ func (mr *MockLinodeNodeBalancerClientMockRecorder) GetNodeBalancerConfig(ctx, n return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNodeBalancerConfig", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).GetNodeBalancerConfig), ctx, nodebalancerID, configID) } +// ListNodeBalancerNodes mocks base method. +func (m *MockLinodeNodeBalancerClient) ListNodeBalancerNodes(ctx context.Context, nodebalancerID, configID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerNode, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListNodeBalancerNodes", ctx, nodebalancerID, configID, opts) + ret0, _ := ret[0].([]linodego.NodeBalancerNode) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListNodeBalancerNodes indicates an expected call of ListNodeBalancerNodes. +func (mr *MockLinodeNodeBalancerClientMockRecorder) ListNodeBalancerNodes(ctx, nodebalancerID, configID, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodeBalancerNodes", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).ListNodeBalancerNodes), ctx, nodebalancerID, configID, opts) +} + // MockLinodeObjectStorageClient is a mock of LinodeObjectStorageClient interface. type MockLinodeObjectStorageClient struct { ctrl *gomock.Controller diff --git a/observability/wrappers/linodeclient/linodeclient.gen.go b/observability/wrappers/linodeclient/linodeclient.gen.go index 05a4ed485..319a323af 100644 --- a/observability/wrappers/linodeclient/linodeclient.gen.go +++ b/observability/wrappers/linodeclient/linodeclient.gen.go @@ -1118,6 +1118,33 @@ func (_d LinodeClientWithTracing) ListInstances(ctx context.Context, opts *linod return _d.LinodeClient.ListInstances(ctx, opts) } +// ListNodeBalancerNodes implements clients.LinodeClient +func (_d LinodeClientWithTracing) ListNodeBalancerNodes(ctx context.Context, nodebalancerID int, configID int, opts *linodego.ListOptions) (na1 []linodego.NodeBalancerNode, err error) { + ctx, _span := tracing.Start(ctx, "clients.LinodeClient.ListNodeBalancerNodes") + defer func() { + if _d._spanDecorator != nil { + _d._spanDecorator(_span, map[string]interface{}{ + "ctx": ctx, + "nodebalancerID": nodebalancerID, + "configID": configID, + "opts": opts}, map[string]interface{}{ + "na1": na1, + "err": err}) + } + + if err != nil { + _span.RecordError(err) + _span.SetAttributes( + attribute.String("event", "error"), + attribute.String("message", err.Error()), + ) + } + + _span.End() + }() + return _d.LinodeClient.ListNodeBalancerNodes(ctx, nodebalancerID, configID, opts) +} + // ListPlacementGroups implements clients.LinodeClient func (_d LinodeClientWithTracing) ListPlacementGroups(ctx context.Context, options *linodego.ListOptions) (pa1 []linodego.PlacementGroup, err error) { ctx, _span := tracing.Start(ctx, "clients.LinodeClient.ListPlacementGroups")