Skip to content

Commit

Permalink
✨ Add support for multiple network interfaces
Browse files Browse the repository at this point in the history
Co-authored-by: Dane Thorsen <[email protected]>
  • Loading branch information
Brian Lieberman and dthorsen committed Dec 22, 2022
1 parent 27ff8d7 commit 4bec8b8
Show file tree
Hide file tree
Showing 47 changed files with 1,853 additions and 84 deletions.
5 changes: 5 additions & 0 deletions api/v1alpha3/azuremachine_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ func (src *AzureMachine) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Diagnostics = restored.Spec.Diagnostics
}

if restored.Spec.NetworkInterfaces != nil {
dst.Spec.NetworkInterfaces = restored.Spec.NetworkInterfaces
}

//nolint:staticcheck // SubnetName is now deprecated, but the v1beta1 defaulting webhook will migrate it to the networkInterfaces field
dst.Spec.SubnetName = restored.Spec.SubnetName

dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha3/azuremachinetemplate_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func (src *AzureMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Template.Spec.Diagnostics = restored.Spec.Template.Spec.Diagnostics
}

//nolint:staticcheck // SubnetName is now deprecated, but the v1beta1 defaulting webhook will migrate it to the networkInterfaces field
dst.Spec.Template.Spec.SubnetName = restored.Spec.Template.Spec.SubnetName
dst.Spec.Template.ObjectMeta = restored.Spec.Template.ObjectMeta

Expand All @@ -77,6 +78,10 @@ func (src *AzureMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Template.Spec.SpotVMOptions.EvictionPolicy = restored.Spec.Template.Spec.SpotVMOptions.EvictionPolicy
}

if restored.Spec.Template.Spec.NetworkInterfaces != nil {
dst.Spec.Template.Spec.NetworkInterfaces = restored.Spec.Template.Spec.NetworkInterfaces
}

return nil
}

Expand Down
1 change: 1 addition & 0 deletions api/v1alpha3/zz_generated.conversion.go

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

4 changes: 4 additions & 0 deletions api/v1alpha4/azuremachine_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ func (src *AzureMachine) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Diagnostics = restored.Spec.Diagnostics
}

if restored.Spec.NetworkInterfaces != nil {
dst.Spec.NetworkInterfaces = restored.Spec.NetworkInterfaces
}

return nil
}

Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha4/azuremachinetemplate_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ func (src *AzureMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Template.Spec.SpotVMOptions.EvictionPolicy = restored.Spec.Template.Spec.SpotVMOptions.EvictionPolicy
}

if restored.Spec.Template.Spec.NetworkInterfaces != nil {
dst.Spec.Template.Spec.NetworkInterfaces = restored.Spec.Template.Spec.NetworkInterfaces
}

return nil
}

Expand Down
1 change: 1 addition & 0 deletions api/v1alpha4/zz_generated.conversion.go

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

29 changes: 29 additions & 0 deletions api/v1beta1/azuremachine_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,34 @@ func (s *AzureMachineSpec) SetDiagnosticsDefaults() {
}
}

// SetNetworkInterfacesDefaults sets the defaults for the network interfaces.
func (s *AzureMachineSpec) SetNetworkInterfacesDefaults() {
// Ensure the deprecated fields and new fields are not populated simultaneously
if (s.SubnetName != "" || s.AcceleratedNetworking != nil) && len(s.NetworkInterfaces) > 0 {
// Both the deprecated and the new fields are both set, return without changes
// and reject the request in the validating webhook which runs later.
return
}

if len(s.NetworkInterfaces) == 0 {
s.NetworkInterfaces = []NetworkInterface{
{
SubnetName: s.SubnetName,
AcceleratedNetworking: s.AcceleratedNetworking,
},
}
s.SubnetName = ""
s.AcceleratedNetworking = nil
}

// Ensure that PrivateIPConfigs defaults to 1 if not specified.
for i := 0; i < len(s.NetworkInterfaces); i++ {
if s.NetworkInterfaces[i].PrivateIPConfigs == 0 {
s.NetworkInterfaces[i].PrivateIPConfigs = 1
}
}
}

// SetDefaults sets to the defaults for the AzureMachineSpec.
func (s *AzureMachineSpec) SetDefaults() {
if err := s.SetDefaultSSHPublicKey(); err != nil {
Expand All @@ -125,4 +153,5 @@ func (s *AzureMachineSpec) SetDefaults() {
s.SetIdentityDefaults()
s.SetSpotEvictionPolicyDefaults()
s.SetDiagnosticsDefaults()
s.SetNetworkInterfacesDefaults()
}
81 changes: 81 additions & 0 deletions api/v1beta1/azuremachine_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,87 @@ func TestAzureMachineSpec_SetDataDisksDefaults(t *testing.T) {
}
}

func TestAzureMachineSpec_SetNetworkInterfacesDefaults(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
machine *AzureMachine
want *AzureMachine
}{
{
name: "defaulting webhook updates machine with deprecated subnetName field",
machine: &AzureMachine{
Spec: AzureMachineSpec{
SubnetName: "test-subnet",
},
},
want: &AzureMachine{
Spec: AzureMachineSpec{
SubnetName: "",
NetworkInterfaces: []NetworkInterface{
{
SubnetName: "test-subnet",
PrivateIPConfigs: 1,
},
},
},
},
},
{
name: "defaulting webhook updates machine with deprecated acceleratedNetworking field",
machine: &AzureMachine{
Spec: AzureMachineSpec{
SubnetName: "test-subnet",
AcceleratedNetworking: to.BoolPtr(true),
},
},
want: &AzureMachine{
Spec: AzureMachineSpec{
SubnetName: "",
AcceleratedNetworking: nil,
NetworkInterfaces: []NetworkInterface{
{
SubnetName: "test-subnet",
PrivateIPConfigs: 1,
AcceleratedNetworking: to.BoolPtr(true),
},
},
},
},
},
{
name: "defaulting webhook does nothing if both new and deprecated subnetName fields are set",
machine: &AzureMachine{
Spec: AzureMachineSpec{
SubnetName: "test-subnet",
NetworkInterfaces: []NetworkInterface{{
SubnetName: "test-subnet",
}},
},
},
want: &AzureMachine{
Spec: AzureMachineSpec{
SubnetName: "test-subnet",
AcceleratedNetworking: nil,
NetworkInterfaces: []NetworkInterface{
{
SubnetName: "test-subnet",
},
},
},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
tc.machine.Spec.SetNetworkInterfacesDefaults()
g.Expect(tc.machine).To(Equal(tc.want))
})
}
}

func createMachineWithSSHPublicKey(sshPublicKey string) *AzureMachine {
machine := hardcodedAzureMachineWithSSHKey(sshPublicKey)
return machine
Expand Down
13 changes: 9 additions & 4 deletions api/v1beta1/azuremachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ type AzureMachineSpec struct {
// +optional
EnableIPForwarding bool `json:"enableIPForwarding,omitempty"`

// AcceleratedNetworking enables or disables Azure accelerated networking. If omitted, it will be set based on
// whether the requested VMSize supports accelerated networking.
// If AcceleratedNetworking is set to true with a VMSize that does not support it, Azure will return an error.
// Deprecated: AcceleratedNetworking should be set in the networkInterfaces field.
// +kubebuilder:validation:nullable
// +optional
AcceleratedNetworking *bool `json:"acceleratedNetworking,omitempty"`
Expand All @@ -120,7 +118,7 @@ type AzureMachineSpec struct {
// +optional
SecurityProfile *SecurityProfile `json:"securityProfile,omitempty"`

// SubnetName selects the Subnet where the VM will be placed
// Deprecated: SubnetName should be set in the networkInterfaces field.
// +optional
SubnetName string `json:"subnetName,omitempty"`

Expand All @@ -131,6 +129,13 @@ type AzureMachineSpec struct {
// VMExtensions specifies a list of extensions to be added to the virtual machine.
// +optional
VMExtensions []VMExtension `json:"vmExtensions,omitempty"`

// NetworkInterfaces specifies a list of network interface configurations.
// If left unspecified, the VM will get a single network interface with a
// single IPConfig in the subnet specified in the cluster's node subnet field.
// The primary interface will be the first networkInterface specified (index 0) in the list.
// +optional
NetworkInterfaces []NetworkInterface `json:"networkInterfaces,omitempty"`
}

// SpotVMOptions defines the options relevant to running the Machine on Spot VMs.
Expand Down
23 changes: 23 additions & 0 deletions api/v1beta1/azuremachine_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,32 @@ func ValidateAzureMachineSpec(spec AzureMachineSpec) field.ErrorList {
allErrs = append(allErrs, errs...)
}

if errs := ValidateNetwork(spec.SubnetName, spec.AcceleratedNetworking, spec.NetworkInterfaces, field.NewPath("networkInterfaces")); len(errs) > 0 {
allErrs = append(allErrs, errs...)
}

return allErrs
}

// ValidateNetwork validates the network configuration.
func ValidateNetwork(subnetName string, acceleratedNetworking *bool, networkInterfaces []NetworkInterface, fldPath *field.Path) field.ErrorList {
if (networkInterfaces != nil) && len(networkInterfaces) > 0 && subnetName != "" {
return field.ErrorList{field.Invalid(fldPath, networkInterfaces, "cannot set both networkInterfaces and machine subnetName")}
}

if (networkInterfaces != nil) && len(networkInterfaces) > 0 && acceleratedNetworking != nil {
return field.ErrorList{field.Invalid(fldPath, networkInterfaces, "cannot set both networkInterfaces and machine acceleratedNetworking")}
}

for _, nic := range networkInterfaces {
if nic.PrivateIPConfigs < 1 {
return field.ErrorList{field.Invalid(fldPath, networkInterfaces, "number of privateIPConfigs per interface must be at least 1")}
}
}

return field.ErrorList{}
}

// ValidateSSHKey validates an SSHKey.
func ValidateSSHKey(sshKey string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down
93 changes: 93 additions & 0 deletions api/v1beta1/azuremachine_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,3 +694,96 @@ func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) {
})
}
}

func TestAzureMachine_ValidateNetwork(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
subnetName string
acceleratedNetworking *bool
networkInterfaces []NetworkInterface
wantErr bool
}{
{
name: "valid config with deprecated network fields",
subnetName: "subnet1",
acceleratedNetworking: to.BoolPtr(true),
networkInterfaces: nil,
wantErr: false,
},
{
name: "valid config with networkInterfaces fields",
subnetName: "",
acceleratedNetworking: nil,
networkInterfaces: []NetworkInterface{{
SubnetName: "subnet1",
AcceleratedNetworking: to.BoolPtr(true),
PrivateIPConfigs: 1,
}},
wantErr: false,
},
{
name: "valid config with multiple networkInterfaces",
subnetName: "",
acceleratedNetworking: nil,
networkInterfaces: []NetworkInterface{
{
SubnetName: "subnet1",
AcceleratedNetworking: to.BoolPtr(true),
PrivateIPConfigs: 1,
},
{
SubnetName: "subnet2",
AcceleratedNetworking: to.BoolPtr(true),
PrivateIPConfigs: 30,
},
},
wantErr: false,
},
{
name: "invalid config using both deprecated subnetName and networkInterfaces",
subnetName: "subnet1",
acceleratedNetworking: nil,
networkInterfaces: []NetworkInterface{{
SubnetName: "subnet1",
AcceleratedNetworking: nil,
PrivateIPConfigs: 1,
}},
wantErr: true,
},
{
name: "invalid config using both deprecated acceleratedNetworking and networkInterfaces",
subnetName: "",
acceleratedNetworking: to.BoolPtr(true),
networkInterfaces: []NetworkInterface{{
SubnetName: "subnet1",
AcceleratedNetworking: to.BoolPtr(true),
PrivateIPConfigs: 1,
}},
wantErr: true,
},
{
name: "invalid config setting privateIPConfigs to less than 1",
subnetName: "",
acceleratedNetworking: nil,
networkInterfaces: []NetworkInterface{{
SubnetName: "subnet1",
AcceleratedNetworking: to.BoolPtr(true),
PrivateIPConfigs: 0,
}},
wantErr: true,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := ValidateNetwork(test.subnetName, test.acceleratedNetworking, test.networkInterfaces, field.NewPath("networkInterfaces"))
if test.wantErr {
g.Expect(err).ToNot(BeEmpty())
} else {
g.Expect(err).To(BeEmpty())
}
})
}
}
20 changes: 20 additions & 0 deletions api/v1beta1/azuremachine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package v1beta1

import (
"reflect"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down Expand Up @@ -152,6 +154,24 @@ func (m *AzureMachine) ValidateUpdate(oldRaw runtime.Object) error {
}
}

if !reflect.DeepEqual(m.Spec.NetworkInterfaces, old.Spec.NetworkInterfaces) {
// The defaulting webhook may have migrated values from the old SubnetName field to the new NetworkInterfaces format.
old.Spec.SetNetworkInterfacesDefaults()

// The reconciler will populate the SubnetName on the first interface if the user left it blank.
if old.Spec.NetworkInterfaces[0].SubnetName == "" && m.Spec.NetworkInterfaces[0].SubnetName != "" {
old.Spec.NetworkInterfaces[0].SubnetName = m.Spec.NetworkInterfaces[0].SubnetName
}

// Enforce immutability for all other changes to NetworkInterfaces.
if !reflect.DeepEqual(m.Spec.NetworkInterfaces, old.Spec.NetworkInterfaces) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "networkInterfaces"),
m.Spec.NetworkInterfaces, "field is immutable"),
)
}
}

if len(allErrs) == 0 {
return nil
}
Expand Down
Loading

0 comments on commit 4bec8b8

Please sign in to comment.