From 1cb133ceae94b801516993977697d400c3b02cfd Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Mon, 14 Oct 2024 18:36:34 -0700 Subject: [PATCH 01/22] feat: support shared image galleries inside of karpenter --- pkg/operator/options/options.go | 8 +- pkg/providers/imagefamily/image.go | 93 ++++++++++++++++++------ pkg/providers/imagefamily/types.go | 11 ++- pkg/providers/imagefamily/ubuntu_2204.go | 9 +-- pkg/providers/instance/instance.go | 28 ++++--- 5 files changed, 108 insertions(+), 41 deletions(-) diff --git a/pkg/operator/options/options.go b/pkg/operator/options/options.go index 3e71012bd..62752e949 100644 --- a/pkg/operator/options/options.go +++ b/pkg/operator/options/options.go @@ -75,9 +75,13 @@ type Options struct { SubnetID string // => VnetSubnetID to use (for nodes in Azure CNI Overlay and Azure CNI + pod subnet; for for nodes and pods in Azure CNI), unless overridden via AKSNodeClass setFlags map[string]bool - NodeResourceGroup string ProvisionMode string NodeBootstrappingServerURL string + ManagedKarpenter bool // => ManagedKarpenter is true if Karpenter is managed by AKS, false if it is a self-hosted karpenter installation + + SharedImageGallerySubscriptionID string + + NodeResourceGroup string } func (o *Options) AddFlags(fs *coreoptions.FlagSet) { @@ -95,6 +99,8 @@ func (o *Options) AddFlags(fs *coreoptions.FlagSet) { fs.StringVar(&o.NodeResourceGroup, "node-resource-group", env.WithDefaultString("AZURE_NODE_RESOURCE_GROUP", ""), "[REQUIRED] the resource group created and managed by AKS where the nodes live.") fs.StringVar(&o.ProvisionMode, "provision-mode", env.WithDefaultString("PROVISION_MODE", consts.ProvisionModeAKSScriptless), "[UNSUPPORTED] The provision mode for the cluster.") fs.StringVar(&o.NodeBootstrappingServerURL, "nodebootstrapping-server-url", env.WithDefaultString("NODEBOOTSTRAPPING_SERVER_URL", ""), "[UNSUPPORTED] The url for the node bootstrapping provider server.") + fs.BoolVar(&o.ManagedKarpenter, "managed-karpenter", env.WithDefaultBool("MANAGED_KARPENTER", false), "Whether Karpenter is managed by AKS or not.") + fs.StringVar(&o.SharedImageGallerySubscriptionID, "shared-image-gallery-subscription-id", env.WithDefaultString("SHARED_IMAGE_GALLERY_SUBSCRIPTION_ID", ""), "The subscription ID of the shared image gallery. Only required alongside managed-karpenter.") } func (o Options) GetAPIServerName() string { diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index b073db7e8..af2760c6f 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -25,6 +25,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" "github.com/Azure/karpenter-provider-azure/pkg/apis/v1alpha2" + "github.com/Azure/karpenter-provider-azure/pkg/operator/options" "github.com/patrickmn/go-cache" "github.com/samber/lo" "k8s.io/client-go/kubernetes" @@ -48,7 +49,8 @@ const ( imageExpirationInterval = time.Hour * 24 * 3 imageCacheCleaningInterval = time.Hour * 1 - imageIDFormat = "/CommunityGalleries/%s/images/%s/versions/%s" + sharedImageGalleryImageIDFormat = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/%s/images/%s/versions/%s" + communityImageIDFormat = "/CommunityGalleries/%s/images/%s/versions/%s" ) func NewProvider(kubernetesInterface kubernetes.Interface, kubernetesVersionCache *cache.Cache, versionsClient CommunityGalleryImageVersionsAPI, location string) *Provider { @@ -67,12 +69,14 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1alpha2.AKSNodeClass, in defaultImages := imageFamily.DefaultImages() for _, defaultImage := range defaultImages { if err := instanceType.Requirements.Compatible(defaultImage.Requirements, v1alpha2.AllowUndefinedWellKnownAndRestrictedLabels); err == nil { - communityImageName, publicGalleryURL := defaultImage.CommunityImage, defaultImage.PublicGalleryURL - imageID, err := p.GetImageID(ctx, communityImageName, publicGalleryURL) - if err != nil { - return "", "", err + // Managed Karpenter will use the AKS Managed Shared Image Galleries + if options.FromContext(ctx).ManagedKarpenter { + imgID, imageRetrievalErr := p.getImageIDSIG(ctx, defaultImage) + return defaultImage.Distro, imgID, imageRetrievalErr } - return defaultImage.Distro, imageID, nil + // Self Hosted Karpenter will use the Community Image Galleries, which are public and have lower scaling limits + imgID, imageRetrievalErr := p.getImageIDCIG(ctx, defaultImage.PublicGalleryURL, defaultImage.ImageDefinition) + return defaultImage.Distro, imgID, imageRetrievalErr } } @@ -95,14 +99,50 @@ func (p *Provider) KubeServerVersion(ctx context.Context) (string, error) { return version, nil } -// Input versionName == "" to get the latest version -func (p *Provider) GetImageID(ctx context.Context, communityImageName, publicGalleryURL string) (string, error) { - key := fmt.Sprintf("%s/%s", publicGalleryURL, communityImageName) - imageID, found := p.imageCache.Get(key) - if found { +// getImageIDSig will return a string of the shape /subscriptions/{subscriptionID}/resourceGroups/{resourceGroup}/providers/Microsoft.Compute/galleries/{galleryName}/images/{imageDefinition}/versions/{imageVersion} +// for a given imageDefinition and imageVersion. If the imageVersion is set to "", then it will return the latest version of the image. +// If the imageVersion is set to "", then we will cache the latest version of the image for imageExpirationInterval days(3d) for all imageDefinitions +// and reuse that get of the latest version of the image for 3d. +func (p *Provider) getImageIDSIG(ctx context.Context, imgStub DefaultImageOutput) (string, error) { + key := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SharedImageGallerySubscriptionID, imgStub.GalleryResourceGroup, imgStub.GalleryName, imgStub.ImageDefinition) + if imageID, ok := p.imageCache.Get(key); ok { return imageID.(string), nil } + versions, err := p.ListNodeImageVersions(ctx) + if err != nil { + return "", err + } + for _, version := range versions.Values { + imageID := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SharedImageGallerySubscriptionID, imgStub.ImageDefinition, version.Version) + p.imageCache.Set(key, imageID, imageExpirationInterval) + } + // return the latest version of the image from the cache after we have caached all of the imageDefinitions + if imageID, ok := p.imageCache.Get(key); ok { + return imageID.(string), nil + } + return "", fmt.Errorf("failed to get the latest version of the image %s", imgStub.ImageDefinition) +} +// getImageCIG will return a community image gallery image url that has the shape of +// /CommunityGalleries/{publicGalleryURL}/images/{communityImageName}/versions/{imageVersion} +// The imageVersion can be set to "" to get the latest version of the image, and after a image version "" has been encountered +// we cache the latest version of the image for imageExpirationInterval days(3d) +func (p *Provider) getImageIDCIG(ctx context.Context, publicGalleryURL, communityImageName string) (string, error) { + key := fmt.Sprintf(communityImageIDFormat, publicGalleryURL, communityImageName) + if imageID, ok := p.imageCache.Get(key); ok { + return imageID.(string), nil + } + // if the image is not found in the cache, we will refresh the lookup for it + imageVersion, err := p.latestNodeImageVersionCommmunity(ctx, publicGalleryURL, communityImageName) + if err != nil { + return "", err + } + imageID := BuildImageIDCIG(publicGalleryURL, communityImageName, imageVersion) + p.imageCache.Set(key, imageID, imageExpirationInterval) + return imageID, nil +} + +func (p *Provider) latestNodeImageVersionCommmunity(ctx context.Context, publicGalleryURL, communityImageName string) (string, error) { pager := p.imageVersionsClient.NewListPager(p.location, publicGalleryURL, communityImageName, nil) topImageVersionCandidate := armcompute.CommunityGalleryImageVersion{} for pager.More() { @@ -116,24 +156,17 @@ func (p *Provider) GetImageID(ctx context.Context, communityImageName, publicGal } } } - versionName := lo.FromPtr(topImageVersionCandidate.Name) - - selectedImageID := BuildImageID(publicGalleryURL, communityImageName, versionName) - if p.cm.HasChanged(key, selectedImageID) { - logging.FromContext(ctx).With("image-id", selectedImageID).Info("discovered new image id") - } - p.imageCache.Set(key, selectedImageID, imageExpirationInterval) - return selectedImageID, nil + return lo.FromPtr(topImageVersionCandidate.Name), nil } -func BuildImageID(publicGalleryURL, communityImageName, imageVersion string) string { - return fmt.Sprintf(imageIDFormat, publicGalleryURL, communityImageName, imageVersion) +func BuildImageIDCIG(publicGalleryURL, communityImageName, imageVersion string) string { + return fmt.Sprintf(communityImageIDFormat, publicGalleryURL, communityImageName, imageVersion) } // ParseImageIDInfo parses the publicGalleryURL, communityImageName, and imageVersion out of an imageID func ParseCommunityImageIDInfo(imageID string) (string, string, string, error) { // TODO (charliedmcb): assess if doing validation on splitting the string and validating the results is better? Mostly is regex too expensive? - regexStr := fmt.Sprintf(imageIDFormat, "(?P.*)", "(?P.*)", "(?P.*)") + regexStr := fmt.Sprintf(communityImageIDFormat, "(?P.*)", "(?P.*)", "(?P.*)") if imageID == "" { return "", "", "", fmt.Errorf("can not parse empty string. Expect it of the form \"%s\"", regexStr) } @@ -147,3 +180,19 @@ func ParseCommunityImageIDInfo(imageID string) (string, string, string, error) { } return matches[r.SubexpIndex("publicGalleryURL")], matches[r.SubexpIndex("communityImageName")], matches[r.SubexpIndex("imageVersion")], nil } + +type NodeImageVersion struct { + FullName string `json:"fullName"` + OS string `json:"os"` + SKU string `json:"sku"` + Version string `json:"version"` +} + +type NodeImageVersionsResponse struct { + Values []NodeImageVersion `json:"values"` +} + +func (p *Provider) ListNodeImageVersions(ctx context.Context) (NodeImageVersionsResponse, error) { + // call the Azure API to get the latest image versions + return NodeImageVersionsResponse{}, nil +} diff --git a/pkg/providers/imagefamily/types.go b/pkg/providers/imagefamily/types.go index b3c3510aa..093d82332 100644 --- a/pkg/providers/imagefamily/types.go +++ b/pkg/providers/imagefamily/types.go @@ -27,12 +27,17 @@ const ( AKSAzureLinuxPublicGalleryURL = "AKSAzureLinux-f7c7cda5-1c9a-4bdc-a222-9614c968580b" ) -// DefaultImageOutput is the Stub of an Image we return from an ImageFamily De +// DefaultImageOutput is a stub describing our desired image with an image's name and requirements to run that image type DefaultImageOutput struct { - CommunityImage string + // Community Image Gallery PublicGalleryURL string - Requirements scheduling.Requirements + // Shared Image Gallery + GalleryResourceGroup string + GalleryName string + // Common + ImageDefinition string Distro string + Requirements scheduling.Requirements } // CommunityGalleryImageVersionsAPI is used for listing community gallery image versions. diff --git a/pkg/providers/imagefamily/ubuntu_2204.go b/pkg/providers/imagefamily/ubuntu_2204.go index 21d3b8c03..0b95e527e 100644 --- a/pkg/providers/imagefamily/ubuntu_2204.go +++ b/pkg/providers/imagefamily/ubuntu_2204.go @@ -47,8 +47,7 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { // image provider will select these images in order, first match wins. This is why we chose to put Ubuntu2204Gen2containerd first in the defaultImages return []DefaultImageOutput{ { - CommunityImage: Ubuntu2204Gen2CommunityImage, - PublicGalleryURL: AKSUbuntuPublicGalleryURL, + ImageDefinition: Ubuntu2204Gen2CommunityImage, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), @@ -56,8 +55,7 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { Distro: "aks-ubuntu-containerd-22.04-gen2", }, { - CommunityImage: Ubuntu2204Gen1CommunityImage, - PublicGalleryURL: AKSUbuntuPublicGalleryURL, + ImageDefinition: Ubuntu2204Gen1CommunityImage, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV1), @@ -65,8 +63,7 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { Distro: "aks-ubuntu-containerd-22.04", }, { - CommunityImage: Ubuntu2204Gen2ArmCommunityImage, - PublicGalleryURL: AKSUbuntuPublicGalleryURL, + ImageDefinition: Ubuntu2204Gen2ArmCommunityImage, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureArm64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index 89939746e..bf89b081e 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -311,22 +311,18 @@ func (p *DefaultProvider) GetNic(ctx context.Context, rg, nicName string) (*armn // newVMObject is a helper func that creates a new armcompute.VirtualMachine // from key input. func newVMObject( + ctx context.Context, vmName, nicReference, zone, - capacityType string, - location string, + capacityType, + location, sshPublicKey string, nodeIdentities []string, nodeClass *v1alpha2.AKSNodeClass, launchTemplate *launchtemplate.Template, instanceType *corecloudprovider.InstanceType, provisionMode string) armcompute.VirtualMachine { - // Build the image reference from template - imageReference := armcompute.ImageReference{ - CommunityGalleryImageID: &launchTemplate.ImageID, - } - if launchTemplate.IsWindows { return armcompute.VirtualMachine{} // TODO(Windows) } @@ -346,7 +342,6 @@ func newVMObject( CreateOption: lo.ToPtr(armcompute.DiskCreateOptionTypesFromImage), DeleteOption: lo.ToPtr(armcompute.DiskDeleteOptionTypesDelete), }, - ImageReference: &imageReference, }, NetworkProfile: &armcompute.NetworkProfile{ @@ -384,6 +379,7 @@ func newVMObject( Tags: launchTemplate.Tags, } setVMPropertiesOSDiskType(vm.Properties, launchTemplate.StorageProfile) + setImageReference(ctx, vm.Properties, launchTemplate.ImageID) setVMPropertiesBillingProfile(vm.Properties, capacityType) if provisionMode == consts.ProvisionModeBootstrappingClient { @@ -406,6 +402,20 @@ func setVMPropertiesOSDiskType(vmProperties *armcompute.VirtualMachineProperties } } +// setImageReference sets the image reference for the VM based on if we are using self hosted karpenter or the node auto provisioning addon +func setImageReference(ctx context.Context, vmProperties *armcompute.VirtualMachineProperties, imageID string) { + if options.FromContext(ctx).ManagedKarpenter { + vmProperties.StorageProfile.ImageReference = &armcompute.ImageReference{ + ID: lo.ToPtr(imageID), + } + return + } + vmProperties.StorageProfile.ImageReference = &armcompute.ImageReference{ + CommunityGalleryImageID: lo.ToPtr(imageID), + } + return +} + // setVMPropertiesBillingProfile sets a default MaxPrice of -1 for Spot func setVMPropertiesBillingProfile(vmProperties *armcompute.VirtualMachineProperties, capacityType string) { if capacityType == karpv1.CapacityTypeSpot { @@ -471,7 +481,7 @@ func (p *DefaultProvider) launchInstance( sshPublicKey := options.FromContext(ctx).SSHPublicKey nodeIdentityIDs := options.FromContext(ctx).NodeIdentities - vm := newVMObject(resourceName, nicReference, zone, capacityType, p.location, sshPublicKey, nodeIdentityIDs, nodeClass, launchTemplate, instanceType, p.provisionMode) + vm := newVMObject(ctx, resourceName, nicReference, zone, capacityType, p.location, sshPublicKey, nodeIdentityIDs, nodeClass, launchTemplate, instanceType, p.provisionMode) logging.FromContext(ctx).Debugf("Creating virtual machine %s (%s)", resourceName, instanceType.Name) // Uses AZ Client to create a new virtual machine using the vm object we prepared earlier From db34e358b4dafaedf3548866747b916fdc00afa3 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Mon, 14 Oct 2024 18:44:24 -0700 Subject: [PATCH 02/22] chore: populating image stubs for shared image galleries --- pkg/providers/imagefamily/azlinux.go | 24 +++++++++++++++--------- pkg/providers/imagefamily/types.go | 8 ++++++++ pkg/providers/imagefamily/ubuntu_2204.go | 15 ++++++++++++--- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/pkg/providers/imagefamily/azlinux.go b/pkg/providers/imagefamily/azlinux.go index 13be8fe07..a89ef898e 100644 --- a/pkg/providers/imagefamily/azlinux.go +++ b/pkg/providers/imagefamily/azlinux.go @@ -30,9 +30,9 @@ import ( ) const ( - AzureLinuxGen2CommunityImage = "V2gen2" - AzureLinuxGen1CommunityImage = "V2" - AzureLinuxGen2ArmCommunityImage = "V2gen2arm64" + AzureLinuxGen2ImageDefinition = "V2gen2" + AzureLinuxGen1ImageDefinition = "V2" + AzureLinuxGen2ArmImageDefinition = "V2gen2arm64" ) type AzureLinux struct { @@ -47,8 +47,10 @@ func (u AzureLinux) DefaultImages() []DefaultImageOutput { // image provider will select these images in order, first match wins. This is why we chose to put Gen2 first in the defaultImages, as we prefer gen2 over gen1 return []DefaultImageOutput{ { - CommunityImage: AzureLinuxGen2CommunityImage, - PublicGalleryURL: AKSAzureLinuxPublicGalleryURL, + PublicGalleryURL: AKSAzureLinuxPublicGalleryURL, + GalleryResourceGroup: AKSAzureLinuxResourceGroup, + GalleryName: AKSAzureLinuxGalleryName, + ImageDefinition: AzureLinuxGen2ImageDefinition, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), @@ -56,8 +58,10 @@ func (u AzureLinux) DefaultImages() []DefaultImageOutput { Distro: "aks-azurelinux-v2-gen2", }, { - CommunityImage: AzureLinuxGen1CommunityImage, - PublicGalleryURL: AKSAzureLinuxPublicGalleryURL, + PublicGalleryURL: AKSAzureLinuxPublicGalleryURL, + GalleryResourceGroup: AKSAzureLinuxResourceGroup, + GalleryName: AKSAzureLinuxGalleryName, + ImageDefinition: AzureLinuxGen1ImageDefinition, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV1), @@ -65,8 +69,10 @@ func (u AzureLinux) DefaultImages() []DefaultImageOutput { Distro: "aks-azurelinux-v2", }, { - CommunityImage: AzureLinuxGen2ArmCommunityImage, - PublicGalleryURL: AKSAzureLinuxPublicGalleryURL, + PublicGalleryURL: AKSAzureLinuxPublicGalleryURL, + GalleryResourceGroup: AKSAzureLinuxResourceGroup, + GalleryName: AKSAzureLinuxGalleryName, + ImageDefinition: AzureLinuxGen2ArmImageDefinition, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureArm64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), diff --git a/pkg/providers/imagefamily/types.go b/pkg/providers/imagefamily/types.go index 093d82332..7623d4af8 100644 --- a/pkg/providers/imagefamily/types.go +++ b/pkg/providers/imagefamily/types.go @@ -25,6 +25,14 @@ import ( const ( AKSUbuntuPublicGalleryURL = "AKSUbuntu-38d80f77-467a-481f-a8d4-09b6d4220bd2" AKSAzureLinuxPublicGalleryURL = "AKSAzureLinux-f7c7cda5-1c9a-4bdc-a222-9614c968580b" + + AKSUbuntuResourceGroup = "AKS-Ubuntu" + AKSAzureLinuxResourceGroup = "AKS-Azure-Linux" + AKSWindowsResourceGroup = "AKS-Windows" + + AKSUbuntuGalleryName = "AKSUbuntu" + AKSAzureLinuxGalleryName = "AKSAzureLinux" + AKSWindowsGalleryName = "AKSWindows" ) // DefaultImageOutput is a stub describing our desired image with an image's name and requirements to run that image diff --git a/pkg/providers/imagefamily/ubuntu_2204.go b/pkg/providers/imagefamily/ubuntu_2204.go index 0b95e527e..a99197b72 100644 --- a/pkg/providers/imagefamily/ubuntu_2204.go +++ b/pkg/providers/imagefamily/ubuntu_2204.go @@ -47,7 +47,10 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { // image provider will select these images in order, first match wins. This is why we chose to put Ubuntu2204Gen2containerd first in the defaultImages return []DefaultImageOutput{ { - ImageDefinition: Ubuntu2204Gen2CommunityImage, + PublicGalleryURL: AKSUbuntuPublicGalleryURL, + GalleryResourceGroup: AKSUbuntuResourceGroup, + GalleryName: AKSUbuntuGalleryName, + ImageDefinition: Ubuntu2204Gen2CommunityImage, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), @@ -55,7 +58,10 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { Distro: "aks-ubuntu-containerd-22.04-gen2", }, { - ImageDefinition: Ubuntu2204Gen1CommunityImage, + PublicGalleryURL: AKSUbuntuPublicGalleryURL, + GalleryResourceGroup: AKSUbuntuResourceGroup, + GalleryName: AKSUbuntuGalleryName, + ImageDefinition: Ubuntu2204Gen1CommunityImage, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV1), @@ -63,7 +69,10 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { Distro: "aks-ubuntu-containerd-22.04", }, { - ImageDefinition: Ubuntu2204Gen2ArmCommunityImage, + PublicGalleryURL: AKSUbuntuPublicGalleryURL, + GalleryResourceGroup: AKSUbuntuResourceGroup, + GalleryName: AKSUbuntuGalleryName, + ImageDefinition: Ubuntu2204Gen2ArmCommunityImage, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureArm64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), From ac5e94352986869911641125e364a20ea840fd97 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 11:18:17 -0700 Subject: [PATCH 03/22] fix: progress --- pkg/cloudprovider/drift.go | 17 ++++---- pkg/providers/imagefamily/image.go | 49 +++++++++++++----------- pkg/providers/imagefamily/image_test.go | 2 +- pkg/providers/imagefamily/types.go | 20 ++++++++++ pkg/providers/imagefamily/ubuntu_2204.go | 12 +++--- pkg/providers/instancetype/suite_test.go | 13 ++++--- 6 files changed, 68 insertions(+), 45 deletions(-) diff --git a/pkg/cloudprovider/drift.go b/pkg/cloudprovider/drift.go index 98c1114bd..aa2befb76 100644 --- a/pkg/cloudprovider/drift.go +++ b/pkg/cloudprovider/drift.go @@ -151,15 +151,14 @@ func (c *CloudProvider) isImageVersionDrifted( logger.Debug("not using a CommunityGalleryImageID for nodeClaim %s", nodeClaim.Name) return "", nil } - - vmImageID := *vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID - - publicGalleryURL, communityImageName, _, err := imagefamily.ParseCommunityImageIDInfo(vmImageID) - if err != nil { - return "", err - } - - expectedImageID, err := c.imageProvider.GetImageID(ctx, communityImageName, publicGalleryURL) + + communityImageID := lo.FromPtr(vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID) + sharedImageGalleryID := lo.FromPtr(vm.Properties.StorageProfile.ImageReference.ID) + vmImageID := lo.Ternary(communityImageID == "", sharedImageGalleryID, communityImageID) + var imageStub *imagefamily.DefaultImageOutput + imageStub.PopulateImageTraitsFromID(vmImageID) + + expectedImageID, err := c.imageProvider.GetLatestImageID(ctx, *imageStub) if err != nil { return "", err } diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index af2760c6f..73afc10f0 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -69,20 +69,23 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1alpha2.AKSNodeClass, in defaultImages := imageFamily.DefaultImages() for _, defaultImage := range defaultImages { if err := instanceType.Requirements.Compatible(defaultImage.Requirements, v1alpha2.AllowUndefinedWellKnownAndRestrictedLabels); err == nil { - // Managed Karpenter will use the AKS Managed Shared Image Galleries - if options.FromContext(ctx).ManagedKarpenter { - imgID, imageRetrievalErr := p.getImageIDSIG(ctx, defaultImage) - return defaultImage.Distro, imgID, imageRetrievalErr - } - // Self Hosted Karpenter will use the Community Image Galleries, which are public and have lower scaling limits - imgID, imageRetrievalErr := p.getImageIDCIG(ctx, defaultImage.PublicGalleryURL, defaultImage.ImageDefinition) - return defaultImage.Distro, imgID, imageRetrievalErr + imageID, imageRetrievalErr := p.GetLatestImageID(ctx, defaultImage) + return defaultImage.Distro, imageID, imageRetrievalErr } } return "", "", fmt.Errorf("no compatible images found for instance type %s", instanceType.Name) } +func (p *Provider) GetLatestImageID(ctx context.Context, defaultImage DefaultImageOutput) (string, error) { + // Managed Karpenter will use the AKS Managed Shared Image Galleries + if options.FromContext(ctx).ManagedKarpenter { + return p.getImageIDSIG(ctx, defaultImage) + } + // Self Hosted Karpenter will use the Community Image Galleries, which are public and have lower scaling limits + return p.getImageIDCIG(ctx, defaultImage.PublicGalleryURL, defaultImage.ImageDefinition) +} + func (p *Provider) KubeServerVersion(ctx context.Context) (string, error) { if version, ok := p.kubernetesVersionCache.Get(kubernetesVersionCacheKey); ok { return version.(string), nil @@ -108,19 +111,19 @@ func (p *Provider) getImageIDSIG(ctx context.Context, imgStub DefaultImageOutput if imageID, ok := p.imageCache.Get(key); ok { return imageID.(string), nil } - versions, err := p.ListNodeImageVersions(ctx) - if err != nil { - return "", err - } - for _, version := range versions.Values { - imageID := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SharedImageGallerySubscriptionID, imgStub.ImageDefinition, version.Version) - p.imageCache.Set(key, imageID, imageExpirationInterval) - } - // return the latest version of the image from the cache after we have caached all of the imageDefinitions - if imageID, ok := p.imageCache.Get(key); ok { - return imageID.(string), nil - } - return "", fmt.Errorf("failed to get the latest version of the image %s", imgStub.ImageDefinition) + versions, err := p.ListNodeImageVersions(ctx) + if err != nil { + return "", err + } + for _, version := range versions.Values { + imageID := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SharedImageGallerySubscriptionID, imgStub.GalleryResourceGroup, imgStub.GalleryName, imgStub.ImageDefinition, version.Version) + p.imageCache.Set(key, imageID, imageExpirationInterval) + } + // return the latest version of the image from the cache after we have caached all of the imageDefinitions + if imageID, ok := p.imageCache.Get(key); ok { + return imageID.(string), nil + } + return "", fmt.Errorf("failed to get the latest version of the image %s", imgStub.ImageDefinition) } // getImageCIG will return a community image gallery image url that has the shape of @@ -133,7 +136,7 @@ func (p *Provider) getImageIDCIG(ctx context.Context, publicGalleryURL, communit return imageID.(string), nil } // if the image is not found in the cache, we will refresh the lookup for it - imageVersion, err := p.latestNodeImageVersionCommmunity(ctx, publicGalleryURL, communityImageName) + imageVersion, err := p.latestNodeImageVersionCommmunity(publicGalleryURL, communityImageName) if err != nil { return "", err } @@ -142,7 +145,7 @@ func (p *Provider) getImageIDCIG(ctx context.Context, publicGalleryURL, communit return imageID, nil } -func (p *Provider) latestNodeImageVersionCommmunity(ctx context.Context, publicGalleryURL, communityImageName string) (string, error) { +func (p *Provider) latestNodeImageVersionCommmunity(publicGalleryURL, communityImageName string) (string, error) { pager := p.imageVersionsClient.NewListPager(p.location, publicGalleryURL, communityImageName, nil) topImageVersionCandidate := armcompute.CommunityGalleryImageVersion{} for pager.More() { diff --git a/pkg/providers/imagefamily/image_test.go b/pkg/providers/imagefamily/image_test.go index 0da43af9e..3a27c68a5 100644 --- a/pkg/providers/imagefamily/image_test.go +++ b/pkg/providers/imagefamily/image_test.go @@ -50,7 +50,7 @@ var _ = Describe("Image ID Parsing", func() { Expect(communityImageName).To(Equal(expectedCommunityImageName)) Expect(imageVersion).To(Equal(expectedImageVersion)) }, - Entry("Valid image id should parse", fmt.Sprintf("/CommunityGalleries/%s/images/%s/versions/%s", imagefamily.AKSUbuntuPublicGalleryURL, imagefamily.Ubuntu2204Gen2CommunityImage, olderImageVersion), imagefamily.AKSUbuntuPublicGalleryURL, imagefamily.Ubuntu2204Gen2CommunityImage, olderImageVersion, nil), + Entry("Valid image id should parse", fmt.Sprintf("/CommunityGalleries/%s/images/%s/versions/%s", imagefamily.AKSUbuntuPublicGalleryURL, imagefamily.Ubuntu2204Gen2ImageDefinition, olderImageVersion), imagefamily.AKSUbuntuPublicGalleryURL, imagefamily.Ubuntu2204Gen2ImageDefinition, olderImageVersion, nil), Entry("invalid image id should not parse", "badimageid", "", "", "", true), Entry("empty image id should not parse", "badimageid", "", "", "", true), ) diff --git a/pkg/providers/imagefamily/types.go b/pkg/providers/imagefamily/types.go index 7623d4af8..3dc169c73 100644 --- a/pkg/providers/imagefamily/types.go +++ b/pkg/providers/imagefamily/types.go @@ -17,6 +17,8 @@ limitations under the License. package imagefamily import ( + "strings" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" armcomputev5 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" "sigs.k8s.io/karpenter/pkg/scheduling" @@ -48,6 +50,24 @@ type DefaultImageOutput struct { Requirements scheduling.Requirements } +func (d DefaultImageOutput) PopulateImageTraitsFromID(imageID string) { + // We want to take a community image gallery image id or a shared image gallery id and populate the contents of DefaultImageOutput + imageIDParts := strings.Split(imageID, "/") + if len(imageIDParts) < 8 { // not enough parts to be a valid image id + return + } + if imageIDParts[1] == "subscriptions" { // Shared Image Gallery + d.GalleryResourceGroup = imageIDParts[4] + d.GalleryName = imageIDParts[8] + d.ImageDefinition = imageIDParts[10] + } + if imageIDParts[1] == "CommunityGalleries" { // Community Image Gallery + d.PublicGalleryURL = imageIDParts[2] + d.GalleryName = imageIDParts[4] + d.ImageDefinition = imageIDParts[6] + } +} + // CommunityGalleryImageVersionsAPI is used for listing community gallery image versions. type CommunityGalleryImageVersionsAPI interface { NewListPager(location string, publicGalleryName string, galleryImageName string, options *armcomputev5.CommunityGalleryImageVersionsClientListOptions) *runtime.Pager[armcomputev5.CommunityGalleryImageVersionsClientListResponse] diff --git a/pkg/providers/imagefamily/ubuntu_2204.go b/pkg/providers/imagefamily/ubuntu_2204.go index a99197b72..947d45e41 100644 --- a/pkg/providers/imagefamily/ubuntu_2204.go +++ b/pkg/providers/imagefamily/ubuntu_2204.go @@ -30,9 +30,9 @@ import ( ) const ( - Ubuntu2204Gen2CommunityImage = "2204gen2containerd" - Ubuntu2204Gen1CommunityImage = "2204containerd" - Ubuntu2204Gen2ArmCommunityImage = "2204gen2arm64containerd" + Ubuntu2204Gen2ImageDefinition = "2204gen2containerd" + Ubuntu2204Gen1ImageDefinition = "2204containerd" + Ubuntu2204Gen2ArmImageDefinition = "2204gen2arm64containerd" ) type Ubuntu2204 struct { @@ -50,7 +50,7 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { PublicGalleryURL: AKSUbuntuPublicGalleryURL, GalleryResourceGroup: AKSUbuntuResourceGroup, GalleryName: AKSUbuntuGalleryName, - ImageDefinition: Ubuntu2204Gen2CommunityImage, + ImageDefinition: Ubuntu2204Gen2ImageDefinition, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), @@ -61,7 +61,7 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { PublicGalleryURL: AKSUbuntuPublicGalleryURL, GalleryResourceGroup: AKSUbuntuResourceGroup, GalleryName: AKSUbuntuGalleryName, - ImageDefinition: Ubuntu2204Gen1CommunityImage, + ImageDefinition: Ubuntu2204Gen1ImageDefinition, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV1), @@ -72,7 +72,7 @@ func (u Ubuntu2204) DefaultImages() []DefaultImageOutput { PublicGalleryURL: AKSUbuntuPublicGalleryURL, GalleryResourceGroup: AKSUbuntuResourceGroup, GalleryName: AKSUbuntuGalleryName, - ImageDefinition: Ubuntu2204Gen2ArmCommunityImage, + ImageDefinition: Ubuntu2204Gen2ArmImageDefinition, Requirements: scheduling.NewRequirements( scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, karpv1.ArchitectureArm64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, v1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2), diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index c0e30f560..50ab45440 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -913,19 +913,20 @@ var _ = Describe("InstanceType Provider", func() { azureEnv.Reset() }, Entry("Gen2, Gen1 instance type with AKSUbuntu image family", - "Standard_D2_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2CommunityImage, imagefamily.AKSUbuntuPublicGalleryURL), + "Standard_D2_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2ImageDefinition, imagefamily.AKSUbuntuPublicGalleryURL), Entry("Gen1 instance type with AKSUbuntu image family", - "Standard_D2_v3", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen1CommunityImage, imagefamily.AKSUbuntuPublicGalleryURL), + "Standard_D2_v3", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen1ImageDefinition, imagefamily.AKSUbuntuPublicGalleryURL), Entry("ARM instance type with AKSUbuntu image family", - "Standard_D16plds_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2ArmCommunityImage, imagefamily.AKSUbuntuPublicGalleryURL), + "Standard_D16plds_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2ArmImageDefinition, imagefamily.AKSUbuntuPublicGalleryURL), Entry("Gen2 instance type with AzureLinux image family", - "Standard_D2_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2CommunityImage, imagefamily.AKSAzureLinuxPublicGalleryURL), + "Standard_D2_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2ImageDefinition, imagefamily.AKSAzureLinuxPublicGalleryURL), Entry("Gen1 instance type with AzureLinux image family", - "Standard_D2_v3", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen1CommunityImage, imagefamily.AKSAzureLinuxPublicGalleryURL), + "Standard_D2_v3", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen1ImageDefinition, imagefamily.AKSAzureLinuxPublicGalleryURL), Entry("ARM instance type with AzureLinux image family", - "Standard_D16plds_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2ArmCommunityImage, imagefamily.AKSAzureLinuxPublicGalleryURL), + "Standard_D16plds_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2ArmImageDefinition, imagefamily.AKSAzureLinuxPublicGalleryURL), ) }) + Context("ImageReference") Context("Instance Types", func() { It("should support provisioning with no labels", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) From 78fc1ec67a37e8e8810da38f1f9bbdf6ccdd53a1 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 16:40:10 -0700 Subject: [PATCH 04/22] fix: PopulateResourceStub accessing the wrong index --- pkg/cloudprovider/drift.go | 17 +++++++---------- pkg/providers/imagefamily/types.go | 8 ++------ pkg/providers/instancetype/suite_test.go | 1 - 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/pkg/cloudprovider/drift.go b/pkg/cloudprovider/drift.go index aa2befb76..54d680129 100644 --- a/pkg/cloudprovider/drift.go +++ b/pkg/cloudprovider/drift.go @@ -145,20 +145,17 @@ func (c *CloudProvider) isImageVersionDrifted( if vm.Properties == nil || vm.Properties.StorageProfile == nil || - vm.Properties.StorageProfile.ImageReference == nil || - vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID == nil || - *vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID == "" { - logger.Debug("not using a CommunityGalleryImageID for nodeClaim %s", nodeClaim.Name) + vm.Properties.StorageProfile.ImageReference == nil { return "", nil } - - communityImageID := lo.FromPtr(vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID) - sharedImageGalleryID := lo.FromPtr(vm.Properties.StorageProfile.ImageReference.ID) - vmImageID := lo.Ternary(communityImageID == "", sharedImageGalleryID, communityImageID) - var imageStub *imagefamily.DefaultImageOutput + CIGID := lo.FromPtr(vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID) + SIGID := lo.FromPtr(vm.Properties.StorageProfile.ImageReference.ID) + vmImageID := lo.Ternary(SIGID != "", SIGID, CIGID) + + var imageStub imagefamily.DefaultImageOutput imageStub.PopulateImageTraitsFromID(vmImageID) - expectedImageID, err := c.imageProvider.GetLatestImageID(ctx, *imageStub) + expectedImageID, err := c.imageProvider.GetLatestImageID(ctx, imageStub) if err != nil { return "", err } diff --git a/pkg/providers/imagefamily/types.go b/pkg/providers/imagefamily/types.go index 3dc169c73..374e0e7e6 100644 --- a/pkg/providers/imagefamily/types.go +++ b/pkg/providers/imagefamily/types.go @@ -50,12 +50,9 @@ type DefaultImageOutput struct { Requirements scheduling.Requirements } -func (d DefaultImageOutput) PopulateImageTraitsFromID(imageID string) { +func (d *DefaultImageOutput) PopulateImageTraitsFromID(imageID string) { // We want to take a community image gallery image id or a shared image gallery id and populate the contents of DefaultImageOutput imageIDParts := strings.Split(imageID, "/") - if len(imageIDParts) < 8 { // not enough parts to be a valid image id - return - } if imageIDParts[1] == "subscriptions" { // Shared Image Gallery d.GalleryResourceGroup = imageIDParts[4] d.GalleryName = imageIDParts[8] @@ -63,8 +60,7 @@ func (d DefaultImageOutput) PopulateImageTraitsFromID(imageID string) { } if imageIDParts[1] == "CommunityGalleries" { // Community Image Gallery d.PublicGalleryURL = imageIDParts[2] - d.GalleryName = imageIDParts[4] - d.ImageDefinition = imageIDParts[6] + d.ImageDefinition = imageIDParts[4] } } diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index 50ab45440..876b982cb 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -926,7 +926,6 @@ var _ = Describe("InstanceType Provider", func() { "Standard_D16plds_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2ArmImageDefinition, imagefamily.AKSAzureLinuxPublicGalleryURL), ) }) - Context("ImageReference") Context("Instance Types", func() { It("should support provisioning with no labels", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) From 24ec40fcd53d8c3a53209b0451a9857082ca14bd Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 18:37:12 -0700 Subject: [PATCH 05/22] test: properly testing ListNodeImageVersions --- pkg/fake/nodeimageversionsapi.go | 361 ++++++++++++++++++ pkg/operator/operator.go | 3 + pkg/operator/options/options.go | 2 +- pkg/providers/imagefamily/image.go | 41 +- pkg/providers/imagefamily/image_test.go | 57 --- .../imagefamily/nodeimageversionsclient.go | 58 +++ pkg/providers/imagefamily/types.go | 16 + pkg/providers/instance/azure_client.go | 14 +- pkg/test/environment.go | 9 +- 9 files changed, 460 insertions(+), 101 deletions(-) create mode 100644 pkg/fake/nodeimageversionsapi.go delete mode 100644 pkg/providers/imagefamily/image_test.go create mode 100644 pkg/providers/imagefamily/nodeimageversionsclient.go diff --git a/pkg/fake/nodeimageversionsapi.go b/pkg/fake/nodeimageversionsapi.go new file mode 100644 index 000000000..45a99bb18 --- /dev/null +++ b/pkg/fake/nodeimageversionsapi.go @@ -0,0 +1,361 @@ +package fake + +import ( + "context" + + "github.com/Azure/karpenter-provider-azure/pkg/providers/imagefamily" +) + +type NodeImageVersionsAPI struct { +} + +var _ imagefamily.NodeImageVersionsAPI = &NodeImageVersionsAPI{} + +func (n NodeImageVersionsAPI) List(_ context.Context, _, _ string) (imagefamily.NodeImageVersionsResponse, error) { + return imagefamily.NodeImageVersionsResponse{ + Values: []imagefamily.NodeImageVersion{ + { + FullName: "AKSUbuntu-1804gpucontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "1804gpucontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSWindows-2019-17763.2019.221114", + OS: "AKSWindows", + SKU: "windows-2019", + Version: "17763.2019.221114", + }, + { + FullName: "AKSAzureLinux-V3-202409.23.0", + OS: "AKSAzureLinux", + SKU: "V3", + Version: "202409.23.0", + }, + { + FullName: "AKSUbuntu-2204gen2containerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "2204gen2containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1804gpu-2022.08.29", + OS: "AKSUbuntu", + SKU: "1804gpu", + Version: "2022.08.29", + }, + { + FullName: "AKSWindows-2022-containerd-gen2-20348.2762.241009", + OS: "AKSWindows", + SKU: "windows-2022-containerd-gen2", + Version: "20348.2762.241009", + }, + { + FullName: "AKSCBLMariner-V2katagen2TL-2022.12.15", + OS: "AKSCBLMariner", + SKU: "V2katagen2TL", + Version: "2022.12.15", + }, + { + FullName: "AKSUbuntu-2004gen2fipscontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "2004gen2fipscontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V2fips-202410.09.0", + OS: "AKSAzureLinux", + SKU: "V2fips", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V2gen2fips-202410.09.0", + OS: "AKSAzureLinux", + SKU: "V2gen2fips", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntuEdgeZone-1804gen2containerd-202410.09.0", + OS: "AKSUbuntuEdgeZone", + SKU: "1804gen2containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V2katagen2-202410.09.0", + OS: "AKSAzureLinux", + SKU: "V2katagen2", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V3gen2-202409.23.0", + OS: "AKSAzureLinux", + SKU: "V3gen2", + Version: "202409.23.0", + }, + { + FullName: "AKSUbuntu-2204gen2arm64containerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "2204gen2arm64containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1804gen2gpucontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "1804gen2gpucontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V2gen2TL-202410.09.0", + OS: "AKSAzureLinux", + SKU: "V2gen2TL", + Version: "202410.09.0", + }, + { + FullName: "AKSCBLMariner-V2-202410.09.0", + OS: "AKSCBLMariner", + SKU: "V2", + Version: "202410.09.0", + }, + { + FullName: "AKSCBLMariner-V2fips-202410.09.0", + OS: "AKSCBLMariner", + SKU: "V2fips", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1804containerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "1804containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntuEdgeZone-1804containerd-202410.09.0", + OS: "AKSUbuntuEdgeZone", + SKU: "1804containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntuEdgeZone-2204gen2containerd-202410.09.0", + OS: "AKSUbuntuEdgeZone", + SKU: "2204gen2containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1804fipscontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "1804fipscontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V2gen2arm64-202410.09.0", + OS: "AKSAzureLinux", + SKU: "V2gen2arm64", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V2gen2-202410.09.0", + OS: "AKSAzureLinux", + SKU: "V2gen2", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-2204gen2fipscontainerd-202404.09.0", + OS: "AKSUbuntu", + SKU: "2204gen2fipscontainerd", + Version: "202404.09.0", + }, + { + FullName: "AKSWindows-2019-containerd-17763.6414.241010", + OS: "AKSWindows", + SKU: "windows-2019-containerd", + Version: "17763.6414.241010", + }, + { + FullName: "AKSUbuntu-2204gen2TLcontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "2204gen2TLcontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1804-2022.08.29", + OS: "AKSUbuntu", + SKU: "1804", + Version: "2022.08.29", + }, + { + FullName: "AKSUbuntuEdgeZone-2204containerd-202410.09.0", + OS: "AKSUbuntuEdgeZone", + SKU: "2204containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSCBLMariner-V2gen2-202410.09.0", + OS: "AKSCBLMariner", + SKU: "V2gen2", + Version: "202410.09.0", + }, + { + FullName: "AKSCBLMariner-V2gen2fips-202410.09.0", + OS: "AKSCBLMariner", + SKU: "V2gen2fips", + Version: "202410.09.0", + }, + { + FullName: "AKSCBLMariner-V2gen2arm64-202410.09.0", + OS: "AKSCBLMariner", + SKU: "V2gen2arm64", + Version: "202410.09.0", + }, + { + FullName: "AKSCBLMariner-V2gen2TL-202410.09.0", + OS: "AKSCBLMariner", + SKU: "V2gen2TL", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-2404gen2arm64containerd-202405.20.0", + OS: "AKSUbuntu", + SKU: "2404gen2arm64containerd", + Version: "202405.20.0", + }, + { + FullName: "AKSAzureLinux-V3gen2arm64-202409.23.0", + OS: "AKSAzureLinux", + SKU: "V3gen2arm64", + Version: "202409.23.0", + }, + { + FullName: "AKSAzureLinux-V3fips-202409.23.0", + OS: "AKSAzureLinux", + SKU: "V3fips", + Version: "202409.23.0", + }, + { + FullName: "AKSUbuntu-2004gen2CVMcontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "2004gen2CVMcontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSCBLMariner-V1-202308.28.0", + OS: "AKSCBLMariner", + SKU: "V1", + Version: "202308.28.0", + }, + { + FullName: "AKSUbuntu-1804gen2containerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "1804gen2containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1804gen2gpu-2022.08.29", + OS: "AKSUbuntu", + SKU: "1804gen2gpu", + Version: "2022.08.29", + }, + { + FullName: "AKSUbuntu-2204gen2minimalcontainerd-202401.12.0", + OS: "AKSUbuntu", + SKU: "2204gen2minimalcontainerd", + Version: "202401.12.0", + }, + { + FullName: "AKSWindows-23H2-gen2-25398.1189.241009", + OS: "AKSWindows", + SKU: "windows-23H2-gen2", + Version: "25398.1189.241009", + }, + { + FullName: "AKSCBLMariner-V2katagen2-202410.09.0", + OS: "AKSCBLMariner", + SKU: "V2katagen2", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1604-2021.11.06", + OS: "AKSUbuntu", + SKU: "1604", + Version: "2021.11.06", + }, + { + FullName: "AKSUbuntu-2204fipscontainerd-202404.09.0", + OS: "AKSUbuntu", + SKU: "2204fipscontainerd", + Version: "202404.09.0", + }, + { + FullName: "AKSUbuntu-2204minimalcontainerd-202401.12.0", + OS: "AKSUbuntu", + SKU: "2204minimalcontainerd", + Version: "202401.12.0", + }, + { + FullName: "AKSWindows-2022-containerd-20348.2762.241009", + OS: "AKSWindows", + SKU: "windows-2022-containerd", + Version: "20348.2762.241009", + }, + { + FullName: "AKSUbuntu-2204containerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "2204containerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-2004fipscontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "2004fipscontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSUbuntu-1804gen2fipscontainerd-202410.09.0", + OS: "AKSUbuntu", + SKU: "1804gen2fipscontainerd", + Version: "202410.09.0", + }, + { + FullName: "AKSWindows-23H2-25398.1189.241009", + OS: "AKSWindows", + SKU: "windows-23H2", + Version: "25398.1189.241009", + }, + { + FullName: "AKSAzureLinux-V2-202410.09.0", + OS: "AKSAzureLinux", + SKU: "V2", + Version: "202410.09.0", + }, + { + FullName: "AKSAzureLinux-V3gen2fips-202409.23.0", + OS: "AKSAzureLinux", + SKU: "V3gen2fips", + Version: "202409.23.0", + }, + { + FullName: "AKSUbuntu-2404gen2containerd-202405.20.0", + OS: "AKSUbuntu", + SKU: "2404gen2containerd", + Version: "202405.20.0", + }, + { + FullName: "AKSUbuntu-2204gen2containerd-2022.10.03", + OS: "AKSUbuntu", + SKU: "2204gen2containerd", + Version: "2022.10.03", + }, + { + FullName: "AKSUbuntu-2404containerd-202405.20.0", + OS: "AKSUbuntu", + SKU: "2404containerd", + Version: "202405.20.0", + }, + { + FullName: "AKSUbuntu-1804gen2-2022.08.29", + OS: "AKSUbuntu", + SKU: "1804gen2", + Version: "2022.08.29", + }, + }, + }, nil +} diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 0cf48ff27..c3adfa6b6 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -98,12 +98,15 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont azConfig.Location, operator.Elected(), ) + imageProvider := imagefamily.NewProvider( operator.KubernetesInterface, cache.New(azurecache.KubernetesVersionTTL, azurecache.DefaultCleanupInterval), azClient.ImageVersionsClient, azConfig.Location, + azConfig.SubscriptionID, + azClient.NodeImageVersionsClient, ) imageResolver := imagefamily.New( operator.GetClient(), diff --git a/pkg/operator/options/options.go b/pkg/operator/options/options.go index 62752e949..da0b7be66 100644 --- a/pkg/operator/options/options.go +++ b/pkg/operator/options/options.go @@ -100,7 +100,7 @@ func (o *Options) AddFlags(fs *coreoptions.FlagSet) { fs.StringVar(&o.ProvisionMode, "provision-mode", env.WithDefaultString("PROVISION_MODE", consts.ProvisionModeAKSScriptless), "[UNSUPPORTED] The provision mode for the cluster.") fs.StringVar(&o.NodeBootstrappingServerURL, "nodebootstrapping-server-url", env.WithDefaultString("NODEBOOTSTRAPPING_SERVER_URL", ""), "[UNSUPPORTED] The url for the node bootstrapping provider server.") fs.BoolVar(&o.ManagedKarpenter, "managed-karpenter", env.WithDefaultBool("MANAGED_KARPENTER", false), "Whether Karpenter is managed by AKS or not.") - fs.StringVar(&o.SharedImageGallerySubscriptionID, "shared-image-gallery-subscription-id", env.WithDefaultString("SHARED_IMAGE_GALLERY_SUBSCRIPTION_ID", ""), "The subscription ID of the shared image gallery. Only required alongside managed-karpenter.") + fs.StringVar(&o.SharedImageGallerySubscriptionID, "shared-image-gallery-subscription-id", env.WithDefaultString("SHARED_IMAGE_GALLERY_SUBSCRIPTION_ID", ""), "The subscription ID of the shared image gallery.") } func (o Options) GetAPIServerName() string { diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index 73afc10f0..10e8fd248 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -19,7 +19,6 @@ package imagefamily import ( "context" "fmt" - "regexp" "strings" "time" @@ -41,6 +40,8 @@ type Provider struct { kubernetesInterface kubernetes.Interface imageCache *cache.Cache imageVersionsClient CommunityGalleryImageVersionsAPI + subscription string + NodeImageVersions NodeImageVersionsAPI } const ( @@ -53,7 +54,7 @@ const ( communityImageIDFormat = "/CommunityGalleries/%s/images/%s/versions/%s" ) -func NewProvider(kubernetesInterface kubernetes.Interface, kubernetesVersionCache *cache.Cache, versionsClient CommunityGalleryImageVersionsAPI, location string) *Provider { +func NewProvider(kubernetesInterface kubernetes.Interface, kubernetesVersionCache *cache.Cache, versionsClient CommunityGalleryImageVersionsAPI, location, subscription string, nodeImageVersionsClient NodeImageVersionsAPI) *Provider { return &Provider{ kubernetesVersionCache: kubernetesVersionCache, imageCache: cache.New(imageExpirationInterval, imageCacheCleaningInterval), @@ -61,6 +62,8 @@ func NewProvider(kubernetesInterface kubernetes.Interface, kubernetesVersionCach imageVersionsClient: versionsClient, cm: pretty.NewChangeMonitor(), kubernetesInterface: kubernetesInterface, + subscription: subscription, + NodeImageVersions: nodeImageVersionsClient, } } @@ -165,37 +168,3 @@ func (p *Provider) latestNodeImageVersionCommmunity(publicGalleryURL, communityI func BuildImageIDCIG(publicGalleryURL, communityImageName, imageVersion string) string { return fmt.Sprintf(communityImageIDFormat, publicGalleryURL, communityImageName, imageVersion) } - -// ParseImageIDInfo parses the publicGalleryURL, communityImageName, and imageVersion out of an imageID -func ParseCommunityImageIDInfo(imageID string) (string, string, string, error) { - // TODO (charliedmcb): assess if doing validation on splitting the string and validating the results is better? Mostly is regex too expensive? - regexStr := fmt.Sprintf(communityImageIDFormat, "(?P.*)", "(?P.*)", "(?P.*)") - if imageID == "" { - return "", "", "", fmt.Errorf("can not parse empty string. Expect it of the form \"%s\"", regexStr) - } - r := regexp.MustCompile(regexStr) - matches := r.FindStringSubmatch(imageID) - if matches == nil { - return "", "", "", fmt.Errorf("no matches while parsing image id %s", imageID) - } - if r.SubexpIndex("publicGalleryURL") == -1 || r.SubexpIndex("communityImageName") == -1 || r.SubexpIndex("imageVersion") == -1 { - return "", "", "", fmt.Errorf("failed to find sub expressions in %s, for imageID: %s", regexStr, imageID) - } - return matches[r.SubexpIndex("publicGalleryURL")], matches[r.SubexpIndex("communityImageName")], matches[r.SubexpIndex("imageVersion")], nil -} - -type NodeImageVersion struct { - FullName string `json:"fullName"` - OS string `json:"os"` - SKU string `json:"sku"` - Version string `json:"version"` -} - -type NodeImageVersionsResponse struct { - Values []NodeImageVersion `json:"values"` -} - -func (p *Provider) ListNodeImageVersions(ctx context.Context) (NodeImageVersionsResponse, error) { - // call the Azure API to get the latest image versions - return NodeImageVersionsResponse{}, nil -} diff --git a/pkg/providers/imagefamily/image_test.go b/pkg/providers/imagefamily/image_test.go deleted file mode 100644 index 3a27c68a5..000000000 --- a/pkg/providers/imagefamily/image_test.go +++ /dev/null @@ -1,57 +0,0 @@ -/* -Portions Copyright (c) Microsoft Corporation. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package imagefamily_test - -import ( - "fmt" - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/Azure/karpenter-provider-azure/pkg/providers/imagefamily" -) - -func TestAzure(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Providers/ImageProvider/Azure") -} - -const ( - testImageID = "/CommunityGalleries/previewaks-1a06572d-8508-419c-a0d1-baffcbcb2f3b/images/2204gen2containerd/Versions/1.1685741267.25933" - olderImageVersion = "1.1686127203.20214" - latestImageVersion = "1.1686127203.20217" -) - -var _ = Describe("Image ID Parsing", func() { - DescribeTable("Parse Image ID", - func(imageID string, expectedPublicGalleryURL, expectedCommunityImageName, expectedImageVersion string, expectError bool) { - publicGalleryURL, communityImageName, imageVersion, err := imagefamily.ParseCommunityImageIDInfo(imageID) - if expectError { - Expect(err).To(HaveOccurred()) - return - } - Expect(err).To(BeNil()) - Expect(publicGalleryURL).To(Equal(expectedPublicGalleryURL)) - Expect(communityImageName).To(Equal(expectedCommunityImageName)) - Expect(imageVersion).To(Equal(expectedImageVersion)) - }, - Entry("Valid image id should parse", fmt.Sprintf("/CommunityGalleries/%s/images/%s/versions/%s", imagefamily.AKSUbuntuPublicGalleryURL, imagefamily.Ubuntu2204Gen2ImageDefinition, olderImageVersion), imagefamily.AKSUbuntuPublicGalleryURL, imagefamily.Ubuntu2204Gen2ImageDefinition, olderImageVersion, nil), - Entry("invalid image id should not parse", "badimageid", "", "", "", true), - Entry("empty image id should not parse", "badimageid", "", "", "", true), - ) -}) diff --git a/pkg/providers/imagefamily/nodeimageversionsclient.go b/pkg/providers/imagefamily/nodeimageversionsclient.go new file mode 100644 index 000000000..f836c4eed --- /dev/null +++ b/pkg/providers/imagefamily/nodeimageversionsclient.go @@ -0,0 +1,58 @@ +package imagefamily + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" +) + +type NodeImageVersionsClient struct { + cred azcore.TokenCredential +} + +func NewNodeImageVersionsClient(cred azcore.TokenCredential) *NodeImageVersionsClient { + return &NodeImageVersionsClient{ + cred: cred, + } +} + +func (l *NodeImageVersionsClient) List(ctx context.Context, location, subscription string) (NodeImageVersionsResponse, error) { + resourceURL := fmt.Sprintf( + "https://management.azure.com/subscriptions/%s/providers/Microsoft.ContainerService/locations/%s/nodeImageVersions?api-version=%s", + subscription, location, "2024-04-02-preview", + ) + + token, err := l.cred.GetToken(ctx, policy.TokenRequestOptions{ + Scopes: []string{"https://management.azure.com/.default"}, + }) + if err != nil { + panic(err) + } + + req, err := http.NewRequestWithContext(context.Background(), "GET", resourceURL, nil) + if err != nil { + panic(err) + } + + req.Header.Set("Authorization", "Bearer "+token.Token) + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + resp, err := client.Do(req) + if err != nil { + panic(err) + } + defer resp.Body.Close() + + var response NodeImageVersionsResponse + decoder := json.NewDecoder(resp.Body) + err = decoder.Decode(&response) + if err != nil { + panic(err) + } + return response, nil +} diff --git a/pkg/providers/imagefamily/types.go b/pkg/providers/imagefamily/types.go index 374e0e7e6..04df41ea8 100644 --- a/pkg/providers/imagefamily/types.go +++ b/pkg/providers/imagefamily/types.go @@ -17,6 +17,7 @@ limitations under the License. package imagefamily import ( + "context" "strings" "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" @@ -68,3 +69,18 @@ func (d *DefaultImageOutput) PopulateImageTraitsFromID(imageID string) { type CommunityGalleryImageVersionsAPI interface { NewListPager(location string, publicGalleryName string, galleryImageName string, options *armcomputev5.CommunityGalleryImageVersionsClientListOptions) *runtime.Pager[armcomputev5.CommunityGalleryImageVersionsClientListResponse] } + +type NodeImageVersion struct { + FullName string `json:"fullName"` + OS string `json:"os"` + SKU string `json:"sku"` + Version string `json:"version"` +} + +type NodeImageVersionsResponse struct { + Values []NodeImageVersion `json:"values"` +} + +type NodeImageVersionsAPI interface { + List(ctx context.Context, location, subscription string) (NodeImageVersionsResponse, error) +} diff --git a/pkg/providers/instance/azure_client.go b/pkg/providers/instance/azure_client.go index 62a41f99b..ff797d4d2 100644 --- a/pkg/providers/instance/azure_client.go +++ b/pkg/providers/instance/azure_client.go @@ -63,7 +63,8 @@ type AZClient struct { virtualMachinesExtensionClient VirtualMachineExtensionsAPI networkInterfacesClient NetworkInterfacesAPI - ImageVersionsClient imagefamily.CommunityGalleryImageVersionsAPI + NodeImageVersionsClient imagefamily.NodeImageVersionsAPI + ImageVersionsClient imagefamily.CommunityGalleryImageVersionsAPI // SKU CLIENT is still using track 1 because skewer does not support the track 2 path. We need to refactor this once skewer supports track 2 SKUClient skuclient.SkuClient LoadBalancersClient loadbalancer.LoadBalancersAPI @@ -76,6 +77,7 @@ func NewAZClientFromAPI( interfacesClient NetworkInterfacesAPI, loadBalancersClient loadbalancer.LoadBalancersAPI, imageVersionsClient imagefamily.CommunityGalleryImageVersionsAPI, + nodeImageVersionsClient imagefamily.NodeImageVersionsAPI, skuClient skuclient.SkuClient, ) *AZClient { return &AZClient{ @@ -84,6 +86,7 @@ func NewAZClientFromAPI( virtualMachinesExtensionClient: virtualMachinesExtensionClient, networkInterfacesClient: interfacesClient, ImageVersionsClient: imageVersionsClient, + NodeImageVersionsClient: nodeImageVersionsClient, SKUClient: skuClient, LoadBalancersClient: loadBalancersClient, } @@ -137,11 +140,13 @@ func NewAZClient(ctx context.Context, cfg *auth.Config, env *azure.Environment) } klog.V(5).Infof("Created azure resource graph client %v, using a token credential", azureResourceGraphClient) - imageVersionsClient, err := armcomputev5.NewCommunityGalleryImageVersionsClient(cfg.SubscriptionID, cred, opts) + communityImageVersionsClient, err := armcomputev5.NewCommunityGalleryImageVersionsClient(cfg.SubscriptionID, cred, opts) if err != nil { return nil, err } - klog.V(5).Infof("Created image versions client %v, using a token credential", imageVersionsClient) + klog.V(5).Infof("Created image versions client %v, using a token credential", communityImageVersionsClient) + + nodeImageVersionsClient := imagefamily.NewNodeImageVersionsClient(cred) loadBalancersClient, err := armnetwork.NewLoadBalancersClient(cfg.SubscriptionID, cred, opts) if err != nil { @@ -158,6 +163,7 @@ func NewAZClient(ctx context.Context, cfg *auth.Config, env *azure.Environment) extensionsClient, interfacesClient, loadBalancersClient, - imageVersionsClient, + communityImageVersionsClient, + nodeImageVersionsClient, skuClient), nil } diff --git a/pkg/test/environment.go b/pkg/test/environment.go index 30b6777bf..1b3522c26 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -46,6 +46,7 @@ func init() { var ( resourceGroup = "test-resourceGroup" + subscription = "12345678-1234-1234-1234-123456789012" ) type Environment struct { @@ -98,6 +99,7 @@ func NewRegionalEnvironment(ctx context.Context, env *coretest.Environment, regi skuClientSingleton := &fake.MockSkuClientSingleton{SKUClient: &fake.ResourceSKUsAPI{Location: region}} communityImageVersionsAPI := &fake.CommunityGalleryImageVersionsAPI{} loadBalancersAPI := &fake.LoadBalancersAPI{} + nodeImageVersionsAPI := &fake.NodeImageVersionsAPI{} // Cache kubernetesVersionCache := cache.New(azurecache.KubernetesVersionTTL, azurecache.DefaultCleanupInterval) @@ -107,7 +109,7 @@ func NewRegionalEnvironment(ctx context.Context, env *coretest.Environment, regi // Providers pricingProvider := pricing.NewProvider(ctx, pricingAPI, region, make(chan struct{})) - imageFamilyProvider := imagefamily.NewProvider(env.KubernetesInterface, kubernetesVersionCache, communityImageVersionsAPI, region) + imageFamilyProvider := imagefamily.NewProvider(env.KubernetesInterface, kubernetesVersionCache, communityImageVersionsAPI, region, subscription, nodeImageVersionsAPI) imageFamilyResolver := imagefamily.New(env.Client, imageFamilyProvider) instanceTypesProvider := instancetype.NewDefaultProvider(region, instanceTypeCache, skuClientSingleton, pricingProvider, unavailableOfferingsCache) launchTemplateProvider := launchtemplate.NewProvider( @@ -117,7 +119,7 @@ func NewRegionalEnvironment(ctx context.Context, env *coretest.Environment, regi ptr.String("ca-bundle"), testOptions.ClusterEndpoint, "test-tenant", - "test-subscription", + subscription, "test-cluster-resource-group", "test-kubelet-identity-client-id", testOptions.NodeResourceGroup, @@ -137,6 +139,7 @@ func NewRegionalEnvironment(ctx context.Context, env *coretest.Environment, regi networkInterfacesAPI, loadBalancersAPI, communityImageVersionsAPI, + nodeImageVersionsAPI, skuClientSingleton, ) instanceProvider := instance.NewDefaultProvider( @@ -147,7 +150,7 @@ func NewRegionalEnvironment(ctx context.Context, env *coretest.Environment, regi unavailableOfferingsCache, region, testOptions.NodeResourceGroup, - "", // subscriptionID + subscription, testOptions.ProvisionMode, ) From c79ecfdfa565a35663d380aa4092ee5bed89b233 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 18:56:34 -0700 Subject: [PATCH 06/22] refactor: rename symbol for SIG Subscription id --- pkg/operator/options/options.go | 4 ++-- pkg/providers/imagefamily/image.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/operator/options/options.go b/pkg/operator/options/options.go index da0b7be66..18023d723 100644 --- a/pkg/operator/options/options.go +++ b/pkg/operator/options/options.go @@ -79,7 +79,7 @@ type Options struct { NodeBootstrappingServerURL string ManagedKarpenter bool // => ManagedKarpenter is true if Karpenter is managed by AKS, false if it is a self-hosted karpenter installation - SharedImageGallerySubscriptionID string + SIGSubscriptionID string NodeResourceGroup string } @@ -100,7 +100,7 @@ func (o *Options) AddFlags(fs *coreoptions.FlagSet) { fs.StringVar(&o.ProvisionMode, "provision-mode", env.WithDefaultString("PROVISION_MODE", consts.ProvisionModeAKSScriptless), "[UNSUPPORTED] The provision mode for the cluster.") fs.StringVar(&o.NodeBootstrappingServerURL, "nodebootstrapping-server-url", env.WithDefaultString("NODEBOOTSTRAPPING_SERVER_URL", ""), "[UNSUPPORTED] The url for the node bootstrapping provider server.") fs.BoolVar(&o.ManagedKarpenter, "managed-karpenter", env.WithDefaultBool("MANAGED_KARPENTER", false), "Whether Karpenter is managed by AKS or not.") - fs.StringVar(&o.SharedImageGallerySubscriptionID, "shared-image-gallery-subscription-id", env.WithDefaultString("SHARED_IMAGE_GALLERY_SUBSCRIPTION_ID", ""), "The subscription ID of the shared image gallery.") + fs.StringVar(&o.SIGSubscriptionID, "sig-subscription-id", env.WithDefaultString("SIG_SUBSCRIPTION_ID", ""), "The subscription ID of the shared image gallery.") } func (o Options) GetAPIServerName() string { diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index 10e8fd248..596b451f6 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -114,12 +114,12 @@ func (p *Provider) getImageIDSIG(ctx context.Context, imgStub DefaultImageOutput if imageID, ok := p.imageCache.Get(key); ok { return imageID.(string), nil } - versions, err := p.ListNodeImageVersions(ctx) + versions, err := p.NodeImageVersions.List(ctx, p.location, p.subscription) if err != nil { return "", err } for _, version := range versions.Values { - imageID := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SharedImageGallerySubscriptionID, imgStub.GalleryResourceGroup, imgStub.GalleryName, imgStub.ImageDefinition, version.Version) + imageID := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SIGSubscriptionID, imgStub.GalleryResourceGroup, imgStub.GalleryName, imgStub.ImageDefinition, version.Version) p.imageCache.Set(key, imageID, imageExpirationInterval) } // return the latest version of the image from the cache after we have caached all of the imageDefinitions From f19c7190ab5c883b0e84adaee82385ed1d52fa28 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 20:11:46 -0700 Subject: [PATCH 07/22] test: conditional use of sig dependent on the managed karpenter flag --- pkg/providers/instancetype/suite_test.go | 37 ++++++++++++++++++++++++ pkg/test/environment.go | 2 +- pkg/test/options.go | 6 ++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index 876b982cb..bbf841e41 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -884,6 +884,43 @@ var _ = Describe("InstanceType Provider", func() { }) }) + Context("ImageReference", func() { + It("should use shared image gallery images when options are set to ManagedKarpenter", func() { + options := test.Options(test.OptionsFields{ + ManagedKarpenter: lo.ToPtr(true), + }) + ctx = options.ToContext(ctx) + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + pod := coretest.UnschedulablePod(coretest.PodOptions{}) + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, coreProvisioner, pod) + ExpectScheduled(ctx, env.Client, pod) + + // Expect virtual machine to have a shared image gallery id set on it + Expect(azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len()).To(Equal(1)) + vm := azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM + Expect(vm.Properties.StorageProfile.ImageReference).ToNot(BeNil()) + Expect(vm.Properties.StorageProfile.ImageReference.ID).ShouldNot(BeNil()) + Expect(vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID).Should(BeNil()) + + Expect(*vm.Properties.StorageProfile.ImageReference.ID).To(ContainSubstring(options.SIGSubscriptionID)) + Expect(*vm.Properties.StorageProfile.ImageReference.ID).To(ContainSubstring("AKSUbuntu")) + }) + It("should use Community Images when options are set to ManagedKarpenter=false", func() { + options := test.Options(test.OptionsFields{ + ManagedKarpenter: lo.ToPtr(false), + }) + ctx = options.ToContext(ctx) + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + pod := coretest.UnschedulablePod(coretest.PodOptions{}) + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, coreProvisioner, pod) + ExpectScheduled(ctx, env.Client, pod) + + Expect(azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len()).To(Equal(1)) + vm := azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM + Expect(vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID).Should(Not(BeNil())) + }) + + }) Context("ImageProvider + Image Family", func() { DescribeTable("should select the right image for a given instance type", func(instanceType string, imageFamily string, expectedImageDefinition string, expectedGalleryURL string) { diff --git a/pkg/test/environment.go b/pkg/test/environment.go index 1b3522c26..11d3faf0e 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -46,7 +46,7 @@ func init() { var ( resourceGroup = "test-resourceGroup" - subscription = "12345678-1234-1234-1234-123456789012" + subscription = "12345678-1234-1234-1234-123456789012" ) type Environment struct { diff --git a/pkg/test/options.go b/pkg/test/options.go index 32e2b2734..9372ac5ea 100644 --- a/pkg/test/options.go +++ b/pkg/test/options.go @@ -41,6 +41,10 @@ type OptionsFields struct { NodeResourceGroup *string ProvisionMode *string NodeBootstrappingServerURL *string + + // ManagedKarpenter Flags not required by the self hosted offering + ManagedKarpenter *bool + SIGSubscriptionID *string } func Options(overrides ...OptionsFields) *azoptions.Options { @@ -65,5 +69,7 @@ func Options(overrides ...OptionsFields) *azoptions.Options { SubnetID: lo.FromPtrOr(options.SubnetID, "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/sillygeese/providers/Microsoft.Network/virtualNetworks/karpentervnet/subnets/karpentersub"), NodeResourceGroup: lo.FromPtrOr(options.NodeResourceGroup, "test-resourceGroup"), ProvisionMode: lo.FromPtrOr(options.ProvisionMode, "aksscriptless"), + ManagedKarpenter: lo.FromPtrOr(options.ManagedKarpenter, false), + SIGSubscriptionID: lo.FromPtrOr(options.SIGSubscriptionID, "10945678-1234-1234-1234-123456789012"), } } From 107b41f6e8dacb09290f0c3007ded8e52e647505 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 20:14:42 -0700 Subject: [PATCH 08/22] refactor: removing panics used in testing --- pkg/providers/imagefamily/nodeimageversionsclient.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/providers/imagefamily/nodeimageversionsclient.go b/pkg/providers/imagefamily/nodeimageversionsclient.go index f836c4eed..84049ccde 100644 --- a/pkg/providers/imagefamily/nodeimageversionsclient.go +++ b/pkg/providers/imagefamily/nodeimageversionsclient.go @@ -30,12 +30,12 @@ func (l *NodeImageVersionsClient) List(ctx context.Context, location, subscripti Scopes: []string{"https://management.azure.com/.default"}, }) if err != nil { - panic(err) + return NodeImageVersionsResponse{}, err } req, err := http.NewRequestWithContext(context.Background(), "GET", resourceURL, nil) if err != nil { - panic(err) + return NodeImageVersionsResponse{}, err } req.Header.Set("Authorization", "Bearer "+token.Token) @@ -44,7 +44,7 @@ func (l *NodeImageVersionsClient) List(ctx context.Context, location, subscripti client := &http.Client{} resp, err := client.Do(req) if err != nil { - panic(err) + return NodeImageVersionsResponse{}, err } defer resp.Body.Close() @@ -52,7 +52,7 @@ func (l *NodeImageVersionsClient) List(ctx context.Context, location, subscripti decoder := json.NewDecoder(resp.Body) err = decoder.Decode(&response) if err != nil { - panic(err) + return NodeImageVersionsResponse{}, err } return response, nil } From 9c179cc69aca6dc4275ed782114bfed1d9e1735d Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 22:47:05 -0700 Subject: [PATCH 09/22] test: adding RBAC and helm values to the template for SIG Gallery logic --- Makefile-az.mk | 6 ++++++ karpenter-values-template.yaml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/Makefile-az.mk b/Makefile-az.mk index 9feaed157..0f0f6305e 100755 --- a/Makefile-az.mk +++ b/Makefile-az.mk @@ -8,6 +8,7 @@ else AZURE_ACR_NAME ?= $(COMMON_NAME) endif +AZURE_SIG_SUBSCRIPTION_ID ?= $(AZURE_SUBSCRIPTION_ID) AZURE_CLUSTER_NAME ?= $(COMMON_NAME) AZURE_RESOURCE_GROUP_MC = MC_$(AZURE_RESOURCE_GROUP)_$(AZURE_CLUSTER_NAME)_$(AZURE_LOCATION) @@ -136,6 +137,11 @@ az-perm: ## Create role assignments to let Karpenter manage VMs and Network az role assignment create --assignee $(KARPENTER_USER_ASSIGNED_CLIENT_ID) --scope /subscriptions/$(AZURE_SUBSCRIPTION_ID)/resourceGroups/$(AZURE_RESOURCE_GROUP) --role "Network Contributor" # in some case we create vnet here @echo Consider "make az-configure-values"! +az-perm-sig: ## Create role assignments when testing with SIG images + $(eval KARPENTER_USER_ASSIGNED_CLIENT_ID=$(shell az identity show --resource-group "${AZURE_RESOURCE_GROUP}" --name "${AZURE_KARPENTER_USER_ASSIGNED_IDENTITY_NAME}" --query 'principalId' -otsv)) + az role assignment create --assignee $(KARPENTER_USER_ASSIGNED_CLIENT_ID) --role "Reader" --scope /subscriptions/$(AZURE_SIG_SUBSCRIPTION_ID)/resourceGroups/AKS-Ubuntu/providers/Microsoft.Compute/galleries/AKSUbuntu + az role assignment create --assignee $(KARPENTER_USER_ASSIGNED_CLIENT_ID) --role "Reader" --scope /subscriptions/$(AZURE_SIG_SUBSCRIPTION_ID)/resourceGroups/AKS-AzureLinux/providers/Microsoft.Compute/galleries/AKSAzureLinux + az-perm-subnet-custom: az-perm ## Create role assignments to let Karpenter manage VMs and Network (custom VNet) $(eval VNET_SUBNET_ID=$(shell az aks show --name $(AZURE_CLUSTER_NAME) --resource-group $(AZURE_RESOURCE_GROUP) | jq -r ".agentPoolProfiles[0].vnetSubnetId")) $(eval KARPENTER_USER_ASSIGNED_CLIENT_ID=$(shell az identity show --resource-group "${AZURE_RESOURCE_GROUP}" --name "${AZURE_KARPENTER_USER_ASSIGNED_IDENTITY_NAME}" --query 'principalId' -otsv)) diff --git a/karpenter-values-template.yaml b/karpenter-values-template.yaml index 01473e98c..0ca86a42c 100644 --- a/karpenter-values-template.yaml +++ b/karpenter-values-template.yaml @@ -37,6 +37,12 @@ controller: value: "" - name: AZURE_NODE_RESOURCE_GROUP value: ${AZURE_RESOURCE_GROUP_MC} + + # managed karpenter settings + - name: MANAGED_KARPENTER + value: "false" + - name: SIG_SUBSCRIPTION_ID + value: ${SIG_SUBSCRIPTION_ID} serviceAccount: name: ${KARPENTER_SERVICE_ACCOUNT_NAME} annotations: From 3f7f761bf1e78af270b39e11863c8326d236b813 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 22:58:19 -0700 Subject: [PATCH 10/22] fix: bug in azure linux sig image resolution --- pkg/providers/imagefamily/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/providers/imagefamily/types.go b/pkg/providers/imagefamily/types.go index 04df41ea8..9c7ee9d74 100644 --- a/pkg/providers/imagefamily/types.go +++ b/pkg/providers/imagefamily/types.go @@ -30,7 +30,7 @@ const ( AKSAzureLinuxPublicGalleryURL = "AKSAzureLinux-f7c7cda5-1c9a-4bdc-a222-9614c968580b" AKSUbuntuResourceGroup = "AKS-Ubuntu" - AKSAzureLinuxResourceGroup = "AKS-Azure-Linux" + AKSAzureLinuxResourceGroup = "AKS-AzureLinux" AKSWindowsResourceGroup = "AKS-Windows" AKSUbuntuGalleryName = "AKSUbuntu" From 0efac0d57ca9c49aaf5a173754b1e4390d4cdc26 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 23:04:12 -0700 Subject: [PATCH 11/22] chore: update cleanupenv to handle inflate too ratehr than just job pods --- Makefile-az.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile-az.mk b/Makefile-az.mk index 0f0f6305e..990e71aea 100755 --- a/Makefile-az.mk +++ b/Makefile-az.mk @@ -48,6 +48,7 @@ az-acrimport: ## Imports an image to an acr registry az acr import --name $(AZURE_ACR_NAME) --source "mcr.microsoft.com/oss/kubernetes/pause:3.6" --image "pause:3.6" az-cleanenv: az-rmnodeclaims-fin ## Deletes a few common karpenter testing resources(pods, nodepools, nodeclaims, aksnodeclasses) + kubectl delete deployments -n default --all kubectl delete pods -n default --all kubectl delete nodeclaims --all kubectl delete nodepools --all From 637e30e2df58a4fce20b8bc38c3003fd22e82935 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 23:11:47 -0700 Subject: [PATCH 12/22] test: fix randomized test order flake --- pkg/providers/instancetype/suite_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index bbf841e41..15ed51a21 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -904,6 +904,9 @@ var _ = Describe("InstanceType Provider", func() { Expect(*vm.Properties.StorageProfile.ImageReference.ID).To(ContainSubstring(options.SIGSubscriptionID)) Expect(*vm.Properties.StorageProfile.ImageReference.ID).To(ContainSubstring("AKSUbuntu")) + + cluster.Reset() + azureEnv.Reset() }) It("should use Community Images when options are set to ManagedKarpenter=false", func() { options := test.Options(test.OptionsFields{ @@ -918,6 +921,9 @@ var _ = Describe("InstanceType Provider", func() { Expect(azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len()).To(Equal(1)) vm := azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM Expect(vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID).Should(Not(BeNil())) + + cluster.Reset() + azureEnv.Reset() }) }) From 0d7a98a84b53620309cb91f3d426eda0802a1a41 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 23:35:44 -0700 Subject: [PATCH 13/22] ci: shadow declaration --- pkg/providers/imagefamily/image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index 596b451f6..75e68633e 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -110,7 +110,7 @@ func (p *Provider) KubeServerVersion(ctx context.Context) (string, error) { // If the imageVersion is set to "", then we will cache the latest version of the image for imageExpirationInterval days(3d) for all imageDefinitions // and reuse that get of the latest version of the image for 3d. func (p *Provider) getImageIDSIG(ctx context.Context, imgStub DefaultImageOutput) (string, error) { - key := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SharedImageGallerySubscriptionID, imgStub.GalleryResourceGroup, imgStub.GalleryName, imgStub.ImageDefinition) + key := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SIGSubscriptionID, imgStub.GalleryResourceGroup, imgStub.GalleryName, imgStub.ImageDefinition) if imageID, ok := p.imageCache.Get(key); ok { return imageID.(string), nil } From 13b410ddf6cbf9ce8bd0bb32f9177149f18c1c85 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Tue, 15 Oct 2024 23:39:59 -0700 Subject: [PATCH 14/22] refactor: comment wording --- pkg/providers/imagefamily/image.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index 75e68633e..0c4c591e9 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -105,10 +105,6 @@ func (p *Provider) KubeServerVersion(ctx context.Context) (string, error) { return version, nil } -// getImageIDSig will return a string of the shape /subscriptions/{subscriptionID}/resourceGroups/{resourceGroup}/providers/Microsoft.Compute/galleries/{galleryName}/images/{imageDefinition}/versions/{imageVersion} -// for a given imageDefinition and imageVersion. If the imageVersion is set to "", then it will return the latest version of the image. -// If the imageVersion is set to "", then we will cache the latest version of the image for imageExpirationInterval days(3d) for all imageDefinitions -// and reuse that get of the latest version of the image for 3d. func (p *Provider) getImageIDSIG(ctx context.Context, imgStub DefaultImageOutput) (string, error) { key := fmt.Sprintf(sharedImageGalleryImageIDFormat, options.FromContext(ctx).SIGSubscriptionID, imgStub.GalleryResourceGroup, imgStub.GalleryName, imgStub.ImageDefinition) if imageID, ok := p.imageCache.Get(key); ok { @@ -129,10 +125,6 @@ func (p *Provider) getImageIDSIG(ctx context.Context, imgStub DefaultImageOutput return "", fmt.Errorf("failed to get the latest version of the image %s", imgStub.ImageDefinition) } -// getImageCIG will return a community image gallery image url that has the shape of -// /CommunityGalleries/{publicGalleryURL}/images/{communityImageName}/versions/{imageVersion} -// The imageVersion can be set to "" to get the latest version of the image, and after a image version "" has been encountered -// we cache the latest version of the image for imageExpirationInterval days(3d) func (p *Provider) getImageIDCIG(ctx context.Context, publicGalleryURL, communityImageName string) (string, error) { key := fmt.Sprintf(communityImageIDFormat, publicGalleryURL, communityImageName) if imageID, ok := p.imageCache.Get(key); ok { From 05e4ac966bdb985295ba365a35f7efdce0128878 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Wed, 16 Oct 2024 11:06:11 -0700 Subject: [PATCH 15/22] test: validate all image ids are resolved correctly --- pkg/fake/communityimageversionsapi.go | 13 ++++++++ pkg/providers/imagefamily/image.go | 6 ++-- pkg/providers/imagefamily/types.go | 1 + pkg/providers/instancetype/suite_test.go | 39 ++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/pkg/fake/communityimageversionsapi.go b/pkg/fake/communityimageversionsapi.go index 6e0c381df..c53cfac54 100644 --- a/pkg/fake/communityimageversionsapi.go +++ b/pkg/fake/communityimageversionsapi.go @@ -18,9 +18,11 @@ package fake import ( "context" + "fmt" "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" + "github.com/samber/lo" "github.com/Azure/karpenter-provider-azure/pkg/providers/imagefamily" ) @@ -51,6 +53,17 @@ func (c *CommunityGalleryImageVersionsAPI) NewListPager(_ string, _ string, _ st return runtime.NewPager(pagingHandler) } +func (c *CommunityGalleryImageVersionsAPI) Get(_ context.Context, location string, publicGalleryName string, galleryImageName string, galleryImageVersionName string, options *armcompute.CommunityGalleryImageVersionsClientGetOptions) (armcompute.CommunityGalleryImageVersionsClientGetResponse, error) { + // TODO: Add case where this get doesn't work or succeed + return armcompute.CommunityGalleryImageVersionsClientGetResponse{ + CommunityGalleryImageVersion: armcompute.CommunityGalleryImageVersion{ + Identifier: &armcompute.CommunityGalleryIdentifier{ + UniqueID: lo.ToPtr(fmt.Sprintf("/CommunityGalleries/%s/images/%s/versions/%s", publicGalleryName, galleryImageName, galleryImageVersionName)), + }, + }, + }, nil +} + func (c *CommunityGalleryImageVersionsAPI) Reset() { if c == nil { return diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index 0c4c591e9..1b9915c3c 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -72,8 +72,8 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1alpha2.AKSNodeClass, in defaultImages := imageFamily.DefaultImages() for _, defaultImage := range defaultImages { if err := instanceType.Requirements.Compatible(defaultImage.Requirements, v1alpha2.AllowUndefinedWellKnownAndRestrictedLabels); err == nil { - imageID, imageRetrievalErr := p.GetLatestImageID(ctx, defaultImage) - return defaultImage.Distro, imageID, imageRetrievalErr + imageID, imageRetrievalErr := p.GetLatestImageID(ctx, defaultImage) + return defaultImage.Distro, imageID, imageRetrievalErr } } @@ -130,7 +130,7 @@ func (p *Provider) getImageIDCIG(ctx context.Context, publicGalleryURL, communit if imageID, ok := p.imageCache.Get(key); ok { return imageID.(string), nil } - // if the image is not found in the cache, we will refresh the lookup for it + // if the image is not found in the cache, we will refresh the lookup for it imageVersion, err := p.latestNodeImageVersionCommmunity(publicGalleryURL, communityImageName) if err != nil { return "", err diff --git a/pkg/providers/imagefamily/types.go b/pkg/providers/imagefamily/types.go index 9c7ee9d74..fa2110e5f 100644 --- a/pkg/providers/imagefamily/types.go +++ b/pkg/providers/imagefamily/types.go @@ -68,6 +68,7 @@ func (d *DefaultImageOutput) PopulateImageTraitsFromID(imageID string) { // CommunityGalleryImageVersionsAPI is used for listing community gallery image versions. type CommunityGalleryImageVersionsAPI interface { NewListPager(location string, publicGalleryName string, galleryImageName string, options *armcomputev5.CommunityGalleryImageVersionsClientListOptions) *runtime.Pager[armcomputev5.CommunityGalleryImageVersionsClientListResponse] + Get(ctx context.Context, location string, publicGalleryName string, galleryImageName string, galleryImageVersionName string, options *armcomputev5.CommunityGalleryImageVersionsClientGetOptions) (armcomputev5.CommunityGalleryImageVersionsClientGetResponse, error) } type NodeImageVersion struct { diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index 15ed51a21..a14e63832 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -928,6 +928,45 @@ var _ = Describe("InstanceType Provider", func() { }) Context("ImageProvider + Image Family", func() { + DescribeTable("should select the right Shared Image Gallery image for a given instance type", func(instanceType string, imageFamily string, expectedImageDefinition string, expectedGalleryRG string, expectedGalleryURL string) { + options := test.Options(test.OptionsFields{ + ManagedKarpenter: lo.ToPtr(true), + }) + ctx = options.ToContext(ctx) + nodeClass.Spec.ImageFamily = lo.ToPtr(imageFamily) + coretest.ReplaceRequirements(nodePool, corev1beta1.NodeSelectorRequirementWithMinValues{ + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1.LabelInstanceTypeStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{instanceType}, + }}) + nodePool.Spec.Template.Spec.NodeClassRef = &corev1beta1.NodeClassReference{Name: nodeClass.Name} + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + pod := coretest.UnschedulablePod(coretest.PodOptions{}) + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, coreProvisioner, pod) + ExpectScheduled(ctx, env.Client, pod) + + Expect(azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len()).To(Equal(1)) + vm := azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM + Expect(vm.Properties.StorageProfile.ImageReference).ToNot(BeNil()) + + expectedPrefix := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/%s/images/%s", options.SIGSubscriptionID, expectedGalleryRG, expectedGalleryURL, expectedImageDefinition) + Expect(*vm.Properties.StorageProfile.ImageReference.ID).To(ContainSubstring(expectedPrefix)) + + // reset options back to the default + options = test.Options(test.OptionsFields{ + ManagedKarpenter: lo.ToPtr(false), + }) + ctx = options.ToContext(ctx) + }, + + Entry("Gen2, Gen1 instance type with AKSUbuntu image family", "Standard_D2_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2ImageDefinition, imagefamily.AKSUbuntuResourceGroup, imagefamily.AKSUbuntuGalleryName), + Entry("Gen1 instance type with AKSUbuntu image family", "Standard_D2_v3", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen1ImageDefinition, imagefamily.AKSUbuntuResourceGroup, imagefamily.AKSUbuntuGalleryName), + Entry("ARM instance type with AKSUbuntu image family", "Standard_D16plds_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2ArmImageDefinition, imagefamily.AKSUbuntuResourceGroup, imagefamily.AKSUbuntuGalleryName), + Entry("Gen2 instance type with AzureLinux image family", "Standard_D2_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2ImageDefinition, imagefamily.AKSAzureLinuxResourceGroup, imagefamily.AKSAzureLinuxGalleryName), + Entry("Gen1 instance type with AzureLinux image family", "Standard_D2_v3", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen1ImageDefinition, imagefamily.AKSAzureLinuxResourceGroup, imagefamily.AKSAzureLinuxGalleryName), + Entry("ARM instance type with AzureLinux image family", "Standard_D16plds_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2ArmImageDefinition, imagefamily.AKSAzureLinuxResourceGroup, imagefamily.AKSAzureLinuxGalleryName), + ) DescribeTable("should select the right image for a given instance type", func(instanceType string, imageFamily string, expectedImageDefinition string, expectedGalleryURL string) { nodeClass.Spec.ImageFamily = lo.ToPtr(imageFamily) From 283484d1035c94342ff3e880d3bd4192a5a41e95 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Wed, 16 Oct 2024 13:03:55 -0700 Subject: [PATCH 16/22] fix: adding filtering for duplicate sku + os combinations and filtering out unsupported galleries --- .../imagefamily/nodeimageversionsclient.go | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/pkg/providers/imagefamily/nodeimageversionsclient.go b/pkg/providers/imagefamily/nodeimageversionsclient.go index 84049ccde..59963a905 100644 --- a/pkg/providers/imagefamily/nodeimageversionsclient.go +++ b/pkg/providers/imagefamily/nodeimageversionsclient.go @@ -54,5 +54,53 @@ func (l *NodeImageVersionsClient) List(ctx context.Context, location, subscripti if err != nil { return NodeImageVersionsResponse{}, err } + + response.Values = FilteredNodeImages(response.Values) return response, nil } + +// FilteredNodeImages filters on two conditions +// 1. The image is the latest version for the given OS and SKU +// 2. the image belongs to a supported gallery(AKS Ubuntu or Azure Linux) +func FilteredNodeImages(nodeImageVersions []NodeImageVersion) []NodeImageVersion { + latestImages := make(map[string]NodeImageVersion) + + for _, image := range nodeImageVersions { + // Skip the galleries that Karpenter does not support + if image.OS != AKSUbuntuGalleryName && image.OS != AKSAzureLinuxGalleryName { + continue + } + + key := image.OS + "-" + image.SKU + + currentLatest, exists := latestImages[key] + if !exists || isNewerVersion(image.Version, currentLatest.Version) { + latestImages[key] = image + } + } + + var filteredImages []NodeImageVersion + for _, image := range latestImages { + filteredImages = append(filteredImages, image) + } + return filteredImages +} + +func isNewerVersion(version1, version2 string) bool { + // Assuming version is in the format: "year.month.day.build" + // Split by dots and compare each segment as an integer + + var v1, v2 [4]int + fmt.Sscanf(version1, "%d.%d.%d.%d", &v1[0], &v1[1], &v1[2], &v1[3]) + fmt.Sscanf(version2, "%d.%d.%d.%d", &v2[0], &v2[1], &v2[2], &v2[3]) + + for i := 0; i < 4; i++ { + if v1[i] > v2[i] { + return true + } else if v1[i] < v2[i] { + return false + } + } + + return false +} From 227f7d6a3be4b22ecf4110b8b00965336942e6d5 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Wed, 30 Oct 2024 14:11:14 -0700 Subject: [PATCH 17/22] refactor: renaming var --- karpenter-values-template.yaml | 2 +- pkg/operator/options/options.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/karpenter-values-template.yaml b/karpenter-values-template.yaml index 0ca86a42c..6a808dd56 100644 --- a/karpenter-values-template.yaml +++ b/karpenter-values-template.yaml @@ -39,7 +39,7 @@ controller: value: ${AZURE_RESOURCE_GROUP_MC} # managed karpenter settings - - name: MANAGED_KARPENTER + - name: USE_SIG value: "false" - name: SIG_SUBSCRIPTION_ID value: ${SIG_SUBSCRIPTION_ID} diff --git a/pkg/operator/options/options.go b/pkg/operator/options/options.go index 18023d723..59cc249b5 100644 --- a/pkg/operator/options/options.go +++ b/pkg/operator/options/options.go @@ -99,7 +99,7 @@ func (o *Options) AddFlags(fs *coreoptions.FlagSet) { fs.StringVar(&o.NodeResourceGroup, "node-resource-group", env.WithDefaultString("AZURE_NODE_RESOURCE_GROUP", ""), "[REQUIRED] the resource group created and managed by AKS where the nodes live.") fs.StringVar(&o.ProvisionMode, "provision-mode", env.WithDefaultString("PROVISION_MODE", consts.ProvisionModeAKSScriptless), "[UNSUPPORTED] The provision mode for the cluster.") fs.StringVar(&o.NodeBootstrappingServerURL, "nodebootstrapping-server-url", env.WithDefaultString("NODEBOOTSTRAPPING_SERVER_URL", ""), "[UNSUPPORTED] The url for the node bootstrapping provider server.") - fs.BoolVar(&o.ManagedKarpenter, "managed-karpenter", env.WithDefaultBool("MANAGED_KARPENTER", false), "Whether Karpenter is managed by AKS or not.") + fs.BoolVar(&o.ManagedKarpenter, "use-sig", env.WithDefaultBool("USE_SIG", false), "If set to true karpenter will use the AKS managed shared image galleries and the node image versions api. If set to false karpenter will use community image galleries. Only a subset of image features will be available in the community image galleries and this flag is only for the managed node provisioning addon.") fs.StringVar(&o.SIGSubscriptionID, "sig-subscription-id", env.WithDefaultString("SIG_SUBSCRIPTION_ID", ""), "The subscription ID of the shared image gallery.") } From 340f48110add14051356e505ec7541e38121677d Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Thu, 14 Nov 2024 12:43:01 -0800 Subject: [PATCH 18/22] refactor: rename the managedKarpenter reference to UseSIG --- pkg/providers/instance/instance.go | 2 +- pkg/providers/instancetype/suite_test.go | 12 ++++++------ pkg/test/options.go | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index bf89b081e..158f20fee 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -404,7 +404,7 @@ func setVMPropertiesOSDiskType(vmProperties *armcompute.VirtualMachineProperties // setImageReference sets the image reference for the VM based on if we are using self hosted karpenter or the node auto provisioning addon func setImageReference(ctx context.Context, vmProperties *armcompute.VirtualMachineProperties, imageID string) { - if options.FromContext(ctx).ManagedKarpenter { + if options.FromContext(ctx).UseSIG { vmProperties.StorageProfile.ImageReference = &armcompute.ImageReference{ ID: lo.ToPtr(imageID), } diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index a14e63832..1f07dc357 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -885,9 +885,9 @@ var _ = Describe("InstanceType Provider", func() { }) Context("ImageReference", func() { - It("should use shared image gallery images when options are set to ManagedKarpenter", func() { + It("should use shared image gallery images when options are set to UseSIG", func() { options := test.Options(test.OptionsFields{ - ManagedKarpenter: lo.ToPtr(true), + UseSIG: lo.ToPtr(true), }) ctx = options.ToContext(ctx) ExpectApplied(ctx, env.Client, nodePool, nodeClass) @@ -908,9 +908,9 @@ var _ = Describe("InstanceType Provider", func() { cluster.Reset() azureEnv.Reset() }) - It("should use Community Images when options are set to ManagedKarpenter=false", func() { + It("should use Community Images when options are set to UseSIG=false", func() { options := test.Options(test.OptionsFields{ - ManagedKarpenter: lo.ToPtr(false), + UseSIG: lo.ToPtr(false), }) ctx = options.ToContext(ctx) ExpectApplied(ctx, env.Client, nodePool, nodeClass) @@ -930,7 +930,7 @@ var _ = Describe("InstanceType Provider", func() { Context("ImageProvider + Image Family", func() { DescribeTable("should select the right Shared Image Gallery image for a given instance type", func(instanceType string, imageFamily string, expectedImageDefinition string, expectedGalleryRG string, expectedGalleryURL string) { options := test.Options(test.OptionsFields{ - ManagedKarpenter: lo.ToPtr(true), + UseSIG: lo.ToPtr(true), }) ctx = options.ToContext(ctx) nodeClass.Spec.ImageFamily = lo.ToPtr(imageFamily) @@ -955,7 +955,7 @@ var _ = Describe("InstanceType Provider", func() { // reset options back to the default options = test.Options(test.OptionsFields{ - ManagedKarpenter: lo.ToPtr(false), + UseSIG: lo.ToPtr(false), }) ctx = options.ToContext(ctx) }, diff --git a/pkg/test/options.go b/pkg/test/options.go index 9372ac5ea..4cdbbb3a4 100644 --- a/pkg/test/options.go +++ b/pkg/test/options.go @@ -42,8 +42,8 @@ type OptionsFields struct { ProvisionMode *string NodeBootstrappingServerURL *string - // ManagedKarpenter Flags not required by the self hosted offering - ManagedKarpenter *bool + // UseSIG Flags not required by the self hosted offering + UseSIG *bool SIGSubscriptionID *string } @@ -69,7 +69,7 @@ func Options(overrides ...OptionsFields) *azoptions.Options { SubnetID: lo.FromPtrOr(options.SubnetID, "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/sillygeese/providers/Microsoft.Network/virtualNetworks/karpentervnet/subnets/karpentersub"), NodeResourceGroup: lo.FromPtrOr(options.NodeResourceGroup, "test-resourceGroup"), ProvisionMode: lo.FromPtrOr(options.ProvisionMode, "aksscriptless"), - ManagedKarpenter: lo.FromPtrOr(options.ManagedKarpenter, false), + UseSIG: lo.FromPtrOr(options.UseSIG, false), SIGSubscriptionID: lo.FromPtrOr(options.SIGSubscriptionID, "10945678-1234-1234-1234-123456789012"), } } From 1901a832da28c145457789dfda0052bad88ca1a3 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Thu, 14 Nov 2024 12:44:18 -0800 Subject: [PATCH 19/22] refactor: spelling --- pkg/operator/options/options.go | 7 +++---- pkg/providers/imagefamily/image.go | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/operator/options/options.go b/pkg/operator/options/options.go index 59cc249b5..ed0ef6db6 100644 --- a/pkg/operator/options/options.go +++ b/pkg/operator/options/options.go @@ -77,10 +77,9 @@ type Options struct { ProvisionMode string NodeBootstrappingServerURL string - ManagedKarpenter bool // => ManagedKarpenter is true if Karpenter is managed by AKS, false if it is a self-hosted karpenter installation + UseSIG bool // => UseSIG is true if Karpenter is managed by AKS, false if it is a self-hosted karpenter installation SIGSubscriptionID string - NodeResourceGroup string } @@ -96,10 +95,10 @@ func (o *Options) AddFlags(fs *coreoptions.FlagSet) { fs.StringVar(&o.NetworkDataplane, "network-dataplane", env.WithDefaultString("NETWORK_DATAPLANE", "cilium"), "The network dataplane used by the cluster.") fs.StringVar(&o.SubnetID, "vnet-subnet-id", env.WithDefaultString("VNET_SUBNET_ID", ""), "The default subnet ID to use for new nodes. This must be a valid ARM resource ID for subnet that does not overlap with the service CIDR or the pod CIDR.") fs.Var(newNodeIdentitiesValue(env.WithDefaultString("NODE_IDENTITIES", ""), &o.NodeIdentities), "node-identities", "User assigned identities for nodes.") - fs.StringVar(&o.NodeResourceGroup, "node-resource-group", env.WithDefaultString("AZURE_NODE_RESOURCE_GROUP", ""), "[REQUIRED] the resource group created and managed by AKS where the nodes live.") fs.StringVar(&o.ProvisionMode, "provision-mode", env.WithDefaultString("PROVISION_MODE", consts.ProvisionModeAKSScriptless), "[UNSUPPORTED] The provision mode for the cluster.") fs.StringVar(&o.NodeBootstrappingServerURL, "nodebootstrapping-server-url", env.WithDefaultString("NODEBOOTSTRAPPING_SERVER_URL", ""), "[UNSUPPORTED] The url for the node bootstrapping provider server.") - fs.BoolVar(&o.ManagedKarpenter, "use-sig", env.WithDefaultBool("USE_SIG", false), "If set to true karpenter will use the AKS managed shared image galleries and the node image versions api. If set to false karpenter will use community image galleries. Only a subset of image features will be available in the community image galleries and this flag is only for the managed node provisioning addon.") + fs.StringVar(&o.NodeResourceGroup, "node-resource-group", env.WithDefaultString("AZURE_NODE_RESOURCE_GROUP", ""), "[REQUIRED] the resource group created and managed by AKS where the nodes live") + fs.BoolVar(&o.UseSIG, "use-sig", env.WithDefaultBool("USE_SIG", false), "If set to true karpenter will use the AKS managed shared image galleries and the node image versions api. If set to false karpenter will use community image galleries. Only a subset of image features will be available in the community image galleries and this flag is only for the managed node provisioning addon.") fs.StringVar(&o.SIGSubscriptionID, "sig-subscription-id", env.WithDefaultString("SIG_SUBSCRIPTION_ID", ""), "The subscription ID of the shared image gallery.") } diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index 1b9915c3c..49990d4e8 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -82,7 +82,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1alpha2.AKSNodeClass, in func (p *Provider) GetLatestImageID(ctx context.Context, defaultImage DefaultImageOutput) (string, error) { // Managed Karpenter will use the AKS Managed Shared Image Galleries - if options.FromContext(ctx).ManagedKarpenter { + if options.FromContext(ctx).UseSIG { return p.getImageIDSIG(ctx, defaultImage) } // Self Hosted Karpenter will use the Community Image Galleries, which are public and have lower scaling limits From cc07eb7746934bde2b0ef3f41b617f2027c44207 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Fri, 15 Nov 2024 01:41:03 -0800 Subject: [PATCH 20/22] fix: v1 migration for test --- pkg/providers/instancetype/suite_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index 1f07dc357..9236a8119 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -1,6 +1,6 @@ /* Portions Copyright (c) Microsoft Corporation. - +/ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at @@ -934,13 +934,13 @@ var _ = Describe("InstanceType Provider", func() { }) ctx = options.ToContext(ctx) nodeClass.Spec.ImageFamily = lo.ToPtr(imageFamily) - coretest.ReplaceRequirements(nodePool, corev1beta1.NodeSelectorRequirementWithMinValues{ + coretest.ReplaceRequirements(nodePool, karpv1.NodeSelectorRequirementWithMinValues{ NodeSelectorRequirement: v1.NodeSelectorRequirement{ Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpIn, Values: []string{instanceType}, }}) - nodePool.Spec.Template.Spec.NodeClassRef = &corev1beta1.NodeClassReference{Name: nodeClass.Name} + nodePool.Spec.Template.Spec.NodeClassRef = &karpv1.NodeClassReference{Name: nodeClass.Name} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod(coretest.PodOptions{}) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, coreProvisioner, pod) From 4b05f7ffee15b02ac98a83f9708cb2444479e7e6 Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Fri, 15 Nov 2024 02:03:48 -0800 Subject: [PATCH 21/22] ci: lint --- pkg/providers/imagefamily/image.go | 8 ++++---- pkg/providers/imagefamily/nodeimageversionsclient.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go index 49990d4e8..1e52b6ce0 100644 --- a/pkg/providers/imagefamily/image.go +++ b/pkg/providers/imagefamily/image.go @@ -50,8 +50,8 @@ const ( imageExpirationInterval = time.Hour * 24 * 3 imageCacheCleaningInterval = time.Hour * 1 - sharedImageGalleryImageIDFormat = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/%s/images/%s/versions/%s" - communityImageIDFormat = "/CommunityGalleries/%s/images/%s/versions/%s" + sharedImageGalleryImageIDFormat = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/%s/images/%s" + communityImageIDFormat = "/CommunityGalleries/%s/images/%s" ) func NewProvider(kubernetesInterface kubernetes.Interface, kubernetesVersionCache *cache.Cache, versionsClient CommunityGalleryImageVersionsAPI, location, subscription string, nodeImageVersionsClient NodeImageVersionsAPI) *Provider { @@ -86,7 +86,7 @@ func (p *Provider) GetLatestImageID(ctx context.Context, defaultImage DefaultIma return p.getImageIDSIG(ctx, defaultImage) } // Self Hosted Karpenter will use the Community Image Galleries, which are public and have lower scaling limits - return p.getImageIDCIG(ctx, defaultImage.PublicGalleryURL, defaultImage.ImageDefinition) + return p.getImageIDCIG(defaultImage.PublicGalleryURL, defaultImage.ImageDefinition) } func (p *Provider) KubeServerVersion(ctx context.Context) (string, error) { @@ -125,7 +125,7 @@ func (p *Provider) getImageIDSIG(ctx context.Context, imgStub DefaultImageOutput return "", fmt.Errorf("failed to get the latest version of the image %s", imgStub.ImageDefinition) } -func (p *Provider) getImageIDCIG(ctx context.Context, publicGalleryURL, communityImageName string) (string, error) { +func (p *Provider) getImageIDCIG(publicGalleryURL, communityImageName string) (string, error) { key := fmt.Sprintf(communityImageIDFormat, publicGalleryURL, communityImageName) if imageID, ok := p.imageCache.Get(key); ok { return imageID.(string), nil diff --git a/pkg/providers/imagefamily/nodeimageversionsclient.go b/pkg/providers/imagefamily/nodeimageversionsclient.go index 59963a905..d9e62b5ad 100644 --- a/pkg/providers/imagefamily/nodeimageversionsclient.go +++ b/pkg/providers/imagefamily/nodeimageversionsclient.go @@ -91,8 +91,8 @@ func isNewerVersion(version1, version2 string) bool { // Split by dots and compare each segment as an integer var v1, v2 [4]int - fmt.Sscanf(version1, "%d.%d.%d.%d", &v1[0], &v1[1], &v1[2], &v1[3]) - fmt.Sscanf(version2, "%d.%d.%d.%d", &v2[0], &v2[1], &v2[2], &v2[3]) + fmt.Sscanf(version1, "%d.%d.%d.%d", &v1[0], &v1[1], &v1[2], &v1[3]) //nolint:errcheck + fmt.Sscanf(version2, "%d.%d.%d.%d", &v2[0], &v2[1], &v2[2], &v2[3]) //nolint:errcheck for i := 0; i < 4; i++ { if v1[i] > v2[i] { From 29deb554c2bed4254944ed3e8d4e5011f644db6f Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Fri, 15 Nov 2024 02:04:36 -0800 Subject: [PATCH 22/22] fix: lint --- pkg/providers/instance/instance.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index 158f20fee..c85b69b3e 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -413,7 +413,6 @@ func setImageReference(ctx context.Context, vmProperties *armcompute.VirtualMach vmProperties.StorageProfile.ImageReference = &armcompute.ImageReference{ CommunityGalleryImageID: lo.ToPtr(imageID), } - return } // setVMPropertiesBillingProfile sets a default MaxPrice of -1 for Spot