Skip to content

Commit 7db8837

Browse files
AshleyDumaineAshleyDumaine
authored
[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
1 parent e039a73 commit 7db8837

File tree

7 files changed

+273
-200
lines changed

7 files changed

+273
-200
lines changed

‎clients/clients.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ type LinodeVPCClient interface {
4848

4949
// LinodeNodeBalancerClient defines the methods that interact with Linode's Node Balancer service.
5050
type LinodeNodeBalancerClient interface {
51-
ListNodeBalancers(ctx context.Context, opts *linodego.ListOptions) ([]linodego.NodeBalancer, error)
5251
CreateNodeBalancer(ctx context.Context, opts linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error)
52+
GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error)
53+
GetNodeBalancerConfig(ctx context.Context, nodebalancerID int, configID int) (*linodego.NodeBalancerConfig, error)
5354
CreateNodeBalancerConfig(ctx context.Context, nodebalancerID int, opts linodego.NodeBalancerConfigCreateOptions) (*linodego.NodeBalancerConfig, error)
5455
DeleteNodeBalancerNode(ctx context.Context, nodebalancerID int, configID int, nodeID int) error
5556
DeleteNodeBalancer(ctx context.Context, nodebalancerID int) error

‎cloud/services/loadbalancers.go

Lines changed: 39 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"errors"
66
"fmt"
77
"net/http"
8-
"slices"
98

109
"github.com/go-logr/logr"
1110
"github.com/linode/linodego"
@@ -20,85 +19,69 @@ const (
2019
DefaultKonnectivityLBPort = 8132
2120
)
2221

23-
// CreateNodeBalancer creates a new NodeBalancer if one doesn't exist
24-
func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) {
25-
var linodeNB *linodego.NodeBalancer
26-
27-
NBLabel := clusterScope.LinodeCluster.Name
28-
clusterUID := string(clusterScope.LinodeCluster.UID)
29-
tags := []string{string(clusterScope.LinodeCluster.UID)}
30-
listFilter := util.Filter{
31-
ID: clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
32-
Label: NBLabel,
33-
Tags: tags,
34-
}
35-
filter, err := listFilter.String()
36-
if err != nil {
37-
return nil, err
38-
}
39-
linodeNBs, err := clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, filter))
40-
if err != nil {
41-
logger.Info("Failed to list NodeBalancers", "error", err.Error())
42-
43-
return nil, err
44-
}
45-
if len(linodeNBs) == 1 {
46-
logger.Info(fmt.Sprintf("NodeBalancer %s already exists", *linodeNBs[0].Label))
47-
if !slices.Contains(linodeNBs[0].Tags, clusterUID) {
48-
err = errors.New("NodeBalancer conflict")
49-
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]))
22+
// EnsureNodeBalancer creates a new NodeBalancer if one doesn't exist or returns the existing NodeBalancer
23+
func EnsureNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) {
24+
nbID := clusterScope.LinodeCluster.Spec.Network.NodeBalancerID
25+
if nbID != nil && *nbID != 0 {
26+
res, err := clusterScope.LinodeClient.GetNodeBalancer(ctx, *nbID)
27+
if err != nil {
28+
logger.Info("Failed to get NodeBalancer", "error", err.Error())
5029

5130
return nil, err
5231
}
53-
54-
return &linodeNBs[0], nil
32+
return res, nil
5533
}
5634

5735
logger.Info(fmt.Sprintf("Creating NodeBalancer %s", clusterScope.LinodeCluster.Name))
5836
createConfig := linodego.NodeBalancerCreateOptions{
5937
Label: util.Pointer(clusterScope.LinodeCluster.Name),
6038
Region: clusterScope.LinodeCluster.Spec.Region,
61-
Tags: tags,
39+
Tags: []string{string(clusterScope.LinodeCluster.UID)},
6240
}
6341

64-
linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig)
65-
if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
66-
return nil, err
67-
}
68-
if linodeNB != nil {
69-
logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label)
70-
}
71-
72-
return linodeNB, nil
42+
return clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig)
7343
}
7444

75-
// CreateNodeBalancerConfigs creates NodeBalancer configs if it does not exist
76-
func CreateNodeBalancerConfigs(
45+
// EnsureNodeBalancerConfigs creates NodeBalancer configs if it does not exist or returns the existing NodeBalancerConfig
46+
func EnsureNodeBalancerConfigs(
7747
ctx context.Context,
7848
clusterScope *scope.ClusterScope,
7949
logger logr.Logger,
8050
) ([]*linodego.NodeBalancerConfig, error) {
8151
nbConfigs := []*linodego.NodeBalancerConfig{}
52+
var apiserverLinodeNBConfig *linodego.NodeBalancerConfig
53+
var err error
8254
apiLBPort := DefaultApiserverLBPort
8355
if clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 {
8456
apiLBPort = clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort
8557
}
86-
apiserverCreateConfig := linodego.NodeBalancerConfigCreateOptions{
87-
Port: apiLBPort,
88-
Protocol: linodego.ProtocolTCP,
89-
Algorithm: linodego.AlgorithmRoundRobin,
90-
Check: linodego.CheckConnection,
91-
}
9258

93-
apiserverLinodeNBConfig, err := clusterScope.LinodeClient.CreateNodeBalancerConfig(
94-
ctx,
95-
*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
96-
apiserverCreateConfig,
97-
)
98-
if err != nil {
99-
logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error())
100-
return nil, err
59+
if clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID != nil {
60+
apiserverLinodeNBConfig, err = clusterScope.LinodeClient.GetNodeBalancerConfig(
61+
ctx,
62+
*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
63+
*clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID)
64+
if err != nil {
65+
logger.Info("Failed to get Linode NodeBalancer config", "error", err.Error())
66+
return nil, err
67+
}
68+
} else {
69+
apiserverLinodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(
70+
ctx,
71+
*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
72+
linodego.NodeBalancerConfigCreateOptions{
73+
Port: apiLBPort,
74+
Protocol: linodego.ProtocolTCP,
75+
Algorithm: linodego.AlgorithmRoundRobin,
76+
Check: linodego.CheckConnection,
77+
},
78+
)
79+
if err != nil {
80+
logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error())
81+
return nil, err
82+
}
10183
}
84+
10285
nbConfigs = append(nbConfigs, apiserverLinodeNBConfig)
10386

10487
// return if additional ports should not be configured
@@ -180,11 +163,6 @@ func AddNodeToNB(
180163
return err
181164
}
182165

183-
// return if additional ports should not be configured
184-
if len(machineScope.LinodeCluster.Spec.Network.AdditionalPorts) == 0 {
185-
return nil
186-
}
187-
188166
for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts {
189167
_, err = machineScope.LinodeClient.CreateNodeBalancerNode(
190168
ctx,
@@ -234,10 +212,6 @@ func DeleteNodeFromNB(
234212
return err
235213
}
236214

237-
if len(machineScope.LinodeCluster.Spec.Network.AdditionalPorts) == 0 {
238-
return nil
239-
}
240-
241215
for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts {
242216
err = machineScope.LinodeClient.DeleteNodeBalancerNode(
243217
ctx,

‎cloud/services/loadbalancers_test.go

Lines changed: 56 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
"github.com/linode/cluster-api-provider-linode/mock"
2020
)
2121

22-
func TestCreateNodeBalancer(t *testing.T) {
22+
func TestEnsureNodeBalancer(t *testing.T) {
2323
t.Parallel()
2424
tests := []struct {
2525
name string
@@ -50,8 +50,7 @@ func TestCreateNodeBalancer(t *testing.T) {
5050
},
5151
},
5252
expects: func(mockClient *mock.MockLinodeClient) {
53-
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{}, nil)
54-
mockClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{
53+
mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{
5554
ID: 1234,
5655
}, nil)
5756
},
@@ -60,7 +59,7 @@ func TestCreateNodeBalancer(t *testing.T) {
6059
},
6160
},
6261
{
63-
name: "Success - List NodeBalancers returns one nodebalancer and we return that",
62+
name: "Success - Get NodeBalancers returns one nodebalancer and we return that",
6463
clusterScope: &scope.ClusterScope{
6564
LinodeCluster: &infrav1alpha2.LinodeCluster{
6665
ObjectMeta: metav1.ObjectMeta{
@@ -75,12 +74,10 @@ func TestCreateNodeBalancer(t *testing.T) {
7574
},
7675
},
7776
expects: func(mockClient *mock.MockLinodeClient) {
78-
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{
79-
{
80-
ID: 1234,
81-
Label: ptr.To("test"),
82-
Tags: []string{"test-uid"},
83-
},
77+
mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{
78+
ID: 1234,
79+
Label: ptr.To("test"),
80+
Tags: []string{"test-uid"},
8481
}, nil)
8582
},
8683
expectedNodeBalancer: &linodego.NodeBalancer{
@@ -90,38 +87,7 @@ func TestCreateNodeBalancer(t *testing.T) {
9087
},
9188
},
9289
{
93-
name: "Error - List NodeBalancers returns one nodebalancer but there is a nodebalancer conflict",
94-
clusterScope: &scope.ClusterScope{
95-
LinodeCluster: &infrav1alpha2.LinodeCluster{
96-
ObjectMeta: metav1.ObjectMeta{
97-
Name: "test-cluster",
98-
UID: "test-uid",
99-
},
100-
Spec: infrav1alpha2.LinodeClusterSpec{
101-
Network: infrav1alpha2.NetworkSpec{
102-
NodeBalancerID: ptr.To(1234),
103-
},
104-
},
105-
},
106-
},
107-
expects: func(mockClient *mock.MockLinodeClient) {
108-
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{
109-
{
110-
ID: 1234,
111-
Label: ptr.To("test"),
112-
Tags: []string{"test"},
113-
},
114-
}, nil)
115-
},
116-
expectedNodeBalancer: &linodego.NodeBalancer{
117-
ID: 1234,
118-
Label: ptr.To("test"),
119-
Tags: []string{"test"},
120-
},
121-
expectedError: fmt.Errorf("NodeBalancer conflict"),
122-
},
123-
{
124-
name: "Error - List NodeBalancers returns an error",
90+
name: "Error - Get NodeBalancer returns an error",
12591
clusterScope: &scope.ClusterScope{
12692
LinodeCluster: &infrav1alpha2.LinodeCluster{
12793
ObjectMeta: metav1.ObjectMeta{
@@ -136,9 +102,9 @@ func TestCreateNodeBalancer(t *testing.T) {
136102
},
137103
},
138104
expects: func(mockClient *mock.MockLinodeClient) {
139-
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to list NodeBalancers"))
105+
mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to get NodeBalancer"))
140106
},
141-
expectedError: fmt.Errorf("Unable to list NodeBalancers"),
107+
expectedError: fmt.Errorf("Unable to get NodeBalancer"),
142108
},
143109
{
144110
name: "Error - Create NodeBalancer returns an error",
@@ -148,15 +114,10 @@ func TestCreateNodeBalancer(t *testing.T) {
148114
Name: "test-cluster",
149115
UID: "test-uid",
150116
},
151-
Spec: infrav1alpha2.LinodeClusterSpec{
152-
Network: infrav1alpha2.NetworkSpec{
153-
NodeBalancerID: ptr.To(1234),
154-
},
155-
},
117+
Spec: infrav1alpha2.LinodeClusterSpec{},
156118
},
157119
},
158120
expects: func(mockClient *mock.MockLinodeClient) {
159-
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{}, nil)
160121
mockClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to create NodeBalancer"))
161122
},
162123
expectedError: fmt.Errorf("Unable to create NodeBalancer"),
@@ -176,7 +137,7 @@ func TestCreateNodeBalancer(t *testing.T) {
176137

177138
testcase.expects(MockLinodeClient)
178139

179-
got, err := CreateNodeBalancer(context.Background(), testcase.clusterScope, logr.Discard())
140+
got, err := EnsureNodeBalancer(context.Background(), testcase.clusterScope, logr.Discard())
180141
if testcase.expectedError != nil {
181142
assert.ErrorContains(t, err, testcase.expectedError.Error())
182143
} else {
@@ -187,7 +148,7 @@ func TestCreateNodeBalancer(t *testing.T) {
187148
}
188149
}
189150

190-
func TestCreateNodeBalancerConfigs(t *testing.T) {
151+
func TestEnsureNodeBalancerConfigs(t *testing.T) {
191152
t.Parallel()
192153

193154
tests := []struct {
@@ -232,6 +193,48 @@ func TestCreateNodeBalancerConfigs(t *testing.T) {
232193
}, nil)
233194
},
234195
},
196+
{
197+
name: "Success - Get NodeBalancerConfig",
198+
clusterScope: &scope.ClusterScope{
199+
LinodeClient: nil,
200+
LinodeCluster: &infrav1alpha2.LinodeCluster{
201+
ObjectMeta: metav1.ObjectMeta{
202+
Name: "test-cluster",
203+
UID: "test-uid",
204+
},
205+
Spec: infrav1alpha2.LinodeClusterSpec{
206+
Network: infrav1alpha2.NetworkSpec{
207+
NodeBalancerID: ptr.To(1234),
208+
ApiserverNodeBalancerConfigID: ptr.To(2),
209+
},
210+
ControlPlaneEndpoint: clusterv1.APIEndpoint{
211+
Host: "",
212+
Port: 0,
213+
},
214+
},
215+
},
216+
},
217+
expectedConfigs: []*linodego.NodeBalancerConfig{
218+
{
219+
Port: DefaultApiserverLBPort,
220+
Protocol: linodego.ProtocolTCP,
221+
Algorithm: linodego.AlgorithmRoundRobin,
222+
Check: linodego.CheckConnection,
223+
NodeBalancerID: 1234,
224+
ID: 2,
225+
},
226+
},
227+
expects: func(mockClient *mock.MockLinodeClient) {
228+
mockClient.EXPECT().GetNodeBalancerConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancerConfig{
229+
ID: 2,
230+
Port: DefaultApiserverLBPort,
231+
Protocol: linodego.ProtocolTCP,
232+
Algorithm: linodego.AlgorithmRoundRobin,
233+
Check: linodego.CheckConnection,
234+
NodeBalancerID: 1234,
235+
}, nil)
236+
},
237+
},
235238
{
236239
name: "Success - Create NodeBalancerConfig using assigned LB ports",
237240
clusterScope: &scope.ClusterScope{
@@ -396,7 +399,7 @@ func TestCreateNodeBalancerConfigs(t *testing.T) {
396399

397400
testcase.expects(MockLinodeClient)
398401

399-
got, err := CreateNodeBalancerConfigs(context.Background(), testcase.clusterScope, logr.Discard())
402+
got, err := EnsureNodeBalancerConfigs(context.Background(), testcase.clusterScope, logr.Discard())
400403
if testcase.expectedError != nil {
401404
assert.ErrorContains(t, err, testcase.expectedError.Error())
402405
} else {

0 commit comments

Comments
 (0)