Skip to content

Commit

Permalink
Merge pull request #754 from CecileRobertMichon/pip-nodes
Browse files Browse the repository at this point in the history
🐛 Add secondary network interface for node public IPs
  • Loading branch information
k8s-ci-robot authored Jul 15, 2020
2 parents 387db5a + 9b6b282 commit 8133094
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 27 deletions.
11 changes: 8 additions & 3 deletions cloud/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,21 @@ func GenerateNodeOutboundIPName(clusterName string) string {
return fmt.Sprintf("pip-%s-node-outbound", clusterName)
}

// GenerateNodePublicIPName generates a node public IP name, based on the NIC name.
func GenerateNodePublicIPName(nicName string) string {
return fmt.Sprintf("%s-public-ip", nicName)
// GenerateNodePublicIPName generates a node public IP name, based on the machine name.
func GenerateNodePublicIPName(machineName string) string {
return fmt.Sprintf("pip-%s", machineName)
}

// GenerateNICName generates the name of a network interface based on the name of a VM.
func GenerateNICName(machineName string) string {
return fmt.Sprintf("%s-nic", machineName)
}

// GeneratePublicNICName generates the name of a public network interface based on the name of a VM.
func GeneratePublicNICName(machineName string) string {
return fmt.Sprintf("%s-public-nic", machineName)
}

// GenerateOSDiskName generates the name of an OS disk based on the name of a VM.
func GenerateOSDiskName(machineName string) string {
return fmt.Sprintf("%s_OSDisk", machineName)
Expand Down
18 changes: 14 additions & 4 deletions cloud/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,8 @@ type MachineScope struct {
func (m *MachineScope) PublicIPSpecs() []azure.PublicIPSpec {
var spec []azure.PublicIPSpec
if m.AzureMachine.Spec.AllocatePublicIP == true {
nicName := azure.GenerateNICName(m.Name())
spec = append(spec, azure.PublicIPSpec{
Name: azure.GenerateNodePublicIPName(nicName),
Name: azure.GenerateNodePublicIPName(m.Name()),
})
}
return spec
Expand All @@ -116,11 +115,22 @@ func (m *MachineScope) NICSpecs() []azure.NICSpec {
} else if m.Role() == infrav1.Node {
spec.PublicLoadBalancerName = m.ClusterName()
}
specs := []azure.NICSpec{spec}
if m.AzureMachine.Spec.AllocatePublicIP == true {
spec.PublicIPName = azure.GenerateNodePublicIPName(azure.GenerateNICName(m.Name()))
specs = append(specs, azure.NICSpec{
Name: azure.GeneratePublicNICName(m.Name()),
MachineName: m.Name(),
MachineRole: m.Role(),
VNetName: m.Vnet().Name,
VNetResourceGroup: m.Vnet().ResourceGroup,
SubnetName: m.Subnet().Name,
PublicIPName: azure.GenerateNodePublicIPName(m.Name()),
VMSize: m.AzureMachine.Spec.VMSize,
AcceleratedNetworking: m.AzureMachine.Spec.AcceleratedNetworking,
})
}

return []azure.NICSpec{spec}
return specs
}

// DiskSpecs returns the public IP specs.
Expand Down
3 changes: 1 addition & 2 deletions cloud/services/networkinterfaces/networkinterfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ func (s *Service) Reconcile(ctx context.Context) error {
if err != nil {
return errors.Wrap(err, "failed to get subnets")
}

nicConfig.Subnet = &network.Subnet{ID: subnet.ID}

nicConfig.PrivateIPAllocationMethod = network.Dynamic
if nicSpec.StaticIPAddress != "" {
nicConfig.PrivateIPAllocationMethod = network.Static
Expand All @@ -50,7 +50,6 @@ func (s *Service) Reconcile(ctx context.Context) error {
if lberr != nil {
return errors.Wrap(lberr, "failed to get public LB")
}

backendAddressPools = append(backendAddressPools,
network.BackendAddressPool{
ID: (*lb.BackendAddressPools)[0].ID,
Expand Down
15 changes: 13 additions & 2 deletions cloud/services/networkinterfaces/networkinterfaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,21 @@ func TestReconcileNetworkInterface(t *testing.T) {
VNetName: "my-vnet",
VNetResourceGroup: "my-rg",
PublicLoadBalancerName: "my-public-lb",
PublicIPName: "my-public-ip",
InternalLoadBalancerName: "my-internal-lb",
VMSize: "Standard_D2v2",
AcceleratedNetworking: nil,
},
{
Name: "my-public-net-interface",
MachineName: "azure-test1",
MachineRole: infrav1.Node,
SubnetName: "my-subnet",
VNetName: "my-vnet",
VNetResourceGroup: "my-rg",
PublicIPName: "my-public-ip",
VMSize: "Standard_D2v2",
AcceleratedNetworking: nil,
},
})
s.ResourceGroup().AnyTimes().Return("my-rg")
s.Location().AnyTimes().Return("fake-location")
Expand All @@ -461,7 +471,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
mLoadBalancer.Get(context.TODO(), "my-rg", "my-public-lb").Return(getFakeNodeOutboundLoadBalancer(), nil),
mPublicIP.Get(context.TODO(), "my-rg", "my-public-ip").Return(network.PublicIPAddress{}, nil),
mResourceSku.HasAcceleratedNetworking(gomock.Any(), gomock.Any()),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})))
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-public-net-interface", gomock.AssignableToTypeOf(network.Interface{})))
},
},
{
Expand Down
31 changes: 17 additions & 14 deletions cloud/services/virtualmachines/virtualmachines.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const azureBuiltInContributorID = "b24988ac-6180-42a0-ab88-20f7382dd24c"
// Spec input specification for Get/CreateOrUpdate/Delete calls
type Spec struct {
Name string
NICName string
NICNames []string
SSHKeyData string
Size string
Zone string
Expand Down Expand Up @@ -90,12 +90,22 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
return err
}

s.Scope.V(2).Info("getting network interface", "network interface", vmSpec.NICName)
nic, err := s.InterfacesClient.Get(ctx, s.Scope.ResourceGroup(), vmSpec.NICName)
if err != nil {
return err
nicRefs := make([]compute.NetworkInterfaceReference, len(vmSpec.NICNames))
for i, nicName := range vmSpec.NICNames {
primary := i == 0
s.Scope.V(2).Info("getting network interface", "network interface", nicName)
nic, err := s.InterfacesClient.Get(ctx, s.Scope.ResourceGroup(), nicName)
if err != nil {
return err
}
s.Scope.V(2).Info("got network interface", "network interface", nicName)
nicRefs[i] = compute.NetworkInterfaceReference{
ID: nic.ID,
NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{
Primary: to.BoolPtr(primary),
},
}
}
s.Scope.V(2).Info("got network interface", "network interface", vmSpec.NICName)

s.Scope.V(2).Info("creating VM", "vm", vmSpec.Name)

Expand Down Expand Up @@ -140,14 +150,7 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
},
},
NetworkProfile: &compute.NetworkProfile{
NetworkInterfaces: &[]compute.NetworkInterfaceReference{
{
ID: nic.ID,
NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{
Primary: to.BoolPtr(true),
},
},
},
NetworkInterfaces: &nicRefs,
},
Priority: priority,
EvictionPolicy: evictionPolicy,
Expand Down
2 changes: 1 addition & 1 deletion cloud/services/virtualmachines/virtualmachines_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ func TestReconcileVM(t *testing.T) {

vmSpec := &Spec{
Name: machineScope.Name(),
NICName: "test-nic",
NICNames: []string{"test-nic"},
SSHKeyData: "fake-key",
Size: machineScope.AzureMachine.Spec.VMSize,
OSDisk: machineScope.AzureMachine.Spec.OSDisk,
Expand Down
7 changes: 6 additions & 1 deletion controllers/azuremachine_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,11 @@ func (s *azureMachineService) reconcileVirtualMachine(ctx context.Context, nicNa
}
}

nicNames := []string{nicName}
if s.machineScope.AzureMachine.Spec.AllocatePublicIP == true {
nicNames = append(nicNames, azure.GeneratePublicNICName(s.machineScope.Name()))
}

image, err := getVMImage(s.machineScope)
if err != nil {
return nil, errors.Wrap(err, "failed to get VM image")
Expand All @@ -221,7 +226,7 @@ func (s *azureMachineService) reconcileVirtualMachine(ctx context.Context, nicNa

vmSpec := &virtualmachines.Spec{
Name: s.machineScope.Name(),
NICName: nicName,
NICNames: nicNames,
SSHKeyData: string(decoded),
Size: s.machineScope.AzureMachine.Spec.VMSize,
OSDisk: s.machineScope.AzureMachine.Spec.OSDisk,
Expand Down

0 comments on commit 8133094

Please sign in to comment.