From 051b4e57682ef48b7064b48c84a3987105388fda Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Wed, 11 Oct 2023 12:38:39 -0500 Subject: [PATCH] preserve user-supplied ASO spec values --- azure/services/agentpools/spec.go | 67 ++++++------ azure/services/agentpools/spec_test.go | 28 +++-- azure/services/managedclusters/spec.go | 114 ++++++++++---------- azure/services/managedclusters/spec_test.go | 3 + 4 files changed, 113 insertions(+), 99 deletions(-) diff --git a/azure/services/agentpools/spec.go b/azure/services/agentpools/spec.go index e1477a80084..58d02e93f48 100644 --- a/azure/services/agentpools/spec.go +++ b/azure/services/agentpools/spec.go @@ -171,36 +171,35 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing *asocontainerse agentPool = &asocontainerservicev1.ManagedClustersAgentPool{} } - agentPool.Spec = asocontainerservicev1.ManagedClusters_AgentPool_Spec{ - AzureName: s.AzureName, - Owner: &genruntime.KnownResourceReference{ - Name: s.Cluster, - }, - AvailabilityZones: s.AvailabilityZones, - Count: &s.Replicas, - EnableAutoScaling: ptr.To(s.EnableAutoScaling), - EnableUltraSSD: s.EnableUltraSSD, - KubeletDiskType: azure.AliasOrNil[asocontainerservicev1.KubeletDiskType]((*string)(s.KubeletDiskType)), - MaxCount: s.MaxCount, - MaxPods: s.MaxPods, - MinCount: s.MinCount, - Mode: ptr.To(asocontainerservicev1.AgentPoolMode(s.Mode)), - NodeLabels: s.NodeLabels, - NodeTaints: s.NodeTaints, - OrchestratorVersion: s.Version, - OsDiskSizeGB: ptr.To(asocontainerservicev1.ContainerServiceOSDisk(s.OSDiskSizeGB)), - OsDiskType: azure.AliasOrNil[asocontainerservicev1.OSDiskType](s.OsDiskType), - OsType: azure.AliasOrNil[asocontainerservicev1.OSType](s.OSType), - ScaleSetPriority: azure.AliasOrNil[asocontainerservicev1.ScaleSetPriority](s.ScaleSetPriority), - ScaleDownMode: azure.AliasOrNil[asocontainerservicev1.ScaleDownMode](s.ScaleDownMode), - Type: ptr.To(asocontainerservicev1.AgentPoolType_VirtualMachineScaleSets), - EnableNodePublicIP: s.EnableNodePublicIP, - Tags: s.AdditionalTags, - EnableFIPS: s.EnableFIPS, + spec := &agentPool.Spec + spec.AzureName = s.AzureName + spec.Owner = &genruntime.KnownResourceReference{ + Name: s.Cluster, } + spec.AvailabilityZones = s.AvailabilityZones + spec.Count = &s.Replicas + spec.EnableAutoScaling = ptr.To(s.EnableAutoScaling) + spec.EnableUltraSSD = s.EnableUltraSSD + spec.KubeletDiskType = azure.AliasOrNil[asocontainerservicev1.KubeletDiskType]((*string)(s.KubeletDiskType)) + spec.MaxCount = s.MaxCount + spec.MaxPods = s.MaxPods + spec.MinCount = s.MinCount + spec.Mode = ptr.To(asocontainerservicev1.AgentPoolMode(s.Mode)) + spec.NodeLabels = s.NodeLabels + spec.NodeTaints = s.NodeTaints + spec.OrchestratorVersion = s.Version + spec.OsDiskSizeGB = ptr.To(asocontainerservicev1.ContainerServiceOSDisk(s.OSDiskSizeGB)) + spec.OsDiskType = azure.AliasOrNil[asocontainerservicev1.OSDiskType](s.OsDiskType) + spec.OsType = azure.AliasOrNil[asocontainerservicev1.OSType](s.OSType) + spec.ScaleSetPriority = azure.AliasOrNil[asocontainerservicev1.ScaleSetPriority](s.ScaleSetPriority) + spec.ScaleDownMode = azure.AliasOrNil[asocontainerservicev1.ScaleDownMode](s.ScaleDownMode) + spec.Type = ptr.To(asocontainerservicev1.AgentPoolType_VirtualMachineScaleSets) + spec.EnableNodePublicIP = s.EnableNodePublicIP + spec.Tags = s.AdditionalTags + spec.EnableFIPS = s.EnableFIPS if s.KubeletConfig != nil { - agentPool.Spec.KubeletConfig = &asocontainerservicev1.KubeletConfig{ + spec.KubeletConfig = &asocontainerservicev1.KubeletConfig{ CpuManagerPolicy: s.KubeletConfig.CPUManagerPolicy, CpuCfsQuota: s.KubeletConfig.CPUCfsQuota, CpuCfsQuotaPeriod: s.KubeletConfig.CPUCfsQuotaPeriod, @@ -216,33 +215,33 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing *asocontainerse } if s.SKU != "" { - agentPool.Spec.VmSize = &s.SKU + spec.VmSize = &s.SKU } if s.SpotMaxPrice != nil { - agentPool.Spec.SpotMaxPrice = ptr.To(s.SpotMaxPrice.AsApproximateFloat64()) + spec.SpotMaxPrice = ptr.To(s.SpotMaxPrice.AsApproximateFloat64()) } if s.VnetSubnetID != "" { - agentPool.Spec.VnetSubnetReference = &genruntime.ResourceReference{ + spec.VnetSubnetReference = &genruntime.ResourceReference{ ARMID: s.VnetSubnetID, } } if s.NodePublicIPPrefixID != "" { - agentPool.Spec.NodePublicIPPrefixReference = &genruntime.ResourceReference{ + spec.NodePublicIPPrefixReference = &genruntime.ResourceReference{ ARMID: s.NodePublicIPPrefixID, } } if s.LinuxOSConfig != nil { - agentPool.Spec.LinuxOSConfig = &asocontainerservicev1.LinuxOSConfig{ + spec.LinuxOSConfig = &asocontainerservicev1.LinuxOSConfig{ SwapFileSizeMB: s.LinuxOSConfig.SwapFileSizeMB, TransparentHugePageEnabled: (*string)(s.LinuxOSConfig.TransparentHugePageEnabled), TransparentHugePageDefrag: (*string)(s.LinuxOSConfig.TransparentHugePageDefrag), } if s.LinuxOSConfig.Sysctls != nil { - agentPool.Spec.LinuxOSConfig.Sysctls = &asocontainerservicev1.SysctlConfig{ + spec.LinuxOSConfig.Sysctls = &asocontainerservicev1.SysctlConfig{ FsAioMaxNr: s.LinuxOSConfig.Sysctls.FsAioMaxNr, FsFileMax: s.LinuxOSConfig.Sysctls.FsFileMax, FsInotifyMaxUserWatches: s.LinuxOSConfig.Sysctls.FsInotifyMaxUserWatches, @@ -279,7 +278,7 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing *asocontainerse // count present in MachinePool or AzureManagedMachinePool, hence we should not make an update API call based // on difference in count. if s.EnableAutoScaling && agentPool.Status.Count != nil { - agentPool.Spec.Count = agentPool.Status.Count + spec.Count = agentPool.Status.Count } return agentPool, nil diff --git a/azure/services/agentpools/spec_test.go b/azure/services/agentpools/spec_test.go index cbb61f5114c..c458ba892cc 100644 --- a/azure/services/agentpools/spec_test.go +++ b/azure/services/agentpools/spec_test.go @@ -202,15 +202,6 @@ func TestParameters(t *testing.T) { expected: sdkFakeAgentPool(), expectedError: nil, }, - { - name: "parameters with an existing agent pool and update needed on spot max price", - spec: fakeAgentPool(), - existing: sdkFakeAgentPool( - sdkWithSpotMaxPrice(123.456), - ), - expected: sdkFakeAgentPool(), - expectedError: nil, - }, { name: "parameters with an existing agent pool and update needed on spot max price", spec: fakeAgentPool( @@ -307,6 +298,25 @@ func TestParameters(t *testing.T) { expectNoChange: true, expectedError: nil, }, + { + name: "user-supplied spec values persist", + spec: fakeAgentPool(), + existing: sdkFakeAgentPool( + func(pool *asocontainerservicev1.ManagedClustersAgentPool) { + pool.Spec.PowerState = &asocontainerservicev1.PowerState{ + Code: ptr.To[asocontainerservicev1.PowerState_Code]("some value"), + } + }, + ), + expected: sdkFakeAgentPool( + func(pool *asocontainerservicev1.ManagedClustersAgentPool) { + pool.Spec.PowerState = &asocontainerservicev1.PowerState{ + Code: ptr.To[asocontainerservicev1.PowerState_Code]("some value"), + } + }, + ), + expectedError: nil, + }, } for _, tc := range testcases { tc := tc diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index 55367920d3d..22ef9f48b0b 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -296,44 +296,46 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai managedCluster := existing if managedCluster == nil { - managedCluster = &asocontainerservicev1.ManagedCluster{} + managedCluster = &asocontainerservicev1.ManagedCluster{ + Spec: asocontainerservicev1.ManagedCluster_Spec{ + Tags: infrav1.Build(infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: s.ClusterName, + Name: ptr.To(s.Name), + Role: ptr.To(infrav1.CommonRole), + // Additional tags managed separately + }), + }, + } } - managedCluster.Spec = asocontainerservicev1.ManagedCluster_Spec{ - AzureName: s.Name, - Owner: &genruntime.KnownResourceReference{ - Name: s.ResourceGroup, - }, - Identity: &asocontainerservicev1.ManagedClusterIdentity{ - Type: ptr.To(asocontainerservicev1.ManagedClusterIdentity_Type_SystemAssigned), - }, - Location: &s.Location, - Tags: infrav1.Build(infrav1.BuildParams{ - Lifecycle: infrav1.ResourceLifecycleOwned, - ClusterName: s.ClusterName, - Name: ptr.To(s.Name), - Role: ptr.To(infrav1.CommonRole), - // Additional tags managed separately - }), - NodeResourceGroup: &s.NodeResourceGroup, - EnableRBAC: ptr.To(true), - DnsPrefix: s.DNSPrefix, - KubernetesVersion: &s.Version, - ServicePrincipalProfile: &asocontainerservicev1.ManagedClusterServicePrincipalProfile{ - ClientId: ptr.To("msi"), - }, - NetworkProfile: &asocontainerservicev1.ContainerServiceNetworkProfile{ - NetworkPlugin: azure.AliasOrNil[asocontainerservicev1.ContainerServiceNetworkProfile_NetworkPlugin](&s.NetworkPlugin), - LoadBalancerSku: azure.AliasOrNil[asocontainerservicev1.ContainerServiceNetworkProfile_LoadBalancerSku](&s.LoadBalancerSKU), - NetworkPolicy: azure.AliasOrNil[asocontainerservicev1.ContainerServiceNetworkProfile_NetworkPolicy](&s.NetworkPolicy), - }, - AutoScalerProfile: buildAutoScalerProfile(s.AutoScalerProfile), - OperatorSpec: &asocontainerservicev1.ManagedClusterOperatorSpec{ - Secrets: &asocontainerservicev1.ManagedClusterOperatorSecrets{ - AdminCredentials: &genruntime.SecretDestination{ - Name: secret.Name(s.ClusterName, secret.Kubeconfig), - Key: secret.KubeconfigDataName, - }, + spec := &managedCluster.Spec + spec.AzureName = s.Name + spec.Owner = &genruntime.KnownResourceReference{ + Name: s.ResourceGroup, + } + spec.Identity = &asocontainerservicev1.ManagedClusterIdentity{ + Type: ptr.To(asocontainerservicev1.ManagedClusterIdentity_Type_SystemAssigned), + } + spec.Location = &s.Location + spec.NodeResourceGroup = &s.NodeResourceGroup + spec.EnableRBAC = ptr.To(true) + spec.DnsPrefix = s.DNSPrefix + spec.KubernetesVersion = &s.Version + spec.ServicePrincipalProfile = &asocontainerservicev1.ManagedClusterServicePrincipalProfile{ + ClientId: ptr.To("msi"), + } + spec.NetworkProfile = &asocontainerservicev1.ContainerServiceNetworkProfile{ + NetworkPlugin: azure.AliasOrNil[asocontainerservicev1.ContainerServiceNetworkProfile_NetworkPlugin](&s.NetworkPlugin), + LoadBalancerSku: azure.AliasOrNil[asocontainerservicev1.ContainerServiceNetworkProfile_LoadBalancerSku](&s.LoadBalancerSKU), + NetworkPolicy: azure.AliasOrNil[asocontainerservicev1.ContainerServiceNetworkProfile_NetworkPolicy](&s.NetworkPolicy), + } + spec.AutoScalerProfile = buildAutoScalerProfile(s.AutoScalerProfile) + spec.OperatorSpec = &asocontainerservicev1.ManagedClusterOperatorSpec{ + Secrets: &asocontainerservicev1.ManagedClusterOperatorSecrets{ + AdminCredentials: &genruntime.SecretDestination{ + Name: secret.Name(s.ClusterName, secret.Kubeconfig), + Key: secret.KubeconfigDataName, }, }, } @@ -346,7 +348,7 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai } } if decodedSSHPublicKey != nil { - managedCluster.Spec.LinuxProfile = &asocontainerservicev1.ContainerServiceLinuxProfile{ + spec.LinuxProfile = &asocontainerservicev1.ContainerServiceLinuxProfile{ AdminUsername: ptr.To(azure.DefaultAKSUserName), Ssh: &asocontainerservicev1.ContainerServiceSshConfiguration{ PublicKeys: []asocontainerservicev1.ContainerServiceSshPublicKey{ @@ -359,16 +361,16 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai } if s.NetworkPluginMode != nil { - managedCluster.Spec.NetworkProfile.NetworkPluginMode = ptr.To(asocontainerservicev1.ContainerServiceNetworkProfile_NetworkPluginMode(*s.NetworkPluginMode)) + spec.NetworkProfile.NetworkPluginMode = ptr.To(asocontainerservicev1.ContainerServiceNetworkProfile_NetworkPluginMode(*s.NetworkPluginMode)) } if s.PodCIDR != "" { - managedCluster.Spec.NetworkProfile.PodCidr = &s.PodCIDR + spec.NetworkProfile.PodCidr = &s.PodCIDR } if s.ServiceCIDR != "" { if s.DNSServiceIP == nil { - managedCluster.Spec.NetworkProfile.ServiceCidr = &s.ServiceCIDR + spec.NetworkProfile.ServiceCidr = &s.ServiceCIDR ip, _, err := net.ParseCIDR(s.ServiceCIDR) if err != nil { return nil, fmt.Errorf("failed to parse service cidr: %w", err) @@ -379,14 +381,14 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai // https://golang.org/src/net/ip.go#L48 ip[15] = byte(10) dnsIP := ip.String() - managedCluster.Spec.NetworkProfile.DnsServiceIP = &dnsIP + spec.NetworkProfile.DnsServiceIP = &dnsIP } else { - managedCluster.Spec.NetworkProfile.DnsServiceIP = s.DNSServiceIP + spec.NetworkProfile.DnsServiceIP = s.DNSServiceIP } } if s.AADProfile != nil { - managedCluster.Spec.AadProfile = &asocontainerservicev1.ManagedClusterAADProfile{ + spec.AadProfile = &asocontainerservicev1.ManagedClusterAADProfile{ Managed: &s.AADProfile.Managed, EnableAzureRBAC: &s.AADProfile.EnableAzureRBAC, AdminGroupObjectIDs: s.AADProfile.AdminGroupObjectIDs, @@ -394,8 +396,8 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai } for i := range s.AddonProfiles { - if managedCluster.Spec.AddonProfiles == nil { - managedCluster.Spec.AddonProfiles = map[string]asocontainerservicev1.ManagedClusterAddonProfile{} + if spec.AddonProfiles == nil { + spec.AddonProfiles = map[string]asocontainerservicev1.ManagedClusterAddonProfile{} } item := s.AddonProfiles[i] addonProfile := asocontainerservicev1.ManagedClusterAddonProfile{ @@ -404,46 +406,46 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai if item.Config != nil { addonProfile.Config = item.Config } - managedCluster.Spec.AddonProfiles[item.Name] = addonProfile + spec.AddonProfiles[item.Name] = addonProfile } if s.SKU != nil { tierName := asocontainerservicev1.ManagedClusterSKU_Tier(s.SKU.Tier) - managedCluster.Spec.Sku = &asocontainerservicev1.ManagedClusterSKU{ + spec.Sku = &asocontainerservicev1.ManagedClusterSKU{ Name: ptr.To(asocontainerservicev1.ManagedClusterSKU_Name("Base")), Tier: ptr.To(tierName), } } if s.LoadBalancerProfile != nil { - managedCluster.Spec.NetworkProfile.LoadBalancerProfile = s.GetLoadBalancerProfile() + spec.NetworkProfile.LoadBalancerProfile = s.GetLoadBalancerProfile() } if s.APIServerAccessProfile != nil { - managedCluster.Spec.ApiServerAccessProfile = &asocontainerservicev1.ManagedClusterAPIServerAccessProfile{ + spec.ApiServerAccessProfile = &asocontainerservicev1.ManagedClusterAPIServerAccessProfile{ EnablePrivateCluster: s.APIServerAccessProfile.EnablePrivateCluster, PrivateDNSZone: s.APIServerAccessProfile.PrivateDNSZone, EnablePrivateClusterPublicFQDN: s.APIServerAccessProfile.EnablePrivateClusterPublicFQDN, } if s.APIServerAccessProfile.AuthorizedIPRanges != nil { - managedCluster.Spec.ApiServerAccessProfile.AuthorizedIPRanges = s.APIServerAccessProfile.AuthorizedIPRanges + spec.ApiServerAccessProfile.AuthorizedIPRanges = s.APIServerAccessProfile.AuthorizedIPRanges } } if s.OutboundType != nil { - managedCluster.Spec.NetworkProfile.OutboundType = ptr.To(asocontainerservicev1.ContainerServiceNetworkProfile_OutboundType(*s.OutboundType)) + spec.NetworkProfile.OutboundType = ptr.To(asocontainerservicev1.ContainerServiceNetworkProfile_OutboundType(*s.OutboundType)) } if s.Identity != nil { - managedCluster.Spec.Identity, err = getIdentity(s.Identity) + spec.Identity, err = getIdentity(s.Identity) if err != nil { return nil, errors.Wrapf(err, "Identity is not valid: %s", err) } } if s.KubeletUserAssignedIdentity != "" { - managedCluster.Spec.IdentityProfile = map[string]asocontainerservicev1.UserAssignedIdentity{ + spec.IdentityProfile = map[string]asocontainerservicev1.UserAssignedIdentity{ kubeletIdentityKey: { ResourceReference: &genruntime.ResourceReference{ ARMID: s.KubeletUserAssignedIdentity, @@ -453,19 +455,19 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai } if s.HTTPProxyConfig != nil { - managedCluster.Spec.HttpProxyConfig = &asocontainerservicev1.ManagedClusterHTTPProxyConfig{ + spec.HttpProxyConfig = &asocontainerservicev1.ManagedClusterHTTPProxyConfig{ HttpProxy: s.HTTPProxyConfig.HTTPProxy, HttpsProxy: s.HTTPProxyConfig.HTTPSProxy, TrustedCa: s.HTTPProxyConfig.TrustedCA, } if s.HTTPProxyConfig.NoProxy != nil { - managedCluster.Spec.HttpProxyConfig.NoProxy = s.HTTPProxyConfig.NoProxy + spec.HttpProxyConfig.NoProxy = s.HTTPProxyConfig.NoProxy } } if s.OIDCIssuerProfile != nil { - managedCluster.Spec.OidcIssuerProfile = &asocontainerservicev1.ManagedClusterOIDCIssuerProfile{ + spec.OidcIssuerProfile = &asocontainerservicev1.ManagedClusterOIDCIssuerProfile{ Enabled: s.OIDCIssuerProfile.Enabled, } } diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go index 58158e58f33..0624ca999cb 100644 --- a/azure/services/managedclusters/spec_test.go +++ b/azure/services/managedclusters/spec_test.go @@ -495,6 +495,9 @@ func getExistingCluster() *asocontainerservicev1.ManagedCluster { mc.Spec.Tags[infrav1.ClusterTagKey("")] = mc.Spec.Tags[infrav1.ClusterTagKey("test-cluster")] delete(mc.Spec.Tags, infrav1.ClusterTagKey("test-cluster")) + // field that CAPZ does not manage, simulating user-supplied value + mc.Spec.EnablePodSecurityPolicy = ptr.To(true) + mc.Spec.AgentPoolProfiles = nil // only nil vs. non-nil matters here mc.Status.AgentPoolProfiles = []asocontainerservicev1.ManagedClusterAgentPoolProfile_STATUS{}