Skip to content

Commit e09e016

Browse files
shyamradhakrishnanshyamradhakrishnan
authored
Fix LB reconciliation to support multiple NSG (#260)
1 parent f82ad97 commit e09e016

File tree

4 files changed

+142
-36
lines changed

4 files changed

+142
-36
lines changed

‎cloud/scope/load_balancer_reconciler.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,6 @@ func (s *ClusterScope) CreateLB(ctx context.Context, lb infrastructurev1beta2.Lo
166166
return nil, nil, errors.New("control plane endpoint subnet not provided")
167167
}
168168

169-
if len(controlPlaneEndpointSubnets) > 1 {
170-
return nil, nil, errors.New("cannot have more than 1 control plane endpoint subnet")
171-
}
172169
lbDetails := loadbalancer.CreateLoadBalancerDetails{
173170
CompartmentId: common.String(s.GetCompartmentId()),
174171
DisplayName: common.String(lb.Name),
@@ -182,14 +179,15 @@ func (s *ClusterScope) CreateLB(ctx context.Context, lb infrastructurev1beta2.Lo
182179
FreeformTags: s.GetFreeFormTags(),
183180
DefinedTags: s.GetDefinedTags(),
184181
}
185-
182+
nsgs := make([]string, 0)
186183
for _, nsg := range s.OCIClusterAccessor.GetNetworkSpec().Vcn.NetworkSecurityGroup.List {
187184
if nsg.Role == infrastructurev1beta2.ControlPlaneEndpointRole {
188185
if nsg.ID != nil {
189-
lbDetails.NetworkSecurityGroupIds = []string{*nsg.ID}
186+
nsgs = append(nsgs, *nsg.ID)
190187
}
191188
}
192189
}
190+
lbDetails.NetworkSecurityGroupIds = nsgs
193191

194192
s.Logger.Info("Creating load balancer...")
195193
lbResponse, err := s.LoadBalancerClient.CreateLoadBalancer(ctx, loadbalancer.CreateLoadBalancerRequest{

‎cloud/scope/load_balancer_reconciler_test.go‎

Lines changed: 109 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,8 @@ func TestLBReconciliation(t *testing.T) {
217217
},
218218
},
219219
{
220-
name: "more than one cp subnet",
221-
errorExpected: true,
222-
matchError: errors.New("cannot have more than 1 control plane endpoint subnet"),
220+
name: "create load balancer more than one subnet",
221+
errorExpected: false,
223222
testSpecificSetup: func(clusterScope *ClusterScope, lbClient *mock_lb.MockLoadBalancerClient) {
224223
clusterScope.OCIClusterAccessor.GetNetworkSpec().Vcn.Subnets = []*infrastructurev1beta2.Subnet{
225224
{
@@ -231,11 +230,89 @@ func TestLBReconciliation(t *testing.T) {
231230
ID: common.String("s2"),
232231
},
233232
}
233+
clusterScope.OCIClusterAccessor.GetNetworkSpec().Vcn.NetworkSecurityGroup = infrastructurev1beta2.NetworkSecurityGroup{
234+
List: []*infrastructurev1beta2.NSG{
235+
{
236+
Role: infrastructurev1beta2.ControlPlaneEndpointRole,
237+
ID: common.String("nsg1"),
238+
},
239+
{
240+
Role: infrastructurev1beta2.ControlPlaneEndpointRole,
241+
ID: common.String("nsg2"),
242+
},
243+
},
244+
}
245+
definedTags, definedTagsInterface := getDefinedTags()
246+
ociClusterAccessor.OCICluster.Spec.DefinedTags = definedTags
234247
lbClient.EXPECT().ListLoadBalancers(gomock.Any(), gomock.Eq(loadbalancer.ListLoadBalancersRequest{
235248
CompartmentId: common.String("compartment-id"),
236249
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
250+
})).Return(loadbalancer.ListLoadBalancersResponse{
251+
Items: []loadbalancer.LoadBalancer{},
252+
}, nil)
253+
lbClient.EXPECT().CreateLoadBalancer(gomock.Any(), gomock.Eq(loadbalancer.CreateLoadBalancerRequest{
254+
CreateLoadBalancerDetails: loadbalancer.CreateLoadBalancerDetails{
255+
CompartmentId: common.String("compartment-id"),
256+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
257+
SubnetIds: []string{"s1", "s2"},
258+
NetworkSecurityGroupIds: []string{"nsg1", "nsg2"},
259+
IsPrivate: common.Bool(false),
260+
ShapeName: common.String("flexible"),
261+
ShapeDetails: &loadbalancer.ShapeDetails{MaximumBandwidthInMbps: common.Int(100),
262+
MinimumBandwidthInMbps: common.Int(10)},
263+
Listeners: map[string]loadbalancer.ListenerDetails{
264+
APIServerLBListener: {
265+
Protocol: common.String("TCP"),
266+
Port: common.Int(int(6443)),
267+
DefaultBackendSetName: common.String(APIServerLBBackendSetName),
268+
},
269+
},
270+
BackendSets: map[string]loadbalancer.BackendSetDetails{
271+
APIServerLBBackendSetName: loadbalancer.BackendSetDetails{
272+
Policy: common.String("ROUND_ROBIN"),
273+
274+
HealthChecker: &loadbalancer.HealthCheckerDetails{
275+
Port: common.Int(6443),
276+
Protocol: common.String("TCP"),
277+
},
278+
Backends: []loadbalancer.BackendDetails{},
279+
},
280+
},
281+
FreeformTags: tags,
282+
DefinedTags: definedTagsInterface,
283+
},
284+
OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-lb", string("resource_uid")),
237285
})).
238-
Return(loadbalancer.ListLoadBalancersResponse{}, nil)
286+
Return(loadbalancer.CreateLoadBalancerResponse{
287+
OpcWorkRequestId: common.String("opc-wr-id"),
288+
}, nil)
289+
lbClient.EXPECT().GetWorkRequest(gomock.Any(), gomock.Eq(loadbalancer.GetWorkRequestRequest{
290+
WorkRequestId: common.String("opc-wr-id"),
291+
})).Return(loadbalancer.GetWorkRequestResponse{
292+
WorkRequest: loadbalancer.WorkRequest{
293+
LoadBalancerId: common.String("lb-id"),
294+
LifecycleState: loadbalancer.WorkRequestLifecycleStateSucceeded,
295+
},
296+
}, nil)
297+
298+
lbClient.EXPECT().GetLoadBalancer(gomock.Any(), gomock.Eq(loadbalancer.GetLoadBalancerRequest{
299+
LoadBalancerId: common.String("lb-id"),
300+
})).
301+
Return(loadbalancer.GetLoadBalancerResponse{
302+
LoadBalancer: loadbalancer.LoadBalancer{
303+
Id: common.String("lb-id"),
304+
FreeformTags: tags,
305+
IsPrivate: common.Bool(false),
306+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
307+
IpAddresses: []loadbalancer.IpAddress{
308+
{
309+
IpAddress: common.String("2.2.2.2"),
310+
IsPublic: common.Bool(true),
311+
},
312+
},
313+
},
314+
}, nil)
315+
239316
},
240317
},
241318
{
@@ -248,6 +325,18 @@ func TestLBReconciliation(t *testing.T) {
248325
ID: common.String("s1"),
249326
},
250327
}
328+
clusterScope.OCIClusterAccessor.GetNetworkSpec().Vcn.NetworkSecurityGroup = infrastructurev1beta2.NetworkSecurityGroup{
329+
List: []*infrastructurev1beta2.NSG{
330+
{
331+
Role: infrastructurev1beta2.ControlPlaneEndpointRole,
332+
ID: common.String("nsg1"),
333+
},
334+
{
335+
Role: infrastructurev1beta2.ControlPlaneEndpointRole,
336+
ID: common.String("nsg2"),
337+
},
338+
},
339+
}
251340
definedTags, definedTagsInterface := getDefinedTags()
252341
ociClusterAccessor.OCICluster.Spec.DefinedTags = definedTags
253342
lbClient.EXPECT().ListLoadBalancers(gomock.Any(), gomock.Eq(loadbalancer.ListLoadBalancersRequest{
@@ -258,11 +347,12 @@ func TestLBReconciliation(t *testing.T) {
258347
}, nil)
259348
lbClient.EXPECT().CreateLoadBalancer(gomock.Any(), gomock.Eq(loadbalancer.CreateLoadBalancerRequest{
260349
CreateLoadBalancerDetails: loadbalancer.CreateLoadBalancerDetails{
261-
CompartmentId: common.String("compartment-id"),
262-
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
263-
SubnetIds: []string{"s1"},
264-
IsPrivate: common.Bool(false),
265-
ShapeName: common.String("flexible"),
350+
CompartmentId: common.String("compartment-id"),
351+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
352+
SubnetIds: []string{"s1"},
353+
NetworkSecurityGroupIds: []string{"nsg1", "nsg2"},
354+
IsPrivate: common.Bool(false),
355+
ShapeName: common.String("flexible"),
266356
ShapeDetails: &loadbalancer.ShapeDetails{MaximumBandwidthInMbps: common.Int(100),
267357
MinimumBandwidthInMbps: common.Int(10)},
268358
Listeners: map[string]loadbalancer.ListenerDetails{
@@ -340,10 +430,11 @@ func TestLBReconciliation(t *testing.T) {
340430
}, nil)
341431
lbClient.EXPECT().CreateLoadBalancer(gomock.Any(), gomock.Eq(loadbalancer.CreateLoadBalancerRequest{
342432
CreateLoadBalancerDetails: loadbalancer.CreateLoadBalancerDetails{
343-
CompartmentId: common.String("compartment-id"),
344-
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
345-
SubnetIds: []string{"s1"},
346-
ShapeName: common.String("flexible"),
433+
CompartmentId: common.String("compartment-id"),
434+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
435+
SubnetIds: []string{"s1"},
436+
NetworkSecurityGroupIds: make([]string, 0),
437+
ShapeName: common.String("flexible"),
347438
ShapeDetails: &loadbalancer.ShapeDetails{MaximumBandwidthInMbps: common.Int(100),
348439
MinimumBandwidthInMbps: common.Int(10)},
349440
IsPrivate: common.Bool(false),
@@ -391,10 +482,11 @@ func TestLBReconciliation(t *testing.T) {
391482
Return(loadbalancer.ListLoadBalancersResponse{}, nil)
392483
lbClient.EXPECT().CreateLoadBalancer(gomock.Any(), gomock.Eq(loadbalancer.CreateLoadBalancerRequest{
393484
CreateLoadBalancerDetails: loadbalancer.CreateLoadBalancerDetails{
394-
CompartmentId: common.String("compartment-id"),
395-
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
396-
SubnetIds: []string{"s1"},
397-
ShapeName: common.String("flexible"),
485+
CompartmentId: common.String("compartment-id"),
486+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
487+
SubnetIds: []string{"s1"},
488+
NetworkSecurityGroupIds: make([]string, 0),
489+
ShapeName: common.String("flexible"),
398490
ShapeDetails: &loadbalancer.ShapeDetails{MaximumBandwidthInMbps: common.Int(100),
399491
MinimumBandwidthInMbps: common.Int(10)},
400492
IsPrivate: common.Bool(false),

‎cloud/scope/network_load_balancer_reconciler.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,15 @@ func (s *ClusterScope) CreateNLB(ctx context.Context, lb infrastructurev1beta2.L
183183
FreeformTags: s.GetFreeFormTags(),
184184
DefinedTags: s.GetDefinedTags(),
185185
}
186-
186+
nsgs := make([]string, 0)
187187
for _, nsg := range s.OCIClusterAccessor.GetNetworkSpec().Vcn.NetworkSecurityGroup.List {
188188
if nsg.Role == infrastructurev1beta2.ControlPlaneEndpointRole {
189189
if nsg.ID != nil {
190-
nlbDetails.NetworkSecurityGroupIds = []string{*nsg.ID}
190+
nsgs = append(nsgs, *nsg.ID)
191191
}
192192
}
193193
}
194+
nlbDetails.NetworkSecurityGroupIds = nsgs
194195

195196
s.Logger.Info("Creating network load balancer")
196197
nlbResponse, err := s.NetworkLoadBalancerClient.CreateNetworkLoadBalancer(ctx, networkloadbalancer.CreateNetworkLoadBalancerRequest{

‎cloud/scope/network_load_balancer_reconciler_test.go‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,18 @@ func TestNLBReconciliation(t *testing.T) {
250250
ID: common.String("s1"),
251251
},
252252
}
253+
clusterScope.OCIClusterAccessor.GetNetworkSpec().Vcn.NetworkSecurityGroup = infrastructurev1beta2.NetworkSecurityGroup{
254+
List: []*infrastructurev1beta2.NSG{
255+
{
256+
Role: infrastructurev1beta2.ControlPlaneEndpointRole,
257+
ID: common.String("nsg1"),
258+
},
259+
{
260+
Role: infrastructurev1beta2.ControlPlaneEndpointRole,
261+
ID: common.String("nsg2"),
262+
},
263+
},
264+
}
253265
definedTags, definedTagsInterface := getDefinedTags()
254266
ociClusterAccessor.OCICluster.Spec.DefinedTags = definedTags
255267
nlbClient.EXPECT().ListNetworkLoadBalancers(gomock.Any(), gomock.Eq(networkloadbalancer.ListNetworkLoadBalancersRequest{
@@ -259,10 +271,11 @@ func TestNLBReconciliation(t *testing.T) {
259271
Return(networkloadbalancer.ListNetworkLoadBalancersResponse{}, nil)
260272
nlbClient.EXPECT().CreateNetworkLoadBalancer(gomock.Any(), gomock.Eq(networkloadbalancer.CreateNetworkLoadBalancerRequest{
261273
CreateNetworkLoadBalancerDetails: networkloadbalancer.CreateNetworkLoadBalancerDetails{
262-
CompartmentId: common.String("compartment-id"),
263-
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
264-
SubnetId: common.String("s1"),
265-
IsPrivate: common.Bool(false),
274+
CompartmentId: common.String("compartment-id"),
275+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
276+
SubnetId: common.String("s1"),
277+
IsPrivate: common.Bool(false),
278+
NetworkSecurityGroupIds: []string{"nsg1", "nsg2"},
266279
Listeners: map[string]networkloadbalancer.ListenerDetails{
267280
APIServerLBListener: {
268281
Protocol: networkloadbalancer.ListenerProtocolsTcp,
@@ -342,10 +355,11 @@ func TestNLBReconciliation(t *testing.T) {
342355
Return(networkloadbalancer.ListNetworkLoadBalancersResponse{}, nil)
343356
nlbClient.EXPECT().CreateNetworkLoadBalancer(gomock.Any(), gomock.Eq(networkloadbalancer.CreateNetworkLoadBalancerRequest{
344357
CreateNetworkLoadBalancerDetails: networkloadbalancer.CreateNetworkLoadBalancerDetails{
345-
CompartmentId: common.String("compartment-id"),
346-
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
347-
SubnetId: common.String("s1"),
348-
IsPrivate: common.Bool(false),
358+
CompartmentId: common.String("compartment-id"),
359+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
360+
SubnetId: common.String("s1"),
361+
IsPrivate: common.Bool(false),
362+
NetworkSecurityGroupIds: make([]string, 0),
349363
Listeners: map[string]networkloadbalancer.ListenerDetails{
350364
APIServerLBListener: {
351365
Protocol: networkloadbalancer.ListenerProtocolsTcp,
@@ -394,10 +408,11 @@ func TestNLBReconciliation(t *testing.T) {
394408
Return(networkloadbalancer.ListNetworkLoadBalancersResponse{}, nil)
395409
nlbClient.EXPECT().CreateNetworkLoadBalancer(gomock.Any(), gomock.Eq(networkloadbalancer.CreateNetworkLoadBalancerRequest{
396410
CreateNetworkLoadBalancerDetails: networkloadbalancer.CreateNetworkLoadBalancerDetails{
397-
CompartmentId: common.String("compartment-id"),
398-
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
399-
SubnetId: common.String("s1"),
400-
IsPrivate: common.Bool(false),
411+
CompartmentId: common.String("compartment-id"),
412+
DisplayName: common.String(fmt.Sprintf("%s-%s", "cluster", "apiserver")),
413+
SubnetId: common.String("s1"),
414+
NetworkSecurityGroupIds: make([]string, 0),
415+
IsPrivate: common.Bool(false),
401416
Listeners: map[string]networkloadbalancer.ListenerDetails{
402417
APIServerLBListener: {
403418
Protocol: networkloadbalancer.ListenerProtocolsTcp,

0 commit comments

Comments
 (0)