Skip to content

Commit

Permalink
let users configure private IP of the internal LB
Browse files Browse the repository at this point in the history 
- update apiserver ilb flavor with custom CIDRs
- update apiserver ilb windows flavor with custom CIDRs
- create ci template for api server ILB PR tests
- add e2e tests for apiserver ilb template
- add e2e test for a default flavored template
  • Loading branch information
@nawazkh
nawazkh committed Dec 12, 2024
1 parent 61c6f3f commit e0f2b44
Show file tree
Hide file tree
Showing 16 changed files with 1,594 additions and 83 deletions.
25 changes: 25 additions & 0 deletions api/v1beta1/azurecluster_default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"fmt"

"k8s.io/utils/ptr"

"sigs.k8s.io/cluster-api-provider-azure/feature"
)

const (
Expand Down Expand Up @@ -245,6 +247,29 @@ func (c *AzureCluster) setAPIServerLBDefaults() {
},
}
}
// if the API Server ILB feature is enabled, we should create a default internal LB IP
if feature.Gates.Enabled(feature.APIServerILB) {
privateIPFound := false
for i := range lb.FrontendIPs {
if lb.FrontendIPs[i].FrontendIPClass.PrivateIPAddress != "" {
if lb.FrontendIPs[i].Name == "" {
lb.FrontendIPs[i].Name = generateFrontendIPConfigName(lb.Name) + "-internal-ip"
}

Check warning on line 257 in api/v1beta1/azurecluster_default.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta1/azurecluster_default.go#L256-L257 

Added lines #L256 - L257 were not covered by tests
privateIPFound = true
break
}
}
// if no private IP is found, we should create a default internal LB IP
if !privateIPFound {
privateIP := FrontendIP{
Name: generateFrontendIPConfigName(lb.Name) + "-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
}
lb.FrontendIPs = append(lb.FrontendIPs, privateIP)
}
}
} else if lb.Type == Internal {
if lb.Name == "" {
lb.Name = generateInternalLBName(c.ObjectMeta.Name)
Expand Down
231 changes: 227 additions & 4 deletions api/v1beta1/azurecluster_default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ import (

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/component-base/featuregate"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/utils/ptr"

"sigs.k8s.io/cluster-api-provider-azure/feature"
)

func TestResourceGroupDefault(t *testing.T) {
Expand Down Expand Up @@ -1236,9 +1240,10 @@ func TestVnetPeeringDefaults(t *testing.T) {

func TestAPIServerLBDefaults(t *testing.T) {
cases := []struct {
name string
cluster *AzureCluster
output *AzureCluster
name string
featureGate featuregate.Feature
cluster *AzureCluster
output *AzureCluster
}{
{
name: "no lb",
Expand Down Expand Up @@ -1282,6 +1287,55 @@ func TestAPIServerLBDefaults(t *testing.T) {
},
},
},
{
name: "no lb with APIServerILB feature gate enabled",
featureGate: feature.APIServerILB,
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
ControlPlaneEnabled: true,
NetworkSpec: NetworkSpec{},
},
},
output: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
ControlPlaneEnabled: true,
NetworkSpec: NetworkSpec{
APIServerLB: &LoadBalancerSpec{
Name: "cluster-test-public-lb",
FrontendIPs: []FrontendIP{
{
Name: "cluster-test-public-lb-frontEnd",
PublicIP: &PublicIPSpec{
Name: "pip-cluster-test-apiserver",
DNSName: "",
},
},
{
Name: "cluster-test-public-lb-frontEnd-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
},
},
BackendPool: BackendPool{
Name: "cluster-test-public-lb-backendPool",
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
SKU: SKUStandard,
Type: Public,
IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes),
},
},
},
},
},
},
{
name: "internal lb",
cluster: &AzureCluster{
Expand Down Expand Up @@ -1327,6 +1381,52 @@ func TestAPIServerLBDefaults(t *testing.T) {
},
},
},
{
name: "internal lb with feature gate API Server ILB enabled",
featureGate: feature.APIServerILB,
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
NetworkSpec: NetworkSpec{
APIServerLB: &LoadBalancerSpec{
LoadBalancerClassSpec: LoadBalancerClassSpec{
Type: Internal,
},
},
},
},
},
output: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
NetworkSpec: NetworkSpec{
APIServerLB: &LoadBalancerSpec{
FrontendIPs: []FrontendIP{
{
Name: "cluster-test-internal-lb-frontEnd",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
},
},
BackendPool: BackendPool{
Name: "cluster-test-internal-lb-backendPool",
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
SKU: SKUStandard,
Type: Internal,
IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes),
},
Name: "cluster-test-internal-lb",
},
},
},
},
},
{
name: "with custom backend pool name",
cluster: &AzureCluster{
Expand Down Expand Up @@ -1375,12 +1475,135 @@ func TestAPIServerLBDefaults(t *testing.T) {
},
},
},
{
name: "with custom backend pool name with feature gate API Server ILB enabled",
featureGate: feature.APIServerILB,
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
NetworkSpec: NetworkSpec{
APIServerLB: &LoadBalancerSpec{
LoadBalancerClassSpec: LoadBalancerClassSpec{
Type: Internal,
},
BackendPool: BackendPool{
Name: "custom-backend-pool",
},
},
},
},
},
output: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
NetworkSpec: NetworkSpec{
APIServerLB: &LoadBalancerSpec{
FrontendIPs: []FrontendIP{
{
Name: "cluster-test-internal-lb-frontEnd",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: DefaultInternalLBIPAddress,
},
},
},
BackendPool: BackendPool{
Name: "custom-backend-pool",
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
SKU: SKUStandard,
Type: Internal,
IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes),
},
Name: "cluster-test-internal-lb",
},
},
},
},
},
{
name: "public lb with APIServerILB feature gate enabled and custom private IP belonging to default control plane CIDR",
featureGate: feature.APIServerILB,
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
ControlPlaneEnabled: true,
NetworkSpec: NetworkSpec{
APIServerLB: &LoadBalancerSpec{
Name: "cluster-test-public-lb",
FrontendIPs: []FrontendIP{
{
Name: "cluster-test-public-lb-frontEnd",
PublicIP: &PublicIPSpec{
Name: "pip-cluster-test-apiserver",
DNSName: "",
},
},
{
Name: "my-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: "10.0.0.111",
},
},
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
Type: Public,
SKU: SKUStandard,
},
},
},
},
},
output: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: AzureClusterSpec{
ControlPlaneEnabled: true,
NetworkSpec: NetworkSpec{
APIServerLB: &LoadBalancerSpec{
Name: "cluster-test-public-lb",
FrontendIPs: []FrontendIP{
{
Name: "cluster-test-public-lb-frontEnd",
PublicIP: &PublicIPSpec{
Name: "pip-cluster-test-apiserver",
DNSName: "",
},
},
{
Name: "my-internal-ip",
FrontendIPClass: FrontendIPClass{
PrivateIPAddress: "10.0.0.111",
},
},
},
BackendPool: BackendPool{
Name: "cluster-test-public-lb-backendPool",
},
LoadBalancerClassSpec: LoadBalancerClassSpec{
SKU: SKUStandard,
Type: Public,
IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes),
},
},
},
},
},
},
}

for _, c := range cases {
tc := c
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
if tc.featureGate != "" {
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.Gates, tc.featureGate, true)()
}
tc.cluster.setAPIServerLBDefaults()
if !reflect.DeepEqual(tc.cluster, tc.output) {
expected, _ := json.MarshalIndent(tc.output, "", "\t")
Expand Down
67 changes: 43 additions & 24 deletions api/v1beta1/azurecluster_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,37 +412,56 @@ func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []st
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer name should not be modified after AzureCluster creation."))
}

// There should only be one IP config.
if len(lb.FrontendIPs) != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs,
"API Server Load balancer should have 1 Frontend IP"))
} else {
// if Internal, IP config should not have a public IP.
if lb.Type == Internal {
if lb.FrontendIPs[0].PublicIP != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"),
"Internal Load Balancers cannot have a Public IP"))
}
if lb.FrontendIPs[0].PrivateIPAddress != "" {
if err := validateInternalLBIPAddress(lb.FrontendIPs[0].PrivateIPAddress, cidrs,
fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP")); err != nil {
allErrs = append(allErrs, err)
}
if len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
}
}
publicIPCount, privateIPCount := 0, 0
privateIP := ""
for i := range lb.FrontendIPs {
if lb.FrontendIPs[i].PublicIP != nil {
publicIPCount++
}

// if Public, IP config should not have a private IP.
if lb.Type == Public {
if lb.FrontendIPs[0].PrivateIPAddress != "" {
if lb.FrontendIPs[i].PrivateIPAddress != "" {
privateIPCount++
privateIP = lb.FrontendIPs[i].PrivateIPAddress
}
}
if lb.Type == Public {
// there should be one public IP for public LB.
if publicIPCount != 1 || ptr.Deref[int32](lb.FrontendIPsCount, 1) != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs,
"API Server Load balancer should have 1 Frontend IP"))
}
if feature.Gates.Enabled(feature.APIServerILB) {
if err := validateInternalLBIPAddress(privateIP, cidrs, fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP")); err != nil {
allErrs = append(allErrs, err)
}
} else {
// API Server LB should not have a Private IP if APIServerILB feature is disabled.
if privateIPCount > 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP"),
"Public Load Balancers cannot have a Private IP"))
}
}
}

// internal LB should not have a public IP.
if lb.Type == Internal {
if publicIPCount != 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("frontendIPConfigs").Index(0).Child("publicIP"),
"Internal Load Balancers cannot have a Public IP"))
}
if privateIPCount != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("frontendIPConfigs"), lb.FrontendIPs,
"API Server Load balancer of type private should have 1 frontend private IP"))
} else {
if err := validateInternalLBIPAddress(lb.FrontendIPs[0].PrivateIPAddress, cidrs,
fldPath.Child("frontendIPConfigs").Index(0).Child("privateIP")); err != nil {
allErrs = append(allErrs, err)
}

if len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation."))
}

Check warning on line 462 in api/v1beta1/azurecluster_validation.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta1/azurecluster_validation.go#L461-L462 

Added lines #L461 - L462 were not covered by tests
}
}
return allErrs
}

Expand Down
Loading

0 comments on commit e0f2b44

Please sign in to comment.