Merge pull request #14514 from hakman/azure_experiments

azure: Fix various issues when creating and updating clusters

Makefile

@@ -49,6 +49,7 @@ UPLOAD_CMD=$(KOPS_ROOT)/hack/upload ${UPLOAD_ARGS}
 unexport AWS_ACCESS_KEY_ID AWS_REGION AWS_SECRET_ACCESS_KEY AWS_SESSION_TOKEN CNI_VERSION_URL DNS_IGNORE_NS_CHECK DNSCONTROLLER_IMAGE DO_ACCESS_TOKEN GOOGLE_APPLICATION_CREDENTIALS
 unexport KOPS_BASE_URL KOPS_CLUSTER_NAME KOPS_RUN_OBSOLETE_VERSION KOPS_STATE_STORE KOPS_STATE_S3_ACL KUBE_API_VERSIONS NODEUP_URL OPENSTACK_CREDENTIAL_FILE SKIP_PACKAGE_UPDATE
 unexport SKIP_REGION_CHECK S3_ACCESS_KEY_ID S3_ENDPOINT S3_REGION S3_SECRET_ACCESS_KEY HCLOUD_TOKEN SCW_ACCESS_KEY SCW_SECRET_KEY SCW_DEFAULT_PROJECT_ID SCW_DEFAULT_REGION SCW_DEFAULT_ZONE
+unexport AZURE_CLIENT_ID AZURE_CLIENT_SECRET AZURE_STORAGE_ACCOUNT AZURE_STORAGE_KEY AZURE_SUBSCRIPTION_ID AZURE_TENANT_ID
 
 
 VERSION=$(shell tools/get_version.sh | grep VERSION | awk '{print $$2}')

nodeup/pkg/model/context.go

@@ -613,6 +613,11 @@ func (c *NodeupModelContext) RunningOnGCE() bool {
 	return c.CloudProvider == kops.CloudProviderGCE
 }
 
+// RunningOnAzure returns true if we are running on Azure
+func (c *NodeupModelContext) RunningOnAzure() bool {
+	return c.CloudProvider == kops.CloudProviderAzure
+}
+
 // GetMetadataLocalIP returns the local IP address read from metadata
 func (c *NodeupModelContext) GetMetadataLocalIP() (string, error) {
 	var internalIP string

nodeup/pkg/model/ntp.go

@@ -59,7 +59,7 @@ func (b *NTPBuilder) Build(c *fi.ModelBuilderContext) error {
 		ntpHost = ""
 	}
 
-	if !b.RunningOnGCE() && b.Distribution.IsUbuntu() && b.Distribution.Version() <= 20.04 {
+	if !b.RunningOnGCE() && !b.RunningOnAzure() && b.Distribution.IsUbuntu() && b.Distribution.Version() <= 20.04 {
 		if ntpHost != "" {
 			c.AddTask(b.buildTimesyncdConf("/etc/systemd/timesyncd.conf", ntpHost))
 		}

pkg/model/azuremodel/vmscaleset.go

@@ -110,7 +110,7 @@ func (b *VMScaleSetModelBuilder) buildVMScaleSetTask(
 		t.SSHPublicKey = fi.String(string(b.SSHPublicKeys[0]))
 	}
 
-	if t.CustomData, err = b.BootstrapScriptBuilder.ResourceNodeUp(c, ig); err != nil {
+	if t.UserData, err = b.BootstrapScriptBuilder.ResourceNodeUp(c, ig); err != nil {
 		return nil, err
 	}
 

pkg/model/master_volumes.go

@@ -114,7 +114,10 @@ func (b *MasterVolumeBuilder) Build(c *fi.ModelBuilderContext) error {
 					return err
 				}
 			case kops.CloudProviderAzure:
-				b.addAzureVolume(c, name, volumeSize, zone, etcd, m, allMembers)
+				err = b.addAzureVolume(c, name, volumeSize, zone, etcd, m, allMembers)
+				if err != nil {
+					return err
+				}
 			default:
 				return fmt.Errorf("unknown cloudprovider %q", b.Cluster.Spec.GetCloudProvider())
 			}
@@ -347,7 +350,7 @@ func (b *MasterVolumeBuilder) addAzureVolume(
 	etcd kops.EtcdClusterSpec,
 	m kops.EtcdMemberSpec,
 	allMembers []string,
-) {
+) error {
 	// The tags are use by Protokube to mount the volume and use it for etcd.
 	tags := map[string]*string{
 		// This is the configuration of the etcd cluster.
@@ -365,7 +368,12 @@ func (b *MasterVolumeBuilder) addAzureVolume(
 		tags[k] = fi.String(v)
 	}
 
-	// TODO(kenji): Respect zone and m.EncryptedVolume.
+	zoneNumber, err := azure.ZoneToAvailabilityZoneNumber(zone)
+	if err != nil {
+		return err
+	}
+
+	// TODO(kenji): Respect m.EncryptedVolume.
 	t := &azuretasks.Disk{
 		Name:      fi.String(name),
 		Lifecycle: b.Lifecycle,
@@ -375,6 +383,9 @@ func (b *MasterVolumeBuilder) addAzureVolume(
 		},
 		SizeGB: fi.Int32(volumeSize),
 		Tags:   tags,
+		Zones:  &[]string{zoneNumber},
 	}
 	c.AddTask(t)
+
+	return nil
 }

upup/pkg/fi/cloudup/azure/azure_cloud.go

@@ -237,7 +237,7 @@ func (c *azureCloudImplementation) GetApiIngressStatus(cluster *kops.Cluster) ([
 			return nil, fmt.Errorf("error getting Master Scale Set Network Interfaces for API Ingress Status: %s", err)
 		}
 		for _, ni := range nis {
-			if !*ni.Primary {
+			if ni.Primary == nil || !*ni.Primary {
 				continue
 			}
 			for _, i := range *ni.IPConfigurations {

upup/pkg/fi/cloudup/azure/status_test.go

@@ -44,6 +44,15 @@ func (c *mockVMScaleSetsClient) List(ctx context.Context, resourceGroupName stri
 	return c.vmsses, nil
 }
 
+func (c *mockVMScaleSetsClient) Get(ctx context.Context, resourceGroupName string, vmssName string) (*compute.VirtualMachineScaleSet, error) {
+	for _, vmss := range c.vmsses {
+		if *vmss.Name == vmssName {
+			return &vmss, nil
+		}
+	}
+	return nil, nil
+}
+
 func (c *mockVMScaleSetsClient) Delete(ctx context.Context, resourceGroupName, vmssName string) error {
 	return fmt.Errorf("unimplemented")
 }

upup/pkg/fi/cloudup/azure/vmscaleset.go

@@ -28,6 +28,7 @@ import (
 type VMScaleSetsClient interface {
 	CreateOrUpdate(ctx context.Context, resourceGroupName, vmScaleSetName string, parameters compute.VirtualMachineScaleSet) (*compute.VirtualMachineScaleSet, error)
 	List(ctx context.Context, resourceGroupName string) ([]compute.VirtualMachineScaleSet, error)
+	Get(ctx context.Context, resourceGroupName string, vmssName string) (*compute.VirtualMachineScaleSet, error)
 	Delete(ctx context.Context, resourceGroupName, vmssName string) error
 }
 
@@ -63,6 +64,14 @@ func (c *vmScaleSetsClientImpl) List(ctx context.Context, resourceGroupName stri
 	return l, nil
 }
 
+func (c *vmScaleSetsClientImpl) Get(ctx context.Context, resourceGroupName string, vmssName string) (*compute.VirtualMachineScaleSet, error) {
+	vmss, err := c.c.Get(ctx, resourceGroupName, vmssName, compute.UserData)
+	if err != nil {
+		return nil, err
+	}
+	return &vmss, nil
+}
+
 func (c *vmScaleSetsClientImpl) Delete(ctx context.Context, resourceGroupName, vmssName string) error {
 	future, err := c.c.Delete(ctx, resourceGroupName, vmssName, nil)
 	if err != nil {

upup/pkg/fi/cloudup/azuretasks/disk.go

@@ -35,6 +35,7 @@ type Disk struct {
 	ResourceGroup *ResourceGroup
 	SizeGB        *int32
 	Tags          map[string]*string
+	Zones         *[]string
 }
 
 var (
@@ -74,6 +75,7 @@ func (d *Disk) Find(c *fi.Context) (*Disk, error) {
 		},
 		SizeGB: found.DiskSizeGB,
 		Tags:   found.Tags,
+		Zones:  found.Zones,
 	}, nil
 }
 
@@ -122,6 +124,7 @@ func (*Disk) RenderAzure(t *azure.AzureAPITarget, a, e, changes *Disk) error {
 			DiskSizeGB: e.SizeGB,
 		},
 		Tags:  e.Tags,
+		Zones: e.Zones,
 	}
 
 	return t.Cloud.Disk().CreateOrUpdate(

upup/pkg/fi/cloudup/azuretasks/roleassignment.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"path/filepath"
 	"strings"
 
 	"k8s.io/kops/upup/pkg/fi"
@@ -112,6 +113,7 @@ func (r *RoleAssignment) Find(c *fi.Context) (*RoleAssignment, error) {
 		return nil, fmt.Errorf("corresponding VM Scale Set not found for Role Assignment: %s", *found.ID)
 	}
 
+	r.ID = found.ID
 	return &RoleAssignment{
 		Name:      r.Name,
 		Lifecycle: r.Lifecycle,
@@ -122,7 +124,7 @@ func (r *RoleAssignment) Find(c *fi.Context) (*RoleAssignment, error) {
 			Name: foundVMSS.Name,
 		},
 		ID:        found.ID,
-		RoleDefID: found.RoleDefinitionID,
+		RoleDefID: fi.String(filepath.Base(fi.StringValue(found.RoleDefinitionID))),
 	}, nil
 }
 

upup/pkg/fi/cloudup/azuretasks/routetable.go

@@ -67,6 +67,7 @@ func (r *RouteTable) Find(c *fi.Context) (*RouteTable, error) {
 	return &RouteTable{
 		Name:      r.Name,
 		Lifecycle: r.Lifecycle,
+		Shared:    r.Shared,
 		ResourceGroup: &ResourceGroup{
 			Name: r.ResourceGroup.Name,
 		},

upup/pkg/fi/cloudup/azuretasks/subnet.go

@@ -68,6 +68,7 @@ func (s *Subnet) Find(c *fi.Context) (*Subnet, error) {
 	return &Subnet{
 		Name:      s.Name,
 		Lifecycle: s.Lifecycle,
+		Shared:    s.Shared,
 		ResourceGroup: &ResourceGroup{
 			Name: s.ResourceGroup.Name,
 		},

upup/pkg/fi/cloudup/azuretasks/testing.go

@@ -403,6 +403,15 @@ func (c *MockVMScaleSetsClient) List(ctx context.Context, resourceGroupName stri
 	return l, nil
 }
 
+// Get Returns a specified VM Scale Set.
+func (c *MockVMScaleSetsClient) Get(ctx context.Context, resourceGroupName string, vmssName string) (*compute.VirtualMachineScaleSet, error) {
+	vmss, ok := c.VMSSes[vmssName]
+	if !ok {
+		return nil, nil
+	}
+	return &vmss, nil
+}
+
 // Delete deletes a specified VM Scale Set.
 func (c *MockVMScaleSetsClient) Delete(ctx context.Context, resourceGroupName, vmssName string) error {
 	// Ignore resourceGroupName for simplicity.

upup/pkg/fi/cloudup/azuretasks/virtualnetwork.go

@@ -76,6 +76,7 @@ func (n *VirtualNetwork) Find(c *fi.Context) (*VirtualNetwork, error) {
 	return &VirtualNetwork{
 		Name:      n.Name,
 		Lifecycle: n.Lifecycle,
+		Shared:    n.Shared,
 		ResourceGroup: &ResourceGroup{
 			Name: n.ResourceGroup.Name,
 		},

upup/pkg/fi/cloudup/azuretasks/vmscaleset.go

@@ -113,8 +113,8 @@ type VMScaleSet struct {
 	// AdmnUser specifies the name of the administrative account.
 	AdminUser    *string
 	SSHPublicKey *string
-	// CustomData is the user data configuration
+	// UserData is the user data configuration
-	CustomData  fi.Resource
+	UserData    fi.Resource
 	Tags        map[string]*string
 	Zones       []string
 	PrincipalID *string
@@ -153,17 +153,10 @@ func (s *VMScaleSet) CompareWithID() *string {
 // Find discovers the VMScaleSet in the cloud provider.
 func (s *VMScaleSet) Find(c *fi.Context) (*VMScaleSet, error) {
 	cloud := c.Cloud.(azure.AzureCloud)
-	l, err := cloud.VMScaleSet().List(context.TODO(), *s.ResourceGroup.Name)
+	found, err := cloud.VMScaleSet().Get(context.TODO(), *s.ResourceGroup.Name, *s.Name)
-	if err != nil {
+	if err != nil && !strings.Contains(err.Error(), "ResourceNotFound") {
 		return nil, err
 	}
-	var found *compute.VirtualMachineScaleSet
-	for _, v := range l {
-		if *v.Name == *s.Name {
-			found = &v
-			break
-		}
-	}
 	if found == nil {
 		return nil, nil
 	}
@@ -204,9 +197,11 @@ func (s *VMScaleSet) Find(c *fi.Context) (*VMScaleSet, error) {
 		return nil, fmt.Errorf("unexpected number of SSH keys found for VM ScaleSet %s: %d", *s.Name, len(sshKeys))
 	}
 
-	// TODO(kenji): Do not check custom data as Azure doesn't
+	userData, err := base64.StdEncoding.DecodeString(*profile.UserData)
-	// populate (https://github.com/Azure/azure-cli/issues/5866).
+	if err != nil {
-	// Find a way to work around this.
+		return nil, fmt.Errorf("failed to decode user data: %w", err)
+	}
+
 	vmss := &VMScaleSet{
 		Name:      s.Name,
 		Lifecycle: s.Lifecycle,
@@ -228,6 +223,7 @@ func (s *VMScaleSet) Find(c *fi.Context) (*VMScaleSet, error) {
 		ComputerNamePrefix: osProfile.ComputerNamePrefix,
 		AdminUser:          osProfile.AdminUsername,
 		SSHPublicKey:       sshKeys[0].KeyData,
+		UserData:           fi.NewBytesResource(userData),
 		Tags:               found.Tags,
 		PrincipalID:        found.Identity.PrincipalID,
 	}
@@ -239,6 +235,7 @@ func (s *VMScaleSet) Find(c *fi.Context) (*VMScaleSet, error) {
 	if found.Zones != nil {
 		vmss.Zones = *found.Zones
 	}
+	s.PrincipalID = found.Identity.PrincipalID
 	return vmss, nil
 }
 
@@ -280,10 +277,10 @@ func (s *VMScaleSet) RenderAzure(t *azure.AzureAPITarget, a, e, changes *VMScale
 	name := *e.Name
 
 	var customData *string
-	if e.CustomData != nil {
+	if e.UserData != nil {
-		d, err := fi.ResourceAsBytes(e.CustomData)
+		d, err := fi.ResourceAsBytes(e.UserData)
 		if err != nil {
-			return fmt.Errorf("error rendering CustomData: %s", err)
+			return fmt.Errorf("error rendering UserData: %s", err)
 		}
 		customData = to.StringPtr(base64.StdEncoding.EncodeToString(d))
 	}
@@ -291,7 +288,6 @@ func (s *VMScaleSet) RenderAzure(t *azure.AzureAPITarget, a, e, changes *VMScale
 	osProfile := &compute.VirtualMachineScaleSetOSProfile{
 		ComputerNamePrefix: e.ComputerNamePrefix,
 		AdminUsername:      e.AdminUser,
-		CustomData:         customData,
 		LinuxConfiguration: &compute.LinuxConfiguration{
 			SSH: &compute.SSHConfiguration{
 				PublicKeys: &[]compute.SSHPublicKey{
@@ -366,6 +362,7 @@ func (s *VMScaleSet) RenderAzure(t *azure.AzureAPITarget, a, e, changes *VMScale
 			VirtualMachineProfile: &compute.VirtualMachineScaleSetVMProfile{
 				OsProfile:      osProfile,
 				StorageProfile: e.StorageProfile.VirtualMachineScaleSetStorageProfile,
+				UserData:       customData,
 				NetworkProfile: &compute.VirtualMachineScaleSetNetworkProfile{
 					NetworkInterfaceConfigurations: &[]compute.VirtualMachineScaleSetNetworkConfiguration{
 						networkConfig,

upup/pkg/fi/cloudup/azuretasks/vmscaleset_test.go

@@ -84,7 +84,7 @@ func newTestVMScaleSet() *VMScaleSet {
 		ComputerNamePrefix: to.StringPtr("cprefix"),
 		AdminUser:          to.StringPtr("admin"),
 		SSHPublicKey:       to.StringPtr("ssh"),
-		CustomData:         fi.NewStringResource("custom"),
+		UserData:           fi.NewStringResource("custom"),
 		Tags:               map[string]*string{},
 		Zones:              []string{"zone1"},
 	}
@@ -113,17 +113,17 @@ func TestVMScaleSetRenderAzure(t *testing.T) {
 	if a, e := *actual.Sku.Capacity, *expected.Capacity; a != e {
 		t.Errorf("unexpected SKU Capacity: expected %d, but got %d", e, a)
 	}
-	actualCData, err := base64.StdEncoding.DecodeString(
+	actualUserData, err := base64.StdEncoding.DecodeString(
-		*actual.VirtualMachineProfile.OsProfile.CustomData)
+		*actual.VirtualMachineProfile.UserData)
 	if err != nil {
-		t.Fatalf("failed to decode custom data: %s", err)
+		t.Fatalf("failed to decode user data: %s", err)
 	}
-	expectedCData, err := fi.ResourceAsBytes(expected.CustomData)
+	expectedUserData, err := fi.ResourceAsBytes(expected.UserData)
 	if err != nil {
-		t.Fatalf("failed to get custom data: %s", err)
+		t.Fatalf("failed to get user data: %s", err)
 	}
-	if !bytes.Equal(actualCData, expectedCData) {
+	if !bytes.Equal(actualUserData, expectedUserData) {
-		t.Errorf("unexpected custom data: expected %v, but got %v", expectedCData, actualCData)
+		t.Errorf("unexpected user data: expected %v, but got %v", expectedUserData, actualUserData)
 	}
 
 	if expected.PrincipalID == nil {
@@ -158,11 +158,10 @@ func TestVMScaleSetFind(t *testing.T) {
 	}
 
 	// Create a VM ScaleSet.
-	customData := []byte("custom")
+	userData := []byte("custom")
 	osProfile := &compute.VirtualMachineScaleSetOSProfile{
 		ComputerNamePrefix: to.StringPtr("prefix"),
 		AdminUsername:      to.StringPtr("admin"),
-		CustomData:         to.StringPtr(base64.RawStdEncoding.EncodeToString(customData)),
 		LinuxConfiguration: &compute.LinuxConfiguration{
 			SSH: &compute.SSHConfiguration{
 				PublicKeys: &[]compute.SSHPublicKey{
@@ -248,6 +247,7 @@ func TestVMScaleSetFind(t *testing.T) {
 			VirtualMachineProfile: &compute.VirtualMachineScaleSetVMProfile{
 				OsProfile:      osProfile,
 				StorageProfile: storageProfile,
+				UserData:       to.StringPtr(base64.RawStdEncoding.EncodeToString(userData)),
 				NetworkProfile: &compute.VirtualMachineScaleSetNetworkProfile{
 					NetworkInterfaceConfigurations: &[]compute.VirtualMachineScaleSetNetworkConfiguration{
 						networkConfig,

upup/pkg/fi/cloudup/populate_instancegroup_spec.go

@@ -38,16 +38,16 @@ import (
 const (
 	defaultNodeMachineTypeGCE     = "n1-standard-2"
 	defaultNodeMachineTypeDO      = "s-2vcpu-4gb"
-	defaultNodeMachineTypeAzure   = "Standard_B2ms"
+	defaultNodeMachineTypeAzure   = "Standard_B2s"
 	defaultNodeMachineTypeHetzner = "cx21"
 
 	defaultBastionMachineTypeGCE     = "f1-micro"
-	defaultBastionMachineTypeAzure   = "Standard_B2ms"
+	defaultBastionMachineTypeAzure   = "Standard_B2s"
 	defaultBastionMachineTypeHetzner = "cx11"
 
 	defaultMasterMachineTypeGCE     = "e2-medium"
 	defaultMasterMachineTypeDO      = "s-2vcpu-4gb"
-	defaultMasterMachineTypeAzure   = "Standard_B2ms"
+	defaultMasterMachineTypeAzure   = "Standard_B2s"
 	defaultMasterMachineTypeHetzner = "cx21"
 
 	defaultDONodeImage = "ubuntu-20-04-x64"