Skip to content

Commit e185705

Browse files
author
Cecile Robert-Michon
committed
🐛 Do not update VMSS network profile if scale set already exists
1 parent 72afeba commit e185705

File tree

4 files changed

+283
-0
lines changed

4 files changed

+283
-0
lines changed

‎cloud/services/scalesets/client.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ type Client interface {
3333
ListInstances(context.Context, string, string) ([]compute.VirtualMachineScaleSetVM, error)
3434
Get(context.Context, string, string) (compute.VirtualMachineScaleSet, error)
3535
CreateOrUpdate(context.Context, string, string, compute.VirtualMachineScaleSet) error
36+
Update(context.Context, string, string, compute.VirtualMachineScaleSetUpdate) error
3637
Delete(context.Context, string, string) error
3738
GetPublicIPAddress(context.Context, string, string) (network.PublicIPAddress, error)
3839
}
@@ -130,6 +131,21 @@ func (ac *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroupName, vm
130131
return err
131132
}
132133

134+
// Update update a VM scale set.
135+
// Parameters: resourceGroupName - the name of the resource group. VMScaleSetName - the name of the VM scale set to create or update. parameters - the scale set object.
136+
func (ac *AzureClient) Update(ctx context.Context, resourceGroupName, vmssName string, parameters compute.VirtualMachineScaleSetUpdate) error {
137+
future, err := ac.scalesets.Update(ctx, resourceGroupName, vmssName, parameters)
138+
if err != nil {
139+
return err
140+
}
141+
err = future.WaitForCompletionRef(ctx, ac.scalesets.Client)
142+
if err != nil {
143+
return err
144+
}
145+
_, err = future.Result(ac.scalesets)
146+
return err
147+
}
148+
133149
// Delete the operation to delete a virtual machine scale set.
134150
func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vmssName string) error {
135151
future, err := ac.scalesets.Delete(ctx, resourceGroupName, vmssName)

‎cloud/services/scalesets/mock_scalesets/scalesets_mock.go‎

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cloud/services/scalesets/vmss.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,21 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
175175
},
176176
}
177177

178+
_, err = s.Client.Get(ctx, vmssSpec.ResourceGroup, vmssSpec.Name)
179+
if !azure.ResourceNotFound(err) {
180+
if err != nil {
181+
return errors.Wrapf(err, "failed to get scale set %s in %s", vmssSpec.Name, vmssSpec.ResourceGroup)
182+
}
183+
// scale set already exists, update it
184+
// we do this to avoid overwriting fields in networkProfile modified by cloud-provider
185+
update, err := getVMSSUpdateFromVMSS(vmss)
186+
if err != nil {
187+
return errors.Wrapf(err, "failed to generate scale set update parameters for %s", vmssSpec.Name)
188+
}
189+
update.VirtualMachineProfile.NetworkProfile = nil
190+
return s.Client.Update(ctx, vmssSpec.ResourceGroup, vmssSpec.Name, update)
191+
}
192+
178193
err = s.Client.CreateOrUpdate(
179194
ctx,
180195
vmssSpec.ResourceGroup,
@@ -229,3 +244,13 @@ func generateStorageProfile(vmssSpec Spec) (*compute.VirtualMachineScaleSetStora
229244

230245
return storageProfile, nil
231246
}
247+
248+
func getVMSSUpdateFromVMSS(vmss compute.VirtualMachineScaleSet) (compute.VirtualMachineScaleSetUpdate, error) {
249+
json, err := vmss.MarshalJSON()
250+
if err != nil {
251+
return compute.VirtualMachineScaleSetUpdate{}, err
252+
}
253+
var update compute.VirtualMachineScaleSetUpdate
254+
err = update.UnmarshalJSON(json)
255+
return update, err
256+
}

‎cloud/services/scalesets/vmss_test.go‎

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package scalesets
1919
import (
2020
"context"
2121
"fmt"
22+
"net/http"
2223
"testing"
2324

2425
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
@@ -354,6 +355,7 @@ func TestService_Reconcile(t *testing.T) {
354355

355356
skusMock.EXPECT().HasAcceleratedNetworking(gomock.Any(), gomock.Any()).Return(false, nil)
356357
lbMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.ClusterName).Return(getFakeNodeOutboundLoadBalancer(), nil)
358+
vmssMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name).Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
357359
vmssMock.EXPECT().CreateOrUpdate(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, matchers.DiffEq(vmss)).Return(nil)
358360
},
359361
Expect: func(ctx context.Context, g *gomega.GomegaWithT, err error) {
@@ -465,12 +467,167 @@ func TestService_Reconcile(t *testing.T) {
465467

466468
skusMock.EXPECT().HasAcceleratedNetworking(gomock.Any(), gomock.Any()).Return(true, nil)
467469
lbMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.ClusterName).Return(getFakeNodeOutboundLoadBalancer(), nil)
470+
vmssMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name).Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
468471
vmssMock.EXPECT().CreateOrUpdate(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, matchers.DiffEq(vmss)).Return(nil)
469472
},
470473
Expect: func(ctx context.Context, g *gomega.GomegaWithT, err error) {
471474
g.Expect(err).ToNot(gomega.HaveOccurred())
472475
},
473476
},
477+
{
478+
Name: "Scale Set already exists",
479+
SpecFactory: func(g *gomega.GomegaWithT, scope *scope.ClusterScope, mpScope *scope.MachinePoolScope) interface{} {
480+
return &Spec{
481+
Name: mpScope.Name(),
482+
ResourceGroup: scope.AzureCluster.Spec.ResourceGroup,
483+
Location: scope.AzureCluster.Spec.Location,
484+
ClusterName: scope.Cluster.Name,
485+
SubnetID: scope.AzureCluster.Spec.NetworkSpec.Subnets[0].ID,
486+
PublicLoadBalancerName: scope.Cluster.Name,
487+
MachinePoolName: mpScope.Name(),
488+
Sku: "skuName",
489+
Capacity: 2,
490+
SSHKeyData: "sshKeyData",
491+
OSDisk: infrav1.OSDisk{
492+
OSType: "Linux",
493+
DiskSizeGB: 120,
494+
ManagedDisk: infrav1.ManagedDisk{
495+
StorageAccountType: "accountType",
496+
},
497+
},
498+
Image: &infrav1.Image{
499+
ID: to.StringPtr("image"),
500+
},
501+
CustomData: "customData",
502+
}
503+
},
504+
Setup: func(ctx context.Context, g *gomega.GomegaWithT, svc *Service, scope *scope.ClusterScope, mpScope *scope.MachinePoolScope, spec *Spec) {
505+
mockCtrl := gomock.NewController(t)
506+
vmssMock := mock_scalesets.NewMockClient(mockCtrl)
507+
svc.Client = vmssMock
508+
skusMock := mock_resourceskus.NewMockClient(mockCtrl)
509+
svc.ResourceSkusClient = skusMock
510+
lbMock := mock_publicloadbalancers.NewMockClient(mockCtrl)
511+
svc.PublicLoadBalancersClient = lbMock
512+
513+
storageProfile, err := generateStorageProfile(*spec)
514+
g.Expect(err).ToNot(gomega.HaveOccurred())
515+
516+
vmss := compute.VirtualMachineScaleSet{
517+
Location: to.StringPtr(scope.Location()),
518+
Tags: map[string]*string{
519+
"Name": to.StringPtr("capz-mp-0"),
520+
"kubernetes.io_cluster_capz-mp-0": to.StringPtr("owned"),
521+
"sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": to.StringPtr("owned"),
522+
"sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("node"),
523+
},
524+
Sku: &compute.Sku{
525+
Name: to.StringPtr(spec.Sku),
526+
Tier: to.StringPtr("Standard"),
527+
Capacity: to.Int64Ptr(spec.Capacity),
528+
},
529+
VirtualMachineScaleSetProperties: &compute.VirtualMachineScaleSetProperties{
530+
UpgradePolicy: &compute.UpgradePolicy{
531+
Mode: compute.Manual,
532+
},
533+
VirtualMachineProfile: &compute.VirtualMachineScaleSetVMProfile{
534+
OsProfile: &compute.VirtualMachineScaleSetOSProfile{
535+
ComputerNamePrefix: to.StringPtr(spec.Name),
536+
AdminUsername: to.StringPtr(azure.DefaultUserName),
537+
CustomData: to.StringPtr(spec.CustomData),
538+
LinuxConfiguration: &compute.LinuxConfiguration{
539+
SSH: &compute.SSHConfiguration{
540+
PublicKeys: &[]compute.SSHPublicKey{
541+
{
542+
Path: to.StringPtr(fmt.Sprintf("/home/%s/.ssh/authorized_keys", azure.DefaultUserName)),
543+
KeyData: to.StringPtr(spec.SSHKeyData),
544+
},
545+
},
546+
},
547+
DisablePasswordAuthentication: to.BoolPtr(true),
548+
},
549+
},
550+
StorageProfile: storageProfile,
551+
NetworkProfile: &compute.VirtualMachineScaleSetNetworkProfile{
552+
NetworkInterfaceConfigurations: &[]compute.VirtualMachineScaleSetNetworkConfiguration{
553+
{
554+
Name: to.StringPtr(spec.Name + "-netconfig"),
555+
VirtualMachineScaleSetNetworkConfigurationProperties: &compute.VirtualMachineScaleSetNetworkConfigurationProperties{
556+
Primary: to.BoolPtr(true),
557+
EnableAcceleratedNetworking: to.BoolPtr(false),
558+
EnableIPForwarding: to.BoolPtr(true),
559+
IPConfigurations: &[]compute.VirtualMachineScaleSetIPConfiguration{
560+
{
561+
Name: to.StringPtr(spec.Name + "-ipconfig"),
562+
VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{
563+
Subnet: &compute.APIEntityReference{
564+
ID: to.StringPtr(scope.AzureCluster.Spec.NetworkSpec.Subnets[0].ID),
565+
},
566+
Primary: to.BoolPtr(true),
567+
PrivateIPAddressVersion: compute.IPv4,
568+
LoadBalancerBackendAddressPools: &[]compute.SubResource{{ID: to.StringPtr("cluster-name-outboundBackendPool")}},
569+
},
570+
},
571+
},
572+
},
573+
},
574+
},
575+
},
576+
},
577+
},
578+
}
579+
580+
update := compute.VirtualMachineScaleSetUpdate{
581+
Tags: map[string]*string{
582+
"Name": to.StringPtr("capz-mp-0"),
583+
"kubernetes.io_cluster_capz-mp-0": to.StringPtr("owned"),
584+
"sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": to.StringPtr("owned"),
585+
"sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("node"),
586+
},
587+
Sku: &compute.Sku{
588+
Name: to.StringPtr(spec.Sku),
589+
Tier: to.StringPtr("Standard"),
590+
Capacity: to.Int64Ptr(spec.Capacity),
591+
},
592+
VirtualMachineScaleSetUpdateProperties: &compute.VirtualMachineScaleSetUpdateProperties{
593+
UpgradePolicy: &compute.UpgradePolicy{
594+
Mode: compute.Manual,
595+
},
596+
VirtualMachineProfile: &compute.VirtualMachineScaleSetUpdateVMProfile{
597+
OsProfile: &compute.VirtualMachineScaleSetUpdateOSProfile{
598+
CustomData: to.StringPtr(spec.CustomData),
599+
LinuxConfiguration: &compute.LinuxConfiguration{
600+
SSH: &compute.SSHConfiguration{
601+
PublicKeys: &[]compute.SSHPublicKey{
602+
{
603+
Path: to.StringPtr(fmt.Sprintf("/home/%s/.ssh/authorized_keys", azure.DefaultUserName)),
604+
KeyData: to.StringPtr(spec.SSHKeyData),
605+
},
606+
},
607+
},
608+
DisablePasswordAuthentication: to.BoolPtr(true),
609+
},
610+
},
611+
StorageProfile: &compute.VirtualMachineScaleSetUpdateStorageProfile{
612+
ImageReference: &compute.ImageReference{ID: to.StringPtr("image")},
613+
OsDisk: &compute.VirtualMachineScaleSetUpdateOSDisk{
614+
DiskSizeGB: to.Int32Ptr(120),
615+
ManagedDisk: &compute.VirtualMachineScaleSetManagedDiskParameters{StorageAccountType: "accountType"},
616+
},
617+
},
618+
},
619+
},
620+
}
621+
622+
skusMock.EXPECT().HasAcceleratedNetworking(gomock.Any(), gomock.Any()).Return(false, nil)
623+
lbMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.ClusterName).Return(getFakeNodeOutboundLoadBalancer(), nil)
624+
vmssMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name).Return(vmss, nil)
625+
vmssMock.EXPECT().Update(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, matchers.DiffEq(update)).Return(nil)
626+
},
627+
Expect: func(ctx context.Context, g *gomega.GomegaWithT, err error) {
628+
g.Expect(err).ToNot(gomega.HaveOccurred())
629+
},
630+
},
474631
}
475632

476633
for _, c := range cases {
@@ -578,6 +735,77 @@ func TestService_Delete(t *testing.T) {
578735
}
579736
}
580737

738+
func TestGetVMSSUpdateFromVMSS(t *testing.T) {
739+
g := gomega.NewGomegaWithT(t)
740+
741+
vmss := compute.VirtualMachineScaleSet{
742+
Location: to.StringPtr("eastus"),
743+
Tags: map[string]*string{
744+
"Name": to.StringPtr("capz-mp-0"),
745+
},
746+
Sku: &compute.Sku{
747+
Name: to.StringPtr("sku"),
748+
},
749+
VirtualMachineScaleSetProperties: &compute.VirtualMachineScaleSetProperties{
750+
UpgradePolicy: &compute.UpgradePolicy{
751+
Mode: compute.Manual,
752+
},
753+
VirtualMachineProfile: &compute.VirtualMachineScaleSetVMProfile{
754+
OsProfile: &compute.VirtualMachineScaleSetOSProfile{
755+
ComputerNamePrefix: to.StringPtr("vmss"),
756+
CustomData: to.StringPtr("data"),
757+
},
758+
StorageProfile: &compute.VirtualMachineScaleSetStorageProfile{
759+
ImageReference: &compute.ImageReference{
760+
ID: to.StringPtr("image-id"),
761+
},
762+
},
763+
NetworkProfile: &compute.VirtualMachineScaleSetNetworkProfile{
764+
NetworkInterfaceConfigurations: &[]compute.VirtualMachineScaleSetNetworkConfiguration{
765+
{
766+
Name: to.StringPtr("netconfig"),
767+
},
768+
},
769+
},
770+
},
771+
},
772+
}
773+
774+
expectedUpdate := compute.VirtualMachineScaleSetUpdate{
775+
Tags: map[string]*string{
776+
"Name": to.StringPtr("capz-mp-0"),
777+
},
778+
Sku: &compute.Sku{
779+
Name: to.StringPtr("sku"),
780+
},
781+
VirtualMachineScaleSetUpdateProperties: &compute.VirtualMachineScaleSetUpdateProperties{
782+
UpgradePolicy: &compute.UpgradePolicy{
783+
Mode: compute.Manual,
784+
},
785+
VirtualMachineProfile: &compute.VirtualMachineScaleSetUpdateVMProfile{
786+
OsProfile: &compute.VirtualMachineScaleSetUpdateOSProfile{
787+
CustomData: to.StringPtr("data"),
788+
},
789+
StorageProfile: &compute.VirtualMachineScaleSetUpdateStorageProfile{
790+
ImageReference: &compute.ImageReference{
791+
ID: to.StringPtr("image-id"),
792+
},
793+
},
794+
NetworkProfile: &compute.VirtualMachineScaleSetUpdateNetworkProfile{
795+
NetworkInterfaceConfigurations: &[]compute.VirtualMachineScaleSetUpdateNetworkConfiguration{
796+
{
797+
Name: to.StringPtr("netconfig"),
798+
},
799+
},
800+
},
801+
},
802+
},
803+
}
804+
result, err := getVMSSUpdateFromVMSS(vmss)
805+
g.Expect(err).ToNot(gomega.HaveOccurred())
806+
g.Expect(result).To(gomega.Equal(expectedUpdate))
807+
}
808+
581809
func getScopes(g *gomega.GomegaWithT) (*scope.ClusterScope, *scope.MachinePoolScope) {
582810
cluster := &clusterv1.Cluster{
583811
ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"},

0 commit comments

Comments
 (0)