Skip to content

Commit 69cac90

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1466 from shysank/cp_lb_config
Make control plane outbound lb configurable
2 parents 7344eb9 + 7d293d5 commit 69cac90

17 files changed

+469
-49
lines changed

‎api/v1alpha3/azurecluster_conversion.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint
5454
dst.Spec.NetworkSpec.APIServerLB.FrontendIPsCount = restored.Spec.NetworkSpec.APIServerLB.FrontendIPsCount
5555
dst.Spec.NetworkSpec.APIServerLB.IdleTimeoutInMinutes = restored.Spec.NetworkSpec.APIServerLB.IdleTimeoutInMinutes
5656
dst.Spec.NetworkSpec.NodeOutboundLB = restored.Spec.NetworkSpec.NodeOutboundLB
57+
dst.Spec.NetworkSpec.ControlPlaneOutboundLB = restored.Spec.NetworkSpec.ControlPlaneOutboundLB
5758
dst.Spec.CloudProviderConfigOverrides = restored.Spec.CloudProviderConfigOverrides
5859
dst.Spec.BastionSpec = restored.Spec.BastionSpec
5960

‎api/v1alpha3/zz_generated.conversion.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1alpha4/azurecluster_default.go‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ func (c *AzureCluster) setNetworkSpecDefaults() {
5353
c.setSubnetDefaults()
5454
c.setAPIServerLBDefaults()
5555
c.setNodeOutboundLBDefaults()
56+
c.setControlPlaneOutboundLBDefaults()
5657
}
5758

5859
func (c *AzureCluster) setResourceGroupDefault() {
@@ -196,6 +197,42 @@ func (c *AzureCluster) setNodeOutboundLBDefaults() {
196197
lb.FrontendIPsCount = pointer.Int32Ptr(1)
197198
}
198199

200+
c.setOutboundLBFrontendIPs(lb, generateNodeOutboundIPName)
201+
}
202+
203+
func (c *AzureCluster) setControlPlaneOutboundLBDefaults() {
204+
// public clusters don't need control plane outbound lb
205+
if c.Spec.NetworkSpec.APIServerLB.Type == Public {
206+
return
207+
}
208+
209+
// private clusters can disable control plane outbound lb by setting it to nil.
210+
if c.Spec.NetworkSpec.ControlPlaneOutboundLB == nil {
211+
return
212+
}
213+
214+
lb := c.Spec.NetworkSpec.ControlPlaneOutboundLB
215+
lb.Type = Public
216+
lb.SKU = SKUStandard
217+
218+
if lb.Name == "" {
219+
lb.Name = generateControlPlaneOutboundLBName(c.ObjectMeta.Name)
220+
}
221+
222+
if lb.IdleTimeoutInMinutes == nil {
223+
lb.IdleTimeoutInMinutes = pointer.Int32Ptr(DefaultOutboundRuleIdleTimeoutInMinutes)
224+
}
225+
226+
if lb.FrontendIPsCount == nil {
227+
lb.FrontendIPsCount = pointer.Int32Ptr(1)
228+
}
229+
230+
c.setOutboundLBFrontendIPs(lb, generateControlPlaneOutboundIPName)
231+
}
232+
233+
// setOutboundLBFrontendIPs sets the frontend ips for the given load balancer.
234+
// The name of the frontend ip is generated using generatePublicIPName function.
235+
func (c *AzureCluster) setOutboundLBFrontendIPs(lb *LoadBalancerSpec, generatePublicIPName func(string) string) {
199236
switch *lb.FrontendIPsCount {
200237
case 0:
201238
lb.FrontendIPs = []FrontendIP{}
@@ -204,7 +241,7 @@ func (c *AzureCluster) setNodeOutboundLBDefaults() {
204241
{
205242
Name: generateFrontendIPConfigName(lb.Name),
206243
PublicIP: &PublicIPSpec{
207-
Name: generateNodeOutboundIPName(c.ObjectMeta.Name),
244+
Name: generatePublicIPName(c.ObjectMeta.Name),
208245
},
209246
},
210247
}
@@ -214,7 +251,7 @@ func (c *AzureCluster) setNodeOutboundLBDefaults() {
214251
lb.FrontendIPs[i] = FrontendIP{
215252
Name: withIndex(generateFrontendIPConfigName(lb.Name), i+1),
216253
PublicIP: &PublicIPSpec{
217-
Name: withIndex(generateNodeOutboundIPName(c.ObjectMeta.Name), i+1),
254+
Name: withIndex(generatePublicIPName(c.ObjectMeta.Name), i+1),
218255
},
219256
}
220257
}
@@ -294,6 +331,11 @@ func generatePublicLBName(clusterName string) string {
294331
return fmt.Sprintf("%s-%s", clusterName, "public-lb")
295332
}
296333

334+
// generateControlPlaneOutboundLBName generates the name of the control plane outbound LB.
335+
func generateControlPlaneOutboundLBName(clusterName string) string {
336+
return fmt.Sprintf("%s-outbound-lb", clusterName)
337+
}
338+
297339
// generatePublicIPName generates a public IP name, based on the cluster name and a hash.
298340
func generatePublicIPName(clusterName string) string {
299341
return fmt.Sprintf("pip-%s-apiserver", clusterName)
@@ -309,6 +351,11 @@ func generateNodeOutboundIPName(clusterName string) string {
309351
return fmt.Sprintf("pip-%s-node-outbound", clusterName)
310352
}
311353

354+
// generateControlPlaneOutboundIPName generates a public IP name, based on the cluster name.
355+
func generateControlPlaneOutboundIPName(clusterName string) string {
356+
return fmt.Sprintf("pip-%s-controlplane-outbound", clusterName)
357+
}
358+
312359
// withIndex appends the index as suffix to a generated name.
313360
func withIndex(name string, n int) string {
314361
return fmt.Sprintf("%s-%d", name, n)

‎api/v1alpha4/azurecluster_default_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,128 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
916916
}
917917
}
918918

919+
func TestControlPlaneOutboundLBDefaults(t *testing.T) {
920+
cases := []struct {
921+
name string
922+
cluster *AzureCluster
923+
output *AzureCluster
924+
}{
925+
{
926+
name: "no cp lb for public clusters",
927+
cluster: &AzureCluster{
928+
ObjectMeta: v1.ObjectMeta{
929+
Name: "cluster-test",
930+
},
931+
Spec: AzureClusterSpec{
932+
NetworkSpec: NetworkSpec{
933+
APIServerLB: LoadBalancerSpec{Type: Public},
934+
},
935+
},
936+
},
937+
output: &AzureCluster{
938+
ObjectMeta: v1.ObjectMeta{
939+
Name: "cluster-test",
940+
},
941+
Spec: AzureClusterSpec{
942+
NetworkSpec: NetworkSpec{
943+
APIServerLB: LoadBalancerSpec{
944+
Type: Public,
945+
},
946+
},
947+
},
948+
},
949+
},
950+
{
951+
name: "no cp lb for private clusters",
952+
cluster: &AzureCluster{
953+
ObjectMeta: v1.ObjectMeta{
954+
Name: "cluster-test",
955+
},
956+
Spec: AzureClusterSpec{
957+
NetworkSpec: NetworkSpec{
958+
APIServerLB: LoadBalancerSpec{Type: Internal},
959+
},
960+
},
961+
},
962+
output: &AzureCluster{
963+
ObjectMeta: v1.ObjectMeta{
964+
Name: "cluster-test",
965+
},
966+
Spec: AzureClusterSpec{
967+
NetworkSpec: NetworkSpec{
968+
APIServerLB: LoadBalancerSpec{
969+
Type: Internal,
970+
},
971+
},
972+
},
973+
},
974+
},
975+
{
976+
name: "frontendIPsCount > 1",
977+
cluster: &AzureCluster{
978+
ObjectMeta: v1.ObjectMeta{
979+
Name: "cluster-test",
980+
},
981+
Spec: AzureClusterSpec{
982+
NetworkSpec: NetworkSpec{
983+
APIServerLB: LoadBalancerSpec{Type: Internal},
984+
ControlPlaneOutboundLB: &LoadBalancerSpec{
985+
FrontendIPsCount: to.Int32Ptr(2),
986+
IdleTimeoutInMinutes: to.Int32Ptr(15),
987+
},
988+
},
989+
},
990+
},
991+
output: &AzureCluster{
992+
ObjectMeta: v1.ObjectMeta{
993+
Name: "cluster-test",
994+
},
995+
Spec: AzureClusterSpec{
996+
NetworkSpec: NetworkSpec{
997+
APIServerLB: LoadBalancerSpec{
998+
Type: Internal,
999+
},
1000+
ControlPlaneOutboundLB: &LoadBalancerSpec{
1001+
Name: "cluster-test-outbound-lb",
1002+
SKU: SKUStandard,
1003+
FrontendIPs: []FrontendIP{
1004+
{
1005+
Name: "cluster-test-outbound-lb-frontEnd-1",
1006+
PublicIP: &PublicIPSpec{
1007+
Name: "pip-cluster-test-controlplane-outbound-1",
1008+
},
1009+
},
1010+
{
1011+
Name: "cluster-test-outbound-lb-frontEnd-2",
1012+
PublicIP: &PublicIPSpec{
1013+
Name: "pip-cluster-test-controlplane-outbound-2",
1014+
},
1015+
},
1016+
},
1017+
Type: Public,
1018+
FrontendIPsCount: to.Int32Ptr(2),
1019+
IdleTimeoutInMinutes: to.Int32Ptr(15),
1020+
},
1021+
},
1022+
},
1023+
},
1024+
},
1025+
}
1026+
1027+
for _, c := range cases {
1028+
tc := c
1029+
t.Run(tc.name, func(t *testing.T) {
1030+
t.Parallel()
1031+
tc.cluster.setControlPlaneOutboundLBDefaults()
1032+
if !reflect.DeepEqual(tc.cluster, tc.output) {
1033+
expected, _ := json.MarshalIndent(tc.output, "", "\t")
1034+
actual, _ := json.MarshalIndent(tc.cluster, "", "\t")
1035+
t.Errorf("Expected %s, got %s", string(expected), string(actual))
1036+
}
1037+
})
1038+
}
1039+
}
1040+
9191041
func TestBastionDefault(t *testing.T) {
9201042
cases := map[string]struct {
9211043
cluster *AzureCluster

‎api/v1alpha4/azurecluster_validation.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ func validateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *fiel
137137

138138
allErrs = append(allErrs, validateNodeOutboundLB(networkSpec.NodeOutboundLB, old.NodeOutboundLB, networkSpec.APIServerLB, nodeSubnet, fldPath.Child("nodeOutboundLB"))...)
139139

140+
allErrs = append(allErrs, validateControlPlaneOutboundLB(networkSpec.ControlPlaneOutboundLB, networkSpec.APIServerLB, fldPath.Child("controlPlaneOutboundLB"))...)
141+
140142
allErrs = append(allErrs, validatePrivateDNSZoneName(networkSpec, fldPath)...)
141143

142144
if len(allErrs) == 0 {
@@ -420,6 +422,34 @@ func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserv
420422
return allErrs
421423
}
422424

425+
func validateControlPlaneOutboundLB(lb *LoadBalancerSpec, apiserverLB LoadBalancerSpec, fldPath *field.Path) field.ErrorList {
426+
var allErrs field.ErrorList
427+
428+
switch apiserverLB.Type {
429+
case Public:
430+
if lb != nil {
431+
allErrs = append(allErrs, field.Forbidden(fldPath, "Control plane outbound load balancer cannot be set for public clusters."))
432+
}
433+
case Internal:
434+
// Control plane outbound lb can be nil when it's disabled for private clusters.
435+
if lb == nil {
436+
return nil
437+
}
438+
439+
if lb.FrontendIPsCount != nil && *lb.FrontendIPsCount > MaxLoadBalancerOutboundIPs {
440+
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPsCount"), *lb.FrontendIPsCount,
441+
fmt.Sprintf("Max front end ips allowed is %d", MaxLoadBalancerOutboundIPs)))
442+
}
443+
444+
if lb.IdleTimeoutInMinutes != nil && (*lb.IdleTimeoutInMinutes < MinLBIdleTimeoutInMinutes || *lb.IdleTimeoutInMinutes > MaxLBIdleTimeoutInMinutes) {
445+
allErrs = append(allErrs, field.Invalid(fldPath.Child("idleTimeoutInMinutes"), *lb.IdleTimeoutInMinutes,
446+
fmt.Sprintf("Control plane outbound idle timeout should be between %d and %d minutes", MinLBIdleTimeoutInMinutes, MaxLoadBalancerOutboundIPs)))
447+
}
448+
}
449+
450+
return allErrs
451+
}
452+
423453
// validatePrivateDNSZoneName validate the PrivateDNSZoneName.
424454
func validatePrivateDNSZoneName(networkSpec NetworkSpec, fldPath *field.Path) field.ErrorList {
425455
var allErrs field.ErrorList

‎api/v1alpha4/azurecluster_validation_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,78 @@ func TestValidateNodeOutboundLB(t *testing.T) {
11181118
}
11191119
}
11201120

1121+
func TestValidateControlPlaneNodeOutboundLB(t *testing.T) {
1122+
g := NewWithT(t)
1123+
1124+
testcases := []struct {
1125+
name string
1126+
lb *LoadBalancerSpec
1127+
old *LoadBalancerSpec
1128+
apiServerLB LoadBalancerSpec
1129+
wantErr bool
1130+
expectedErr field.Error
1131+
}{
1132+
{
1133+
name: "cp outbound lb cannot be set for public clusters",
1134+
lb: &LoadBalancerSpec{Name: "foo"},
1135+
apiServerLB: LoadBalancerSpec{Type: Public},
1136+
wantErr: true,
1137+
expectedErr: field.Error{
1138+
Type: "FieldValueForbidden",
1139+
Field: "controlPlaneOutboundLB",
1140+
BadValue: LoadBalancerSpec{Name: "foo"},
1141+
Detail: "Control plane outbound load balancer cannot be set for public clusters.",
1142+
},
1143+
},
1144+
{
1145+
name: "cp outbound lb can be set for private clusters",
1146+
lb: &LoadBalancerSpec{Name: "foo"},
1147+
apiServerLB: LoadBalancerSpec{Type: Internal},
1148+
wantErr: false,
1149+
},
1150+
{
1151+
name: "cp outbound lb can be nil for private clusters",
1152+
lb: nil,
1153+
apiServerLB: LoadBalancerSpec{Type: Internal},
1154+
wantErr: false,
1155+
},
1156+
{
1157+
name: "frontend ips count exceeds max value",
1158+
lb: &LoadBalancerSpec{
1159+
FrontendIPsCount: pointer.Int32Ptr(100),
1160+
},
1161+
apiServerLB: LoadBalancerSpec{Type: Internal},
1162+
wantErr: true,
1163+
expectedErr: field.Error{
1164+
Type: "FieldValueInvalid",
1165+
Field: "controlPlaneOutboundLB.frontendIPsCount",
1166+
BadValue: 100,
1167+
Detail: "Max front end ips allowed is 16",
1168+
},
1169+
},
1170+
}
1171+
1172+
for _, test := range testcases {
1173+
test := test
1174+
t.Run(test.name, func(t *testing.T) {
1175+
t.Parallel()
1176+
err := validateControlPlaneOutboundLB(test.lb, test.apiServerLB, field.NewPath("controlPlaneOutboundLB"))
1177+
if test.wantErr {
1178+
g.Expect(err).NotTo(HaveLen(0))
1179+
found := false
1180+
for _, actual := range err {
1181+
if actual.Error() == test.expectedErr.Error() {
1182+
found = true
1183+
}
1184+
}
1185+
g.Expect(found).To(BeTrue())
1186+
} else {
1187+
g.Expect(err).To(HaveLen(0))
1188+
}
1189+
})
1190+
}
1191+
}
1192+
11211193
func TestValidateCloudProviderConfigOverrides(t *testing.T) {
11221194
g := NewWithT(t)
11231195

‎api/v1alpha4/azurecluster_webhook.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@ func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
106106
)
107107
}
108108

109+
if !reflect.DeepEqual(c.Spec.NetworkSpec.ControlPlaneOutboundLB, old.Spec.NetworkSpec.ControlPlaneOutboundLB) {
110+
allErrs = append(allErrs,
111+
field.Invalid(field.NewPath("spec", "networkSpec", "controlPlaneOutboundLB"),
112+
c.Spec.NetworkSpec.ControlPlaneOutboundLB, "field is immutable"),
113+
)
114+
}
115+
109116
if len(allErrs) == 0 {
110117
return c.validateCluster(old)
111118
}

0 commit comments

Comments
 (0)