Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple NIC support #2

Merged
merged 10 commits into from
Jun 22, 2022
Merged

Multiple NIC support #2

merged 10 commits into from
Jun 22, 2022

Conversation

brianlieberman
Copy link

@brianlieberman brianlieberman commented Jun 7, 2022

/kind feature

What this PR does / why we need it:
This PR adds support for attaching multiple network interfaces to both AzureMachines as well as AzureMachinePools (VMSS), it also allows to specify one or more IPConfigs for those interfaces to pre-warm them with multiple IPs on each interface.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes kubernetes-sigs#2327

Special notes for your reviewer:

  • [x ] squashed commits
  • [x ] includes documentation
  • [ x] adds unit tests

Release note:

Adds a new optional field for a slice of NetworkInterfaces for AzureMachines and AzureMachinePools to specify a list of NetworkInterfaces, the SubnetName each should be in, and a slice of optional IPConfigs for each interface if desire.

Example below create a machine template with two interfaces in different subnets, and five IPs on the node-subnet interface: 

apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AzureMachineTemplate
metadata:
  name: capi-quickstart-control-plane
  namespace: default
spec:
  template:
    spec:
       networkInterfaces:
           - subnetName: control-plane-subnet
             acceleratedNetworking: false
           - subnetName: node-subnet
             acceleratedNetworking: true
             ipConfigs:
              - {}
              - {}
              - {}
              - {}
              - {}
      dataDisks:
      - diskSizeGB: 256
        lun: 0
        nameSuffix: etcddisk
      osDisk:
        diskSizeGB: 128
        osType: Linux
      sshPublicKey: ""
      vmSize: Standard_D2s_v3

@brianlieberman brianlieberman changed the title Multiple nics Multiple NIC support Jun 7, 2022
@dthorsen
Copy link

dthorsen commented Jun 7, 2022

@brianlieberman Some of the functions have modifications but no corresponding tests.

I am also getting some linter errors when running make lint

azure/types.go:23:2: Expected 'i', Found '"' at azure/types.go[line 23,col 2] (gci)
	"sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
	^
azure/types.go:24:2: ST1019(related information): other import of "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" (stylecheck)
	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
	^
api/v1beta1/types.go:555:68: Comment should end in a period (godot)
// IP Configuration defines options to confiure a network interface
                                                                   ^
api/v1beta1/types.go:550:2: var-naming: struct field IpConfigs should be IPConfigs (revive)
	IpConfigs             []AzureIPConfig `json:"ipConfigs,omitempty"`
	^
api/v1beta1/types.go:552:2: var-naming: struct field Id should be ID (revive)
	Id                    string          `json:"id,omitempty"`
	^
azure/scope/machine.go:223: unnecessary leading newline (whitespace)
func (m *MachineScope) NICSpecs() []azure.ResourceSpecGetter {

azure/services/networkinterfaces/spec.go:146:3: var-naming: var newIpConfigPropertiesFormat should be newIPConfigPropertiesFormat (revive)
		newIpConfigPropertiesFormat := &network.InterfaceIPConfigurationPropertiesFormat{}
		^
azure/services/scalesets/scalesets.go:486: File is not `gofmt`-ed with `-s` (gofmt)
					for j, _ := range n.IpConfigs {
exp/api/v1beta1/azuremachinepool_types.go:22:2: Expected 'i', Found '"' at exp/api/v1beta1/azuremachinepool_types.go[line 22,col 2] (gci)
	"sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
	^
exp/api/v1beta1/azuremachinepool_types.go:23:2: ST1019(related information): other import of "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" (stylecheck)
	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
	^

@dthorsen
Copy link

dthorsen commented Jun 7, 2022

This also needs a make generate-manifests run and committed. There are changes to the azuremachinepools CRD yaml that need to be generated.

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml
index a206f603..0b5229f1 100644
--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml
@@ -1647,11 +1647,16 @@ spec:
                     description: Network Interfaces to attach to the to a virtual
                       machine
                     items:
+                      description: Network Interfaces to attach to each VM
                       properties:
                         acceleratedNetworking:
                           type: boolean
+                        id:
+                          type: string
                         ipConfigs:
                           items:
+                            description: IP Configuration defines options to confiure
+                              a network interface
                             properties:
                               privateIP:
                                 type: string

@dthorsen dthorsen self-assigned this Jun 21, 2022
Copy link

@dthorsen dthorsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@brianlieberman brianlieberman merged commit 9ab3955 into main Jun 22, 2022
@trutx trutx deleted the multiple-nics branch July 6, 2022 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support configurable Network Interfaces
2 participants