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

azure: Fix various issues when creating and updating clusters #14514

Merged
merged 8 commits into from
Nov 9, 2022
Merged
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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}')
Expand Down
5 changes: 5 additions & 0 deletions nodeup/pkg/model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion nodeup/pkg/model/ntp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/azuremodel/vmscaleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
17 changes: 14 additions & 3 deletions pkg/model/master_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -375,6 +383,9 @@ func (b *MasterVolumeBuilder) addAzureVolume(
},
SizeGB: fi.Int32(volumeSize),
Tags: tags,
Zones: &[]string{zoneNumber},
}
c.AddTask(t)

return nil
}
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/azure/azure_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions upup/pkg/fi/cloudup/azure/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
9 changes: 9 additions & 0 deletions upup/pkg/fi/cloudup/azure/vmscaleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion upup/pkg/fi/cloudup/azuretasks/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Disk struct {
ResourceGroup *ResourceGroup
SizeGB *int32
Tags map[string]*string
Zones *[]string
}

var (
Expand Down Expand Up @@ -74,6 +75,7 @@ func (d *Disk) Find(c *fi.Context) (*Disk, error) {
},
SizeGB: found.DiskSizeGB,
Tags: found.Tags,
Zones: found.Zones,
}, nil
}

Expand Down Expand Up @@ -121,7 +123,8 @@ func (*Disk) RenderAzure(t *azure.AzureAPITarget, a, e, changes *Disk) error {
},
DiskSizeGB: e.SizeGB,
},
Tags: e.Tags,
Tags: e.Tags,
Zones: e.Zones,
}

return t.Cloud.Disk().CreateOrUpdate(
Expand Down
4 changes: 3 additions & 1 deletion upup/pkg/fi/cloudup/azuretasks/roleassignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"path/filepath"
"strings"

"k8s.io/kops/upup/pkg/fi"
Expand Down Expand Up @@ -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,
Expand All @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions upup/pkg/fi/cloudup/azuretasks/routetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
1 change: 1 addition & 0 deletions upup/pkg/fi/cloudup/azuretasks/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
9 changes: 9 additions & 0 deletions upup/pkg/fi/cloudup/azuretasks/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions upup/pkg/fi/cloudup/azuretasks/virtualnetwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
33 changes: 15 additions & 18 deletions upup/pkg/fi/cloudup/azuretasks/vmscaleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
CustomData fi.Resource
// UserData is the user data configuration
UserData fi.Resource
Tags map[string]*string
Zones []string
PrincipalID *string
Expand Down Expand Up @@ -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)
if err != nil {
found, err := cloud.VMScaleSet().Get(context.TODO(), *s.ResourceGroup.Name, *s.Name)
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
}
Expand Down Expand Up @@ -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
// populate (https://github.com/Azure/azure-cli/issues/5866).
// Find a way to work around this.
userData, err := base64.StdEncoding.DecodeString(*profile.UserData)
if err != nil {
return nil, fmt.Errorf("failed to decode user data: %w", err)
}

vmss := &VMScaleSet{
Name: s.Name,
Lifecycle: s.Lifecycle,
Expand All @@ -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,
}
Expand All @@ -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
}

Expand Down Expand Up @@ -280,18 +277,17 @@ func (s *VMScaleSet) RenderAzure(t *azure.AzureAPITarget, a, e, changes *VMScale
name := *e.Name

var customData *string
if e.CustomData != nil {
d, err := fi.ResourceAsBytes(e.CustomData)
if e.UserData != nil {
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))
}

osProfile := &compute.VirtualMachineScaleSetOSProfile{
ComputerNamePrefix: e.ComputerNamePrefix,
AdminUsername: e.AdminUser,
CustomData: customData,
LinuxConfiguration: &compute.LinuxConfiguration{
SSH: &compute.SSHConfiguration{
PublicKeys: &[]compute.SSHPublicKey{
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 10 additions & 10 deletions upup/pkg/fi/cloudup/azuretasks/vmscaleset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
}
Expand Down Expand Up @@ -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(
*actual.VirtualMachineProfile.OsProfile.CustomData)
actualUserData, err := base64.StdEncoding.DecodeString(
*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) {
t.Errorf("unexpected custom data: expected %v, but got %v", expectedCData, actualCData)
if !bytes.Equal(actualUserData, expectedUserData) {
t.Errorf("unexpected user data: expected %v, but got %v", expectedUserData, actualUserData)
}

if expected.PrincipalID == nil {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions upup/pkg/fi/cloudup/populate_instancegroup_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down