Skip to content

Commit 3944724

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#674 from CecileRobertMichon/provider-slb
🐛 Fix nodes outbound connectivity with SLB
2 parents fd8ead6 + 244677a commit 3944724

File tree

14 files changed

+522
-226
lines changed

14 files changed

+522
-226
lines changed

‎api/v1alpha3/tags.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,20 +104,23 @@ const (
104104
// dedicated to this cluster api provider implementation.
105105
NameAzureClusterAPIRole = NameAzureProviderPrefix + "role"
106106

107-
// APIServerRoleTagValue describes the value for the apiserver role
108-
APIServerRoleTagValue = "apiserver"
107+
// APIServerRole describes the value for the apiserver role
108+
APIServerRole = "apiserver"
109109

110-
// BastionRoleTagValue describes the value for the bastion role
111-
BastionRoleTagValue = "bastion"
110+
// NodeOutboundRole describes the value for the node outbound LB role
111+
NodeOutboundRole = "nodeOutbound"
112112

113-
// CommonRoleTagValue describes the value for the common role
114-
CommonRoleTagValue = "common"
113+
// BastionRole describes the value for the bastion role
114+
BastionRole = "bastion"
115115

116-
// PublicRoleTagValue describes the value for the public role
117-
PublicRoleTagValue = "public"
116+
// CommonRole describes the value for the common role
117+
CommonRole = "common"
118118

119-
// PrivateRoleTagValue describes the value for the private role
120-
PrivateRoleTagValue = "private"
119+
// PublicRole describes the value for the public role
120+
PublicRole = "public"
121+
122+
// PrivateRole describes the value for the private role
123+
PrivateRole = "private"
121124
)
122125

123126
// ClusterTagKey generates the key for resources associated with a cluster.

‎cloud/defaults.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ func GeneratePublicIPName(clusterName, hash string) string {
7979
return fmt.Sprintf("%s-%s", clusterName, hash)
8080
}
8181

82+
// GenerateNodeOutboundIPName generates a public IP name, based on the cluster name.
83+
func GenerateNodeOutboundIPName(clusterName string) string {
84+
return fmt.Sprintf("pip-%s-node-outbound", clusterName)
85+
}
86+
8287
// GenerateNodePublicIPName generates a node public IP name, based on the NIC name.
8388
func GenerateNodePublicIPName(nicName string) string {
8489
return fmt.Sprintf("%s-public-ip", nicName)

‎cloud/services/groups/groups.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
5151
ClusterName: s.Scope.Name(),
5252
Lifecycle: infrav1.ResourceLifecycleOwned,
5353
Name: to.StringPtr(s.Scope.ResourceGroup()),
54-
Role: to.StringPtr(infrav1.CommonRoleTagValue),
54+
Role: to.StringPtr(infrav1.CommonRole),
5555
Additional: s.Scope.AdditionalTags(),
5656
})),
5757
}

‎cloud/services/networkinterfaces/networkinterfaces.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@ import (
2525
"github.com/Azure/go-autorest/autorest/to"
2626
"github.com/pkg/errors"
2727
"k8s.io/klog"
28+
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3"
2829
azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
2930
)
3031

3132
// Spec specification for routetable
3233
type Spec struct {
3334
Name string
35+
MachineRole string
3436
SubnetName string
3537
VnetName string
3638
StaticIPAddress string
@@ -77,27 +79,28 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
7779

7880
backendAddressPools := []network.BackendAddressPool{}
7981
if nicSpec.PublicLoadBalancerName != "" {
80-
// only control planes have an attached public LB
8182
lb, lberr := s.PublicLoadBalancersClient.Get(ctx, s.Scope.ResourceGroup(), nicSpec.PublicLoadBalancerName)
8283
if lberr != nil {
83-
return errors.Wrap(lberr, "failed to get publicLB")
84+
return errors.Wrap(lberr, "failed to get public LB")
8485
}
8586

8687
backendAddressPools = append(backendAddressPools,
8788
network.BackendAddressPool{
8889
ID: (*lb.BackendAddressPools)[0].ID,
8990
})
9091

91-
ruleName := s.MachineScope.Name()
92-
naterr := s.createInboundNatRule(ctx, lb, ruleName)
93-
if naterr != nil {
94-
return errors.Wrap(naterr, "failed to create NAT rule")
95-
}
92+
if nicSpec.MachineRole == infrav1.ControlPlane {
93+
ruleName := s.MachineScope.Name()
94+
naterr := s.createInboundNatRule(ctx, lb, ruleName)
95+
if naterr != nil {
96+
return errors.Wrap(naterr, "failed to create NAT rule")
97+
}
9698

97-
nicConfig.LoadBalancerInboundNatRules = &[]network.InboundNatRule{
98-
{
99-
ID: to.StringPtr(fmt.Sprintf("%s/inboundNatRules/%s", to.String(lb.ID), ruleName)),
100-
},
99+
nicConfig.LoadBalancerInboundNatRules = &[]network.InboundNatRule{
100+
{
101+
ID: to.StringPtr(fmt.Sprintf("%s/inboundNatRules/%s", to.String(lb.ID), ruleName)),
102+
},
103+
}
101104
}
102105
}
103106
if nicSpec.InternalLoadBalancerName != "" {

‎cloud/services/networkinterfaces/networkinterfaces_test.go‎

Lines changed: 80 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,11 @@ func TestReconcileNetworkInterface(t *testing.T) {
244244
{
245245
name: "get subnets fails",
246246
netInterfaceSpec: Spec{
247-
Name: "my-net-interface",
248-
VnetName: "my-vnet",
249-
SubnetName: "my-subnet",
247+
Name: "my-net-interface",
248+
VnetName: "my-vnet",
249+
SubnetName: "my-subnet",
250+
PublicLoadBalancerName: "my-cluster",
251+
MachineRole: infrav1.Node,
250252
},
251253
expectedError: "failed to get subnets: #: Internal Server Error: StatusCode=500",
252254
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -263,9 +265,11 @@ func TestReconcileNetworkInterface(t *testing.T) {
263265
{
264266
name: "node network interface create fails",
265267
netInterfaceSpec: Spec{
266-
Name: "my-net-interface",
267-
VnetName: "my-vnet",
268-
SubnetName: "my-subnet",
268+
Name: "my-net-interface",
269+
VnetName: "my-vnet",
270+
SubnetName: "my-subnet",
271+
PublicLoadBalancerName: "my-cluster",
272+
MachineRole: infrav1.Node,
269273
},
270274
expectedError: "failed to create network interface my-net-interface in resource group my-rg: #: Internal Server Error: StatusCode=500",
271275
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -279,17 +283,20 @@ func TestReconcileNetworkInterface(t *testing.T) {
279283
gomock.InOrder(
280284
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").
281285
Return(network.Subnet{}, nil),
286+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
282287
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})).
283288
Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")))
284289
},
285290
},
286291
{
287292
name: "node network interface with Static private IP successfully created",
288293
netInterfaceSpec: Spec{
289-
Name: "my-net-interface",
290-
VnetName: "my-vnet",
291-
SubnetName: "my-subnet",
292-
StaticIPAddress: "1.2.3.4",
294+
Name: "my-net-interface",
295+
VnetName: "my-vnet",
296+
SubnetName: "my-subnet",
297+
StaticIPAddress: "1.2.3.4",
298+
PublicLoadBalancerName: "my-cluster",
299+
MachineRole: infrav1.Node,
293300
},
294301
expectedError: "",
295302
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -302,15 +309,18 @@ func TestReconcileNetworkInterface(t *testing.T) {
302309
mResourceSku.EXPECT().HasAcceleratedNetworking(gomock.Any(), gomock.Any())
303310
gomock.InOrder(
304311
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil),
312+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
305313
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})))
306314
},
307315
},
308316
{
309317
name: "node network interface with Dynamic private IP successfully created",
310318
netInterfaceSpec: Spec{
311-
Name: "my-net-interface",
312-
VnetName: "my-vnet",
313-
SubnetName: "my-subnet",
319+
Name: "my-net-interface",
320+
VnetName: "my-vnet",
321+
SubnetName: "my-subnet",
322+
PublicLoadBalancerName: "my-cluster",
323+
MachineRole: infrav1.Node,
314324
},
315325
expectedError: "",
316326
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -323,6 +333,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
323333
mResourceSku.EXPECT().HasAcceleratedNetworking(gomock.Any(), gomock.Any())
324334
gomock.InOrder(
325335
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil),
336+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
326337
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})))
327338
},
328339
},
@@ -334,6 +345,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
334345
SubnetName: "my-subnet",
335346
PublicLoadBalancerName: "my-publiclb",
336347
InternalLoadBalancerName: "my-internal-lb",
348+
MachineRole: infrav1.ControlPlane,
337349
},
338350
expectedError: "",
339351
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -413,8 +425,9 @@ func TestReconcileNetworkInterface(t *testing.T) {
413425
SubnetName: "my-subnet",
414426
PublicLoadBalancerName: "my-publiclb",
415427
InternalLoadBalancerName: "my-internal-lb",
428+
MachineRole: infrav1.ControlPlane,
416429
},
417-
expectedError: "failed to get publicLB: #: Internal Server Error: StatusCode=500",
430+
expectedError: "failed to get public LB: #: Internal Server Error: StatusCode=500",
418431
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
419432
mSubnet *mock_subnets.MockClientMockRecorder,
420433
mPublicLoadBalancer *mock_publicloadbalancers.MockClientMockRecorder,
@@ -436,6 +449,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
436449
SubnetName: "my-subnet",
437450
PublicLoadBalancerName: "my-publiclb",
438451
InternalLoadBalancerName: "my-internal-lb",
452+
MachineRole: infrav1.ControlPlane,
439453
},
440454
expectedError: "failed to create NAT rule: #: Internal Server Error: StatusCode=500",
441455
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -501,6 +515,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
501515
VnetName: "my-vnet",
502516
SubnetName: "my-subnet",
503517
InternalLoadBalancerName: "my-internal-lb",
518+
MachineRole: infrav1.ControlPlane,
504519
},
505520
expectedError: "failed to get internalLB: #: Internal Server Error: StatusCode=500",
506521
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -519,10 +534,12 @@ func TestReconcileNetworkInterface(t *testing.T) {
519534
{
520535
name: "network interface with Public IP successfully created",
521536
netInterfaceSpec: Spec{
522-
Name: "my-net-interface",
523-
VnetName: "my-vnet",
524-
SubnetName: "my-subnet",
525-
PublicIPName: "my-public-ip",
537+
Name: "my-net-interface",
538+
VnetName: "my-vnet",
539+
SubnetName: "my-subnet",
540+
PublicIPName: "my-public-ip",
541+
PublicLoadBalancerName: "my-cluster",
542+
MachineRole: infrav1.Node,
526543
},
527544
expectedError: "",
528545
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -535,6 +552,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
535552
mResourceSku.EXPECT().HasAcceleratedNetworking(gomock.Any(), gomock.Any())
536553
gomock.InOrder(
537554
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil),
555+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
538556
mPublicIP.CreateOrUpdate(context.TODO(), "my-rg", "my-public-ip", gomock.AssignableToTypeOf(network.PublicIPAddress{})),
539557
mPublicIP.Get(context.TODO(), "my-rg", "my-public-ip").Return(network.PublicIPAddress{}, nil),
540558
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})))
@@ -543,10 +561,12 @@ func TestReconcileNetworkInterface(t *testing.T) {
543561
{
544562
name: "network interface with Public IP fail to get Public IP",
545563
netInterfaceSpec: Spec{
546-
Name: "my-net-interface",
547-
VnetName: "my-vnet",
548-
SubnetName: "my-subnet",
549-
PublicIPName: "my-public-ip",
564+
Name: "my-net-interface",
565+
VnetName: "my-vnet",
566+
SubnetName: "my-subnet",
567+
PublicIPName: "my-public-ip",
568+
PublicLoadBalancerName: "my-cluster",
569+
MachineRole: infrav1.Node,
550570
},
551571
expectedError: "failed to get publicIP: #: Internal Server Error: StatusCode=500",
552572
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -558,17 +578,19 @@ func TestReconcileNetworkInterface(t *testing.T) {
558578
mResourceSku *mock_resourceskus.MockClient) {
559579
gomock.InOrder(
560580
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil),
581+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
561582
mPublicIP.CreateOrUpdate(context.TODO(), "my-rg", "my-public-ip", gomock.AssignableToTypeOf(network.PublicIPAddress{})),
562-
mPublicIP.Get(context.TODO(), "my-rg", "my-public-ip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")),
563-
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})))
583+
mPublicIP.Get(context.TODO(), "my-rg", "my-public-ip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")))
564584
},
565585
},
566586
{
567587
name: "network interface with accelerated networking successfully created",
568588
netInterfaceSpec: Spec{
569-
Name: "my-net-interface",
570-
VnetName: "my-vnet",
571-
SubnetName: "my-subnet",
589+
Name: "my-net-interface",
590+
VnetName: "my-vnet",
591+
SubnetName: "my-subnet",
592+
PublicLoadBalancerName: "my-cluster",
593+
MachineRole: infrav1.Node,
572594
},
573595
expectedError: "",
574596
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -581,6 +603,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
581603
mResourceSku.EXPECT().HasAcceleratedNetworking(context.TODO(), gomock.Any()).Return(true, nil)
582604
gomock.InOrder(
583605
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil),
606+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
584607
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", matchers.DiffEq(network.Interface{
585608
Location: to.StringPtr("test-location"),
586609
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
@@ -591,7 +614,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
591614
InterfaceIPConfigurationPropertiesFormat: &network.InterfaceIPConfigurationPropertiesFormat{
592615
Subnet: &network.Subnet{},
593616
PrivateIPAllocationMethod: network.Dynamic,
594-
LoadBalancerBackendAddressPools: &[]network.BackendAddressPool{},
617+
LoadBalancerBackendAddressPools: &[]network.BackendAddressPool{{ID: to.StringPtr("cluster-name-outboundBackendPool")}},
595618
},
596619
},
597620
},
@@ -603,9 +626,11 @@ func TestReconcileNetworkInterface(t *testing.T) {
603626
{
604627
name: "network interface without accelerated networking successfully created",
605628
netInterfaceSpec: Spec{
606-
Name: "my-net-interface",
607-
VnetName: "my-vnet",
608-
SubnetName: "my-subnet",
629+
Name: "my-net-interface",
630+
VnetName: "my-vnet",
631+
SubnetName: "my-subnet",
632+
PublicLoadBalancerName: "my-cluster",
633+
MachineRole: infrav1.Node,
609634
},
610635
expectedError: "",
611636
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -618,6 +643,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
618643
mResourceSku.EXPECT().HasAcceleratedNetworking(context.TODO(), gomock.Any()).Return(false, nil)
619644
gomock.InOrder(
620645
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil),
646+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
621647
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", matchers.DiffEq(network.Interface{
622648
Location: to.StringPtr("test-location"),
623649
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
@@ -628,7 +654,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
628654
InterfaceIPConfigurationPropertiesFormat: &network.InterfaceIPConfigurationPropertiesFormat{
629655
Subnet: &network.Subnet{},
630656
PrivateIPAllocationMethod: network.Dynamic,
631-
LoadBalancerBackendAddressPools: &[]network.BackendAddressPool{},
657+
LoadBalancerBackendAddressPools: &[]network.BackendAddressPool{{ID: to.StringPtr("cluster-name-outboundBackendPool")}},
632658
},
633659
},
634660
},
@@ -640,9 +666,11 @@ func TestReconcileNetworkInterface(t *testing.T) {
640666
{
641667
name: "network interface fails to get accelerated networking capability",
642668
netInterfaceSpec: Spec{
643-
Name: "my-net-interface",
644-
VnetName: "my-vnet",
645-
SubnetName: "my-subnet",
669+
Name: "my-net-interface",
670+
VnetName: "my-vnet",
671+
SubnetName: "my-subnet",
672+
PublicLoadBalancerName: "my-cluster",
673+
MachineRole: infrav1.Node,
646674
},
647675
expectedError: "failed to get accelerated networking capability: #: Internal Server Error: StatusCode=500",
648676
expect: func(m *mock_networkinterfaces.MockClientMockRecorder,
@@ -656,6 +684,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
656684
false, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error"))
657685
gomock.InOrder(
658686
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil),
687+
mPublicLoadBalancer.Get(context.TODO(), "my-rg", "my-cluster").Return(getFakeNodeOutboundLoadBalancer(), nil),
659688
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})),
660689
)
661690
},
@@ -953,3 +982,19 @@ func TestDeleteNetworkInterface(t *testing.T) {
953982
})
954983
}
955984
}
985+
986+
func getFakeNodeOutboundLoadBalancer() network.LoadBalancer {
987+
return network.LoadBalancer{
988+
LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{
989+
FrontendIPConfigurations: &[]network.FrontendIPConfiguration{
990+
{
991+
ID: to.StringPtr("frontend-ip-config-id"),
992+
},
993+
},
994+
BackendAddressPools: &[]network.BackendAddressPool{
995+
{
996+
ID: pointer.StringPtr("cluster-name-outboundBackendPool"),
997+
},
998+
},
999+
}}
1000+
}

0 commit comments

Comments
 (0)