Skip to content

Commit

Permalink
switch to int-based ipConfig management + feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Lieberman committed Jun 24, 2022
1 parent 9ab3955 commit 3685380
Show file tree
Hide file tree
Showing 10 changed files with 142 additions and 110 deletions.
8 changes: 4 additions & 4 deletions api/v1beta1/azuremachine_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,17 @@ func TestAzureMachine_ValidateCreate(t *testing.T) {
},
{
name: "azuremachine with invalid network configuration",
machine: createrMachineWithNetworkConfig("subnet", []AzureNetworkInterface{{SubnetName: "subnet1"}}),
machine: createMachineWithNetworkConfig("subnet", []AzureNetworkInterface{{SubnetName: "subnet1"}}),
wantErr: true,
},
{
name: "azuremachine with valid legacy network configuration",
machine: createrMachineWithNetworkConfig("subnet", []AzureNetworkInterface{}),
machine: createMachineWithNetworkConfig("subnet", []AzureNetworkInterface{}),
wantErr: false,
},
{
name: "azuremachine with valid network configuration",
machine: createrMachineWithNetworkConfig("", []AzureNetworkInterface{{SubnetName: "subnet"}}),
machine: createMachineWithNetworkConfig("", []AzureNetworkInterface{{SubnetName: "subnet"}}),
wantErr: false,
},
}
Expand Down Expand Up @@ -621,7 +621,7 @@ func TestAzureMachine_Default(t *testing.T) {
}
}

func createrMachineWithNetworkConfig(subnetName string, interfaces []AzureNetworkInterface) *AzureMachine {
func createMachineWithNetworkConfig(subnetName string, interfaces []AzureNetworkInterface) *AzureMachine {
return &AzureMachine{
Spec: AzureMachineSpec{
SubnetName: subnetName,
Expand Down
23 changes: 19 additions & 4 deletions api/v1beta1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,10 +582,25 @@ type SubnetSpec struct {
// Network Interfaces to attach to each VM
// +optional
type AzureNetworkInterface struct {
SubnetName string `json:"subnetName,omitempty"`
IPConfigs []AzureIPConfig `json:"ipConfigs,omitempty"`
AcceleratedNetworking *bool `json:"acceleratedNetworking,omitempty"`
ID string `json:"id,omitempty"`
// The subnet to place the interface in
SubnetName string `json:"subnetName,omitempty"`

// Number of private IP address to attach to the interface
// +optional
PrivateIPConfigs int `json:"privateIPConfigs,omitempty"`

// Number of public IP addresses to attach to the interface
// +optional

PublicIPConfigs int `json:"publicIPConfigs,omitempty"`

// Enable acccelerated networking on the interface
// +optional
AcceleratedNetworking *bool `json:"acceleratedNetworking,omitempty"`

// Attach an already provisioned interface by ID
// +optional
ID string `json:"id,omitempty"`
}

// IP Configuration defines options to confiure a network interface.
Expand Down
5 changes: 0 additions & 5 deletions api/v1beta1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 51 additions & 45 deletions azure/scope/machine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,45 +224,7 @@ func (m *MachineScope) NICSpecs() []azure.ResourceSpecGetter {
nicSpecs := []azure.ResourceSpecGetter{}

if len(m.AzureMachine.Spec.NetworkInterfaces) < 1 {
spec := &networkinterfaces.NICSpec{
Name: azure.GenerateNICName(m.Name()),
ResourceGroup: m.ResourceGroup(),
Location: m.Location(),
SubscriptionID: m.SubscriptionID(),
MachineName: m.Name(),
VNetName: m.Vnet().Name,
VNetResourceGroup: m.Vnet().ResourceGroup,
AcceleratedNetworking: m.AzureMachine.Spec.AcceleratedNetworking,
IPv6Enabled: m.IsIPv6Enabled(),
EnableIPForwarding: m.AzureMachine.Spec.EnableIPForwarding,
SubnetName: m.Subnet().Name,
}
if m.Role() == infrav1.ControlPlane {
spec.PublicLBName = m.OutboundLBName(m.Role())
spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))
if m.IsAPIServerPrivate() {
spec.InternalLBName = m.APIServerLBName()
spec.InternalLBAddressPoolName = m.APIServerLBPoolName(m.APIServerLBName())
} else {
spec.PublicLBNATRuleName = m.Name()
spec.PublicLBAddressPoolName = m.APIServerLBPoolName(m.APIServerLBName())
}
}

// If NAT gateway is not enabled and node has no public IP, then the NIC needs to reference the LB to get outbound traffic.
if m.Role() == infrav1.Node && !m.Subnet().IsNatGatewayEnabled() && !m.AzureMachine.Spec.AllocatePublicIP {
spec.PublicLBName = m.OutboundLBName(m.Role())
spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))
}

if m.cache != nil {
spec.SKU = &m.cache.VMSKU
}

if m.Role() == infrav1.Node && m.AzureMachine.Spec.AllocatePublicIP {
spec.PublicIPName = azure.GenerateNodePublicIPName(m.Name())
}
return append(nicSpecs, spec)
return append(nicSpecs, getDefaultNetworkInterfaceSpec(m))
}

for i, n := range m.AzureMachine.Spec.NetworkInterfaces {
Expand Down Expand Up @@ -313,13 +275,16 @@ func (m *MachineScope) NICSpecs() []azure.ResourceSpecGetter {
spec.SKU = &m.cache.VMSKU
}

for _, c := range n.IPConfigs {
config := networkinterfaces.IPConfig{
PublicIP: c.PublicIP,
PrivateIP: c.PrivateIP,
PublicIPAddress: c.PublicIPAddress,
// Create an IPconfig for both public + private IP pair
if n.PrivateIPConfigs >= n.PublicIPConfigs {
for i := 0; i < n.PublicIPConfigs; i++ {
spec.IPConfigs = append(spec.IPConfigs, networkinterfaces.IPConfig{PublicIP: true})
}
spec.IPConfigs = append(spec.IPConfigs, config)
}

// Create any remaining private IPConfigs
for i := 0; i < n.PrivateIPConfigs-n.PublicIPConfigs; i++ {
spec.IPConfigs = append(spec.IPConfigs, networkinterfaces.IPConfig{})
}
nicSpecs = append(nicSpecs, spec)
}
Expand All @@ -340,6 +305,47 @@ func (m *MachineScope) NICIDs() []string {
}
return nicIDs
}
func getDefaultNetworkInterfaceSpec(m *MachineScope) *networkinterfaces.NICSpec {
spec := &networkinterfaces.NICSpec{
Name: azure.GenerateNICName(m.Name()),
ResourceGroup: m.ResourceGroup(),
Location: m.Location(),
SubscriptionID: m.SubscriptionID(),
MachineName: m.Name(),
VNetName: m.Vnet().Name,
VNetResourceGroup: m.Vnet().ResourceGroup,
AcceleratedNetworking: m.AzureMachine.Spec.AcceleratedNetworking,
IPv6Enabled: m.IsIPv6Enabled(),
EnableIPForwarding: m.AzureMachine.Spec.EnableIPForwarding,
SubnetName: m.Subnet().Name,
}
if m.Role() == infrav1.ControlPlane {
spec.PublicLBName = m.OutboundLBName(m.Role())
spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))
if m.IsAPIServerPrivate() {
spec.InternalLBName = m.APIServerLBName()
spec.InternalLBAddressPoolName = m.APIServerLBPoolName(m.APIServerLBName())
} else {
spec.PublicLBNATRuleName = m.Name()
spec.PublicLBAddressPoolName = m.APIServerLBPoolName(m.APIServerLBName())
}
}

// If NAT gateway is not enabled and node has no public IP, then the NIC needs to reference the LB to get outbound traffic.
if m.Role() == infrav1.Node && !m.Subnet().IsNatGatewayEnabled() && !m.AzureMachine.Spec.AllocatePublicIP {
spec.PublicLBName = m.OutboundLBName(m.Role())
spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))
}

if m.cache != nil {
spec.SKU = &m.cache.VMSKU
}

if m.Role() == infrav1.Node && m.AzureMachine.Spec.AllocatePublicIP {
spec.PublicIPName = azure.GenerateNodePublicIPName(m.Name())
}
return spec
}

// DiskSpecs returns the disk specs.
func (m *MachineScope) DiskSpecs() []azure.ResourceSpecGetter {
Expand Down
11 changes: 6 additions & 5 deletions azure/scope/machine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2145,12 +2145,13 @@ func TestMachineScope_NICSpecs(t *testing.T) {
{
SubnetName: "subnet1",
AcceleratedNetworking: pointer.Bool(true),
IPConfigs: []infrav1.AzureIPConfig{{}},
PrivateIPConfigs: 2,
},
{
SubnetName: "subnet2",
AcceleratedNetworking: pointer.Bool(true),
IPConfigs: []infrav1.AzureIPConfig{{}},
PrivateIPConfigs: 2,
PublicIPConfigs: 1,
},
},
},
Expand All @@ -2170,7 +2171,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
SubscriptionID: "123",
MachineName: "machine-name",
SubnetName: "subnet1",
IPConfigs: []networkinterfaces.IPConfig{{}},
IPConfigs: []networkinterfaces.IPConfig{{}, {}},
VNetName: "vnet1",
VNetResourceGroup: "rg1",
PublicLBName: "outbound-lb",
Expand All @@ -2191,7 +2192,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
SubscriptionID: "123",
MachineName: "machine-name",
SubnetName: "subnet2",
IPConfigs: []networkinterfaces.IPConfig{{}},
IPConfigs: []networkinterfaces.IPConfig{{PublicIP: true}, {}},
VNetName: "vnet1",
VNetResourceGroup: "rg1",
PublicLBName: "",
Expand Down Expand Up @@ -2274,7 +2275,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
{
SubnetName: "subnet1",
AcceleratedNetworking: pointer.Bool(true),
IPConfigs: []infrav1.AzureIPConfig{{}, {}, {}, {}, {}, {}, {}, {}, {}, {}},
PrivateIPConfigs: 10,
},
},
},
Expand Down
32 changes: 29 additions & 3 deletions azure/services/scalesets/scalesets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ func (s *Service) buildVMSSFromSpec(ctx context.Context, vmssSpec azure.ScaleSet
} else {
nicConfig.VirtualMachineScaleSetNetworkConfigurationProperties.EnableAcceleratedNetworking = to.BoolPtr(false)
}
if len(n.IPConfigs) == 0 {
if n.PrivateIPConfigs == 0 && n.PublicIPConfigs == 0 {
nicConfig.VirtualMachineScaleSetNetworkConfigurationProperties.IPConfigurations = &[]compute.VirtualMachineScaleSetIPConfiguration{
{
Name: to.StringPtr(vmssSpec.Name + "-" + strconv.Itoa(i)),
Expand All @@ -483,9 +483,35 @@ func (s *Service) buildVMSSFromSpec(ctx context.Context, vmssSpec azure.ScaleSet
}
} else {
ipconfigs := []compute.VirtualMachineScaleSetIPConfiguration{}
for j := range n.IPConfigs {
for j := 0; j < n.PublicIPConfigs; j++ {
ipconfig := compute.VirtualMachineScaleSetIPConfiguration{
Name: to.StringPtr(fmt.Sprintf("ipConfig%v", j)),
Name: to.StringPtr(fmt.Sprintf("public-ipConfig%v", j)),
VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{
PrivateIPAddressVersion: compute.IPVersionIPv4,
Subnet: &compute.APIEntityReference{
ID: to.StringPtr(azure.SubnetID(s.Scope.SubscriptionID(), vmssSpec.VNetResourceGroup, vmssSpec.VNetName, n.SubnetName)),
},
PublicIPAddressConfiguration: &compute.VirtualMachineScaleSetPublicIPAddressConfiguration{},
},
}
if j == 0 {
ipconfig.Primary = to.BoolPtr(true)
if i == 0 {
// only set Load Balancer Backend Address Pool on primary nic/ipconfig
ipconfig.LoadBalancerBackendAddressPools = &backendAddressPools
}
} else {
ipconfig.Primary = to.BoolPtr(false)
}
ipconfig.Subnet = &compute.APIEntityReference{
ID: to.StringPtr(azure.SubnetID(s.Scope.SubscriptionID(), vmssSpec.VNetResourceGroup, vmssSpec.VNetName, n.SubnetName)),
}
ipconfigs = append(ipconfigs, ipconfig)
}
// Create any remaining private IPConfigs
for j := 0; j < n.PrivateIPConfigs-n.PublicIPConfigs; j++ {
ipconfig := compute.VirtualMachineScaleSetIPConfiguration{
Name: to.StringPtr(fmt.Sprintf("private-ipConfig%v", j)),
VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{
PrivateIPAddressVersion: compute.IPVersionIPv4,
Subnet: &compute.APIEntityReference{
Expand Down
10 changes: 5 additions & 5 deletions azure/services/scalesets/scalesets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,12 @@ func TestReconcileVMSS(t *testing.T) {
spec.NetworkInterfaces = []infrav1.AzureNetworkInterface{
{
SubnetName: "my-subnet",
IPConfigs: []infrav1.AzureIPConfig{{}},
PrivateIPConfigs: 1,
AcceleratedNetworking: pointer.Bool(true),
},
{
SubnetName: "subnet2",
IPConfigs: []infrav1.AzureIPConfig{{}, {}},
PrivateIPConfigs: 2,
AcceleratedNetworking: pointer.Bool(true),
},
}
Expand All @@ -288,12 +288,12 @@ func TestReconcileVMSS(t *testing.T) {
(*netConfigs)[0].Name = to.StringPtr("my-vmss-0")
(*netConfigs)[0].EnableIPForwarding = nil
nic1IPConfigs := (*netConfigs)[0].IPConfigurations
(*nic1IPConfigs)[0].Name = to.StringPtr("ipConfig0")
(*nic1IPConfigs)[0].Name = to.StringPtr("private-ipConfig0")
(*nic1IPConfigs)[0].PrivateIPAddressVersion = compute.IPVersionIPv4
(*netConfigs)[0].EnableAcceleratedNetworking = to.BoolPtr(true)
vmssIPConfigs := []compute.VirtualMachineScaleSetIPConfiguration{
{
Name: to.StringPtr("ipConfig0"),
Name: to.StringPtr("private-ipConfig0"),
VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{
Primary: to.BoolPtr(true),
PrivateIPAddressVersion: compute.IPVersionIPv4,
Expand All @@ -304,7 +304,7 @@ func TestReconcileVMSS(t *testing.T) {
},
},
{
Name: to.StringPtr("ipConfig1"),
Name: to.StringPtr("private-ipConfig1"),
VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{
Primary: to.BoolPtr(false),
PrivateIPAddressVersion: compute.IPVersionIPv4,
Expand Down
23 changes: 10 additions & 13 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1712,23 +1712,20 @@ spec:
description: Network Interfaces to attach to each VM
properties:
acceleratedNetworking:
description: Enable acccelerated networking on the interface
type: boolean
id:
description: Attach an already provisioned interface by
ID
type: string
ipConfigs:
items:
description: IP Configuration defines options to confiure
a network interface.
properties:
privateIP:
type: string
publicIP:
type: boolean
publicIPAddress:
type: string
type: object
type: array
privateIPConfigs:
description: Number of private IP address to attach to the
interface
type: integer
publicIPConfigs:
type: integer
subnetName:
description: The subnet to place the interface in
type: string
type: object
type: array
Expand Down
21 changes: 8 additions & 13 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1317,23 +1317,18 @@ spec:
description: Network Interfaces to attach to each VM
properties:
acceleratedNetworking:
description: Enable acccelerated networking on the interface
type: boolean
id:
description: Attach an already provisioned interface by ID
type: string
ipConfigs:
items:
description: IP Configuration defines options to confiure
a network interface.
properties:
privateIP:
type: string
publicIP:
type: boolean
publicIPAddress:
type: string
type: object
type: array
privateIPConfigs:
description: Number of private IP address to attach to the interface
type: integer
publicIPConfigs:
type: integer
subnetName:
description: The subnet to place the interface in
type: string
type: object
type: array
Expand Down
Loading

0 comments on commit 3685380

Please sign in to comment.