Skip to content

Commit 4874668

Browse files
cPu1cPu1
committed
Allow configuring API server LB rule name and health probe name in AzureCluster
Signed-off-by: cpu1 <[email protected]>
1 parent e859273 commit 4874668

15 files changed

+556
-78
lines changed

‎api/v1beta1/azurecluster_default.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ const (
4747
DefaultOutboundRuleIdleTimeoutInMinutes = 4
4848
// DefaultAzureCloud is the public cloud that will be used by most users.
4949
DefaultAzureCloud = "AzurePublicCloud"
50+
// DefaultLoadBalancingRuleName is the default load balancer rule name.
51+
DefaultLoadBalancingRuleName = "LBRuleHTTPS"
52+
// DefaultHealthProbeName is the default health probe name.
53+
DefaultHealthProbeName = "HTTPSProbe"
5054
)
5155

5256
func (c *AzureCluster) setDefaults() {
@@ -303,6 +307,13 @@ func (c *AzureCluster) setAPIServerLBDefaults() {
303307
}
304308
}
305309
c.SetAPIServerLBBackendPoolNameDefault()
310+
311+
if lb.LoadBalancingRule.Name == "" {
312+
lb.LoadBalancingRule.Name = DefaultLoadBalancingRuleName
313+
}
314+
if lb.HealthProbe.Name == "" {
315+
lb.HealthProbe.Name = DefaultHealthProbeName
316+
}
306317
}
307318

308319
// SetNodeOutboundLBDefaults sets the default values for the NodeOutboundLB.

‎api/v1beta1/azurecluster_default_test.go‎

Lines changed: 169 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,14 @@ func TestVnetDefaults(t *testing.T) {
138138
},
139139
},
140140
},
141+
LoadBalancingRule: LoadBalancingRule{
142+
Name: DefaultLoadBalancingRuleName,
143+
},
144+
HealthProbe: HealthProbe{
145+
Name: DefaultHealthProbeName,
146+
},
141147
LoadBalancerClassSpec: LoadBalancerClassSpec{
142-
SKU: SKUStandard,
143-
148+
SKU: SKUStandard,
144149
Type: Public,
145150
},
146151
},
@@ -1321,6 +1326,12 @@ func TestAPIServerLBDefaults(t *testing.T) {
13211326
BackendPool: BackendPool{
13221327
Name: "cluster-test-public-lb-backendPool",
13231328
},
1329+
LoadBalancingRule: LoadBalancingRule{
1330+
Name: "LBRuleHTTPS",
1331+
},
1332+
HealthProbe: HealthProbe{
1333+
Name: "HTTPSProbe",
1334+
},
13241335
LoadBalancerClassSpec: LoadBalancerClassSpec{
13251336
SKU: SKUStandard,
13261337
Type: Public,
@@ -1370,6 +1381,12 @@ func TestAPIServerLBDefaults(t *testing.T) {
13701381
BackendPool: BackendPool{
13711382
Name: "cluster-test-public-lb-backendPool",
13721383
},
1384+
LoadBalancingRule: LoadBalancingRule{
1385+
Name: "LBRuleHTTPS",
1386+
},
1387+
HealthProbe: HealthProbe{
1388+
Name: "HTTPSProbe",
1389+
},
13731390
LoadBalancerClassSpec: LoadBalancerClassSpec{
13741391
SKU: SKUStandard,
13751392
Type: Public,
@@ -1414,6 +1431,12 @@ func TestAPIServerLBDefaults(t *testing.T) {
14141431
BackendPool: BackendPool{
14151432
Name: "cluster-test-internal-lb-backendPool",
14161433
},
1434+
LoadBalancingRule: LoadBalancingRule{
1435+
Name: "LBRuleHTTPS",
1436+
},
1437+
HealthProbe: HealthProbe{
1438+
Name: "HTTPSProbe",
1439+
},
14171440
LoadBalancerClassSpec: LoadBalancerClassSpec{
14181441
SKU: SKUStandard,
14191442
Type: Internal,
@@ -1460,6 +1483,12 @@ func TestAPIServerLBDefaults(t *testing.T) {
14601483
BackendPool: BackendPool{
14611484
Name: "cluster-test-internal-lb-backendPool",
14621485
},
1486+
LoadBalancingRule: LoadBalancingRule{
1487+
Name: "LBRuleHTTPS",
1488+
},
1489+
HealthProbe: HealthProbe{
1490+
Name: "HTTPSProbe",
1491+
},
14631492
LoadBalancerClassSpec: LoadBalancerClassSpec{
14641493
SKU: SKUStandard,
14651494
Type: Internal,
@@ -1508,6 +1537,12 @@ func TestAPIServerLBDefaults(t *testing.T) {
15081537
BackendPool: BackendPool{
15091538
Name: "custom-backend-pool",
15101539
},
1540+
LoadBalancingRule: LoadBalancingRule{
1541+
Name: "LBRuleHTTPS",
1542+
},
1543+
HealthProbe: HealthProbe{
1544+
Name: "HTTPSProbe",
1545+
},
15111546
LoadBalancerClassSpec: LoadBalancerClassSpec{
15121547
SKU: SKUStandard,
15131548
Type: Internal,
@@ -1557,6 +1592,12 @@ func TestAPIServerLBDefaults(t *testing.T) {
15571592
BackendPool: BackendPool{
15581593
Name: "custom-backend-pool",
15591594
},
1595+
LoadBalancingRule: LoadBalancingRule{
1596+
Name: "LBRuleHTTPS",
1597+
},
1598+
HealthProbe: HealthProbe{
1599+
Name: "HTTPSProbe",
1600+
},
15601601
LoadBalancerClassSpec: LoadBalancerClassSpec{
15611602
SKU: SKUStandard,
15621603
Type: Internal,
@@ -1630,6 +1671,132 @@ func TestAPIServerLBDefaults(t *testing.T) {
16301671
BackendPool: BackendPool{
16311672
Name: "cluster-test-public-lb-backendPool",
16321673
},
1674+
LoadBalancingRule: LoadBalancingRule{
1675+
Name: "LBRuleHTTPS",
1676+
},
1677+
HealthProbe: HealthProbe{
1678+
Name: "HTTPSProbe",
1679+
},
1680+
LoadBalancerClassSpec: LoadBalancerClassSpec{
1681+
SKU: SKUStandard,
1682+
Type: Public,
1683+
IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes),
1684+
},
1685+
},
1686+
},
1687+
},
1688+
},
1689+
},
1690+
{
1691+
name: "load balancer with custom rule and probe names",
1692+
cluster: &AzureCluster{
1693+
ObjectMeta: metav1.ObjectMeta{
1694+
Name: "cluster-test",
1695+
},
1696+
Spec: AzureClusterSpec{
1697+
ControlPlaneEnabled: true,
1698+
NetworkSpec: NetworkSpec{
1699+
APIServerLB: &LoadBalancerSpec{
1700+
LoadBalancingRule: LoadBalancingRule{
1701+
Name: "CustomLBRule",
1702+
},
1703+
HealthProbe: HealthProbe{
1704+
Name: "CustomProbe",
1705+
},
1706+
LoadBalancerClassSpec: LoadBalancerClassSpec{
1707+
Type: Public,
1708+
},
1709+
},
1710+
},
1711+
},
1712+
},
1713+
output: &AzureCluster{
1714+
ObjectMeta: metav1.ObjectMeta{
1715+
Name: "cluster-test",
1716+
},
1717+
Spec: AzureClusterSpec{
1718+
ControlPlaneEnabled: true,
1719+
NetworkSpec: NetworkSpec{
1720+
APIServerLB: &LoadBalancerSpec{
1721+
Name: "cluster-test-public-lb",
1722+
FrontendIPs: []FrontendIP{
1723+
{
1724+
Name: "cluster-test-public-lb-frontEnd",
1725+
PublicIP: &PublicIPSpec{
1726+
Name: "pip-cluster-test-apiserver",
1727+
DNSName: "",
1728+
},
1729+
},
1730+
},
1731+
BackendPool: BackendPool{
1732+
Name: "cluster-test-public-lb-backendPool",
1733+
},
1734+
LoadBalancingRule: LoadBalancingRule{
1735+
Name: "CustomLBRule",
1736+
},
1737+
HealthProbe: HealthProbe{
1738+
Name: "CustomProbe",
1739+
},
1740+
LoadBalancerClassSpec: LoadBalancerClassSpec{
1741+
SKU: SKUStandard,
1742+
Type: Public,
1743+
IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes),
1744+
},
1745+
},
1746+
},
1747+
},
1748+
},
1749+
},
1750+
{
1751+
name: "load balancer with empty rule and probe names should use defaults",
1752+
cluster: &AzureCluster{
1753+
ObjectMeta: metav1.ObjectMeta{
1754+
Name: "cluster-test",
1755+
},
1756+
Spec: AzureClusterSpec{
1757+
ControlPlaneEnabled: true,
1758+
NetworkSpec: NetworkSpec{
1759+
APIServerLB: &LoadBalancerSpec{
1760+
LoadBalancingRule: LoadBalancingRule{
1761+
Name: "",
1762+
},
1763+
HealthProbe: HealthProbe{
1764+
Name: "",
1765+
},
1766+
LoadBalancerClassSpec: LoadBalancerClassSpec{
1767+
Type: Public,
1768+
},
1769+
},
1770+
},
1771+
},
1772+
},
1773+
output: &AzureCluster{
1774+
ObjectMeta: metav1.ObjectMeta{
1775+
Name: "cluster-test",
1776+
},
1777+
Spec: AzureClusterSpec{
1778+
ControlPlaneEnabled: true,
1779+
NetworkSpec: NetworkSpec{
1780+
APIServerLB: &LoadBalancerSpec{
1781+
Name: "cluster-test-public-lb",
1782+
FrontendIPs: []FrontendIP{
1783+
{
1784+
Name: "cluster-test-public-lb-frontEnd",
1785+
PublicIP: &PublicIPSpec{
1786+
Name: "pip-cluster-test-apiserver",
1787+
DNSName: "",
1788+
},
1789+
},
1790+
},
1791+
BackendPool: BackendPool{
1792+
Name: "cluster-test-public-lb-backendPool",
1793+
},
1794+
LoadBalancingRule: LoadBalancingRule{
1795+
Name: "LBRuleHTTPS",
1796+
},
1797+
HealthProbe: HealthProbe{
1798+
Name: "HTTPSProbe",
1799+
},
16331800
LoadBalancerClassSpec: LoadBalancerClassSpec{
16341801
SKU: SKUStandard,
16351802
Type: Public,

‎api/v1beta1/azurecluster_validation.go‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,11 @@ func validateSecurityRule(rule SecurityRule, fldPath *field.Path) (allErrs field
391391
return allErrs
392392
}
393393

394-
func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []string, fldPath *field.Path) field.ErrorList {
394+
func immutableAzureClusterFieldError(fldPath *field.Path, fieldName string) *field.Error {
395+
return field.Forbidden(fldPath, fmt.Sprintf("%s should not be modified after AzureCluster creation.", fieldName))
396+
}
397+
398+
func validateAPIServerLB(lb, old *LoadBalancerSpec, cidrs []string, fldPath *field.Path) field.ErrorList {
395399
var allErrs field.ErrorList
396400

397401
lbClassSpec := lb.LoadBalancerClassSpec
@@ -406,9 +410,19 @@ func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []st
406410
if err := validateLoadBalancerName(lb.Name, fldPath.Child("name")); err != nil {
407411
allErrs = append(allErrs, err)
408412
}
409-
// Name should be immutable.
410-
if old != nil && old.Name != "" && old.Name != lb.Name {
411-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer name should not be modified after AzureCluster creation."))
413+
if old != nil {
414+
// Name should be immutable.
415+
if old.Name != "" && old.Name != lb.Name {
416+
allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath, "API Server load balancer name"))
417+
}
418+
if (old.LoadBalancingRule.Name != "" && old.LoadBalancingRule.Name != lb.LoadBalancingRule.Name) ||
419+
(old.LoadBalancingRule.Name == "" && lb.LoadBalancingRule.Name != DefaultLoadBalancingRuleName) {
420+
allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("loadBalancingRule").Child("name"), "Load balancer rule name"))
421+
}
422+
if (old.HealthProbe.Name != "" && old.HealthProbe.Name != lb.HealthProbe.Name) ||
423+
(old.HealthProbe.Name == "" && lb.HealthProbe.Name != DefaultHealthProbeName) {
424+
allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("healthProbe").Child("name"), "Health probe name"))
425+
}
412426
}
413427

414428
publicIPCount, privateIPCount := 0, 0
@@ -458,7 +472,7 @@ func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []st
458472
}
459473

460474
if old != nil && len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress {
461-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
475+
allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("name"), "API Server load balancer private IP"))
462476
}
463477
}
464478
}
@@ -484,11 +498,11 @@ func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserv
484498
}
485499

486500
if old != nil && old.ID != lb.ID {
487-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("id"), "Node outbound load balancer ID should not be modified after AzureCluster creation."))
501+
allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("id"), "Node outbound load balancer ID"))
488502
}
489503

490504
if old != nil && old.Name != lb.Name {
491-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "Node outbound load balancer Name should not be modified after AzureCluster creation."))
505+
allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("name"), "Node outbound load balancer Name"))
492506
}
493507

494508
if old != nil && old.FrontendIPsCount == lb.FrontendIPsCount {
@@ -598,12 +612,12 @@ func validateClassSpecForAPIServerLB(lb LoadBalancerClassSpec, old *LoadBalancer
598612

599613
// SKU should be immutable.
600614
if old != nil && old.SKU != "" && old.SKU != lb.SKU {
601-
allErrs = append(allErrs, field.Forbidden(apiServerLBPath.Child("sku"), "API Server load balancer SKU should not be modified after AzureCluster creation."))
615+
allErrs = append(allErrs, immutableAzureClusterFieldError(apiServerLBPath.Child("sku"), "API Server load balancer SKU"))
602616
}
603617

604618
// Type should be immutable.
605619
if old != nil && old.Type != "" && old.Type != lb.Type {
606-
allErrs = append(allErrs, field.Forbidden(apiServerLBPath.Child("type"), "API Server load balancer type should not be modified after AzureCluster creation."))
620+
allErrs = append(allErrs, immutableAzureClusterFieldError(apiServerLBPath.Child("type"), "API Server load balancer type"))
607621
}
608622

609623
// IdletimeoutInMinutes should be immutable.
@@ -633,7 +647,7 @@ func validateClassSpecForNodeOutboundLB(lb *LoadBalancerClassSpec, old *LoadBala
633647
}
634648

635649
if old != nil && old.SKU != lb.SKU {
636-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("sku"), "Node outbound load balancer SKU should not be modified after AzureCluster creation."))
650+
allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("sku"), "Node outbound load balancer SKU"))
637651
}
638652

639653
if old != nil && old.Type != lb.Type {

‎api/v1beta1/azurecluster_validation_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,6 +1905,12 @@ func createValidAPIServerLB() *LoadBalancerSpec {
19051905
},
19061906
},
19071907
},
1908+
LoadBalancingRule: LoadBalancingRule{
1909+
Name: DefaultLoadBalancingRuleName,
1910+
},
1911+
HealthProbe: HealthProbe{
1912+
Name: DefaultHealthProbeName,
1913+
},
19081914
LoadBalancerClassSpec: LoadBalancerClassSpec{
19091915
SKU: SKUStandard,
19101916
Type: Public,

‎api/v1beta1/azurecluster_webhook.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func (*AzureClusterWebhook) ValidateUpdate(_ context.Context, oldRaw, newObj run
120120
// The equality failure could be because of default mismatch between v1alpha3 and v1beta1. This happens because
121121
// the new object `r` will have run through the default webhooks but the old object `old` would not have so.
122122
// This means if the old object was in v1alpha3, it would not get the new defaults set in v1beta1 resulting
123-
// in object inequality. To workaround this, we set the v1beta1 defaults here so that the old object also gets
123+
// in object inequality. To work around this, we set the v1beta1 defaults here so that the old object also gets
124124
// the new defaults.
125125
old.setAzureEnvironmentDefault()
126126

0 commit comments

Comments
 (0)