test: adding test coverage for image resolution codepath (#89)

* testing az linux coverage * test: adding coverage for Name() and UserData() in imagefamily interface for azlinux * ci * Update pkg/providers/instancetype/suite_test.go Co-authored-by: Alex Leites <[email protected]> * Update pkg/providers/imagefamily/image_test.go Co-authored-by: Alex Leites <[email protected]> * test: using acceptance style for AzureLinux Coverage * test: moving image selection testing to acceptance test style * test: converting ImageProvider + Image Family tests to ginkgo acceptance, removing old tests * ci * test: refactored Public Gallery URLS back into imagefamily * fix: comment on gen2 over gen1 preference * . --------- Co-authored-by: Alex Leites <[email protected]>
Azure · Jan 20, 2024 · 7f19cd1 · 7f19cd1
1 parent e781996
commit 7f19cd1
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 58 deletions.
diff --git a/hack/codegen.sh b/hack/codegen.sh
@@ -32,7 +32,7 @@ skugen() {
   NO_UPDATE=" pkg/fake/zz_generated.sku.$location.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)"
   SUBJECT="SKUGEN"
 
-  go run hack/code/instancetype_testdata_gen.go -- "${GENERATED_FILE}" "$location" "Standard_B1s,Standard_A0,Standard_D2_v2,Standard_D2_v3,Standard_DS2_v2,Standard_D2s_v3,Standard_D2_v5,Standard_F16s_v2,Standard_NC6s,Standard_NC6s_v3,Standard_NC16as_T4_v3,Standard_NC24ads_A100_v4,Standard_M8-2ms,Standard_D4s_v3,Standard_D64s_v3,Standard_DC8s_v3"
+  go run hack/code/instancetype_testdata_gen.go -- "${GENERATED_FILE}" "$location" "Standard_B1s,Standard_A0,Standard_D2_v2,Standard_D2_v3,Standard_DS2_v2,Standard_D2s_v3,Standard_D2_v5,Standard_D16plds_v5,Standard_F16s_v2,Standard_NC6s,Standard_NC6s_v3,Standard_NC16as_T4_v3,Standard_NC24ads_A100_v4,Standard_M8-2ms,Standard_D4s_v3,Standard_D64s_v3,Standard_DC8s_v3"
   go fmt "${GENERATED_FILE}"
 
   GIT_DIFF=$(git diff --stat "${GENERATED_FILE}")

diff --git a/pkg/fake/zz_generated.sku.eastus.go b/pkg/fake/zz_generated.sku.eastus.go
@@ -24,7 +24,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute"
 )
 
-// generated at 2023-12-18T15:28:53Z
+// generated at 2024-01-12T06:16:27Z
 
 func init() {
 	// ResourceSkus is a list of selected VM SKUs for a given region
@@ -39,17 +39,6 @@ func init() {
 			APIVersions:  &[]string{},
 			Costs:        &[]compute.ResourceSkuCosts{},
 			Restrictions: &[]compute.ResourceSkuRestrictions{
-				{
-					Type:   compute.ResourceSkuRestrictionsType("Location"),
-					Values: &[]string{"eastus"},
-					RestrictionInfo: &compute.ResourceSkuRestrictionInfo{
-						Locations: &[]string{
-							"eastus",
-						},
-						Zones: &[]string{},
-					},
-					ReasonCode: "NotAvailableForSubscription",
-				},
 				{
 					Type:   compute.ResourceSkuRestrictionsType("Zone"),
 					Values: &[]string{"eastus"},
@@ -137,6 +126,53 @@ func init() {
 			},
 			},
 		},
+		{
+			Name:         lo.ToPtr("Standard_D16plds_v5"),
+			Tier:         lo.ToPtr("Standard"),
+			Kind:         lo.ToPtr(""),
+			Size:         lo.ToPtr("D16plds_v5"),
+			Family:       lo.ToPtr("standardDPLDSv5Family"),
+			ResourceType: lo.ToPtr("virtualMachines"),
+			APIVersions:  &[]string{},
+			Costs:        &[]compute.ResourceSkuCosts{},
+			Restrictions: &[]compute.ResourceSkuRestrictions{},
+			Capabilities: &[]compute.ResourceSkuCapabilities{
+				{Name: lo.ToPtr("MaxResourceVolumeMB"), Value: lo.ToPtr("614400")},
+				{Name: lo.ToPtr("OSVhdSizeMB"), Value: lo.ToPtr("1047552")},
+				{Name: lo.ToPtr("vCPUs"), Value: lo.ToPtr("16")},
+				{Name: lo.ToPtr("MemoryPreservingMaintenanceSupported"), Value: lo.ToPtr("True")},
+				{Name: lo.ToPtr("HyperVGenerations"), Value: lo.ToPtr("V2")},
+				{Name: lo.ToPtr("MemoryGB"), Value: lo.ToPtr("32")},
+				{Name: lo.ToPtr("MaxDataDiskCount"), Value: lo.ToPtr("32")},
+				{Name: lo.ToPtr("CpuArchitectureType"), Value: lo.ToPtr("Arm64")},
+				{Name: lo.ToPtr("LowPriorityCapable"), Value: lo.ToPtr("True")},
+				{Name: lo.ToPtr("PremiumIO"), Value: lo.ToPtr("True")},
+				{Name: lo.ToPtr("VMDeploymentTypes"), Value: lo.ToPtr("IaaS")},
+				{Name: lo.ToPtr("vCPUsAvailable"), Value: lo.ToPtr("16")},
+				{Name: lo.ToPtr("vCPUsPerCore"), Value: lo.ToPtr("1")},
+				{Name: lo.ToPtr("CombinedTempDiskAndCachedIOPS"), Value: lo.ToPtr("75000")},
+				{Name: lo.ToPtr("CombinedTempDiskAndCachedReadBytesPerSecond"), Value: lo.ToPtr("1000000000")},
+				{Name: lo.ToPtr("CombinedTempDiskAndCachedWriteBytesPerSecond"), Value: lo.ToPtr("1000000000")},
+				{Name: lo.ToPtr("CachedDiskBytes"), Value: lo.ToPtr("429496729600")},
+				{Name: lo.ToPtr("UncachedDiskIOPS"), Value: lo.ToPtr("25600")},
+				{Name: lo.ToPtr("UncachedDiskBytesPerSecond"), Value: lo.ToPtr("600000000")},
+				{Name: lo.ToPtr("EphemeralOSDiskSupported"), Value: lo.ToPtr("True")},
+				{Name: lo.ToPtr("EncryptionAtHostSupported"), Value: lo.ToPtr("True")},
+				{Name: lo.ToPtr("CapacityReservationSupported"), Value: lo.ToPtr("False")},
+				{Name: lo.ToPtr("TrustedLaunchDisabled"), Value: lo.ToPtr("True")},
+				{Name: lo.ToPtr("AcceleratedNetworkingEnabled"), Value: lo.ToPtr("True")},
+				{Name: lo.ToPtr("RdmaEnabled"), Value: lo.ToPtr("False")},
+				{Name: lo.ToPtr("MaxNetworkInterfaces"), Value: lo.ToPtr("4")},
+			},
+			Locations: &[]string{"eastus"},
+			LocationInfo: &[]compute.ResourceSkuLocationInfo{{Location: lo.ToPtr("eastus"), Zones: &[]string{
+				"1",
+				"2",
+				"3",
+			},
+			},
+			},
+		},
 		{
 			Name:         lo.ToPtr("Standard_D2s_v3"),
 			Tier:         lo.ToPtr("Standard"),
@@ -449,7 +485,7 @@ func init() {
 			},
 			Locations: &[]string{"eastus"},
 			LocationInfo: &[]compute.ResourceSkuLocationInfo{{Location: lo.ToPtr("eastus"), Zones: &[]string{
-				"3",
+				"1",
 			},
 			},
 			},
@@ -566,7 +602,7 @@ func init() {
 							"eastus",
 						},
 						Zones: &[]string{
-							"3",
+							"1",
 						},
 					},
 					ReasonCode: "NotAvailableForSubscription",
@@ -700,7 +736,7 @@ func init() {
 			},
 			Locations: &[]string{"eastus"},
 			LocationInfo: &[]compute.ResourceSkuLocationInfo{{Location: lo.ToPtr("eastus"), Zones: &[]string{
-				"3",
+				"1",
 			},
 			},
 			},

diff --git a/pkg/providers/imagefamily/azlinux.go b/pkg/providers/imagefamily/azlinux.go
@@ -43,7 +43,7 @@ func (u AzureLinux) Name() string {
 }
 
 func (u AzureLinux) DefaultImages() []DefaultImageOutput {
-	// image provider will select these images in order, first match wins. This is why we chose to put AzureLinuxGen2containerd first in the defaultImages
+	// 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,

diff --git a/pkg/providers/imagefamily/image_test.go b/pkg/providers/imagefamily/image_test.go
@@ -30,11 +30,8 @@ import (
 	"github.com/Azure/karpenter/pkg/fake"
 	"github.com/Azure/karpenter/pkg/providers/imagefamily"
 	"github.com/Azure/karpenter/pkg/test"
-	corev1beta1 "github.com/aws/karpenter-core/pkg/apis/v1beta1"
 	"github.com/aws/karpenter-core/pkg/cloudprovider"
-	"github.com/aws/karpenter-core/pkg/scheduling"
 	"github.com/samber/lo"
-	corev1 "k8s.io/api/core/v1"
 )
 
 var imageProvider *imagefamily.Provider
@@ -141,44 +138,6 @@ var _ = Describe("Image ID Resolution", func() {
 		Entry("Image version is specified, should use it", imagefamily.Ubuntu2204Gen2CommunityImage, imagefamily.AKSUbuntuPublicGalleryURL, olderImageVersion, fmt.Sprintf("/CommunityGalleries/%s/images/%s/versions/%s", imagefamily.AKSUbuntuPublicGalleryURL, imagefamily.Ubuntu2204Gen2CommunityImage, olderImageVersion)),
 	)
 
-	DescribeTable("Ubuntu2204 Image Resolution",
-		func(nodeClass *v1alpha2.AKSNodeClass, instanceType *cloudprovider.InstanceType, expectedImageID string) {
-			imageID, err := imageProvider.Get(context.Background(), nodeClass, instanceType, imagefamily.Ubuntu2204{})
-			Expect(err).To(BeNil())
-			Expect(imageID).To(Equal(expectedImageID))
-		},
-		Entry("Arm64 Image for HyperV Gen 2",
-			test.AKSNodeClass(),
-			&cloudprovider.InstanceType{
-				Name:         "Standard_D8pls_v5",
-				Requirements: scheduling.NewRequirements(scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, corev1beta1.ArchitectureArm64), scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, corev1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2)),
-			},
-			fmt.Sprintf("/CommunityGalleries/%s/images/2204gen2arm64containerd/versions/1.1686127203.20217", imagefamily.AKSUbuntuPublicGalleryURL)),
-		Entry("Gen2 Image for HyperV Gen 2",
-			test.AKSNodeClass(),
-			&cloudprovider.InstanceType{
-				Name: "Standard_D2s_v3",
-				Requirements: scheduling.NewRequirements(
-					//					scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, corev1beta1.ArchitectureAmd64),
-					scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, corev1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV2),
-				),
-			},
-			fmt.Sprintf("/CommunityGalleries/%s/images/2204gen2containerd/versions/1.1686127203.20217", imagefamily.AKSUbuntuPublicGalleryURL)),
-		Entry("Gen1 Image for HyperV Gen 1",
-			test.AKSNodeClass(),
-			&cloudprovider.InstanceType{
-				Name: "Standard_D2s_v3",
-				Requirements: scheduling.NewRequirements(
-					//					scheduling.NewRequirement(v1.LabelArchStable, v1.NodeSelectorOpIn, corev1beta1.ArchitectureAmd64),
-					scheduling.NewRequirement(v1alpha2.LabelSKUHyperVGeneration, corev1.NodeSelectorOpIn, v1alpha2.HyperVGenerationV1),
-				),
-			},
-			fmt.Sprintf("/CommunityGalleries/%s/images/2204containerd/versions/1.1686127203.20217", imagefamily.AKSUbuntuPublicGalleryURL)),
-		Entry("Gen2 Image if no preference is specified",
-			test.AKSNodeClass(),
-			&cloudprovider.InstanceType{Name: "Standard_D2s_v3", Requirements: scheduling.NewRequirements()},
-			fmt.Sprintf("/CommunityGalleries/%s/images/2204gen2containerd/versions/1.1686127203.20217", imagefamily.AKSUbuntuPublicGalleryURL)),
-	)
 })
 
 var _ = Describe("Image ID Parsing", func() {

diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go
@@ -60,6 +60,7 @@ import (
 	"github.com/Azure/karpenter/pkg/apis/v1alpha2"
 	"github.com/Azure/karpenter/pkg/cloudprovider"
 	"github.com/Azure/karpenter/pkg/fake"
+	"github.com/Azure/karpenter/pkg/providers/imagefamily"
 	"github.com/Azure/karpenter/pkg/providers/instancetype"
 	"github.com/Azure/karpenter/pkg/providers/loadbalancer"
 	"github.com/Azure/karpenter/pkg/test"
@@ -759,6 +760,47 @@ var _ = Describe("InstanceType Provider", func() {
 		})
 	})
 
+	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) {
+				nodeClass.Spec.ImageFamily = lo.ToPtr(imageFamily)
+				coretest.ReplaceRequirements(nodePool, 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())
+				Expect(vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID).ToNot(BeNil())
+				parts := strings.Split(*vm.Properties.StorageProfile.ImageReference.CommunityGalleryImageID, "/")
+				Expect(parts[2]).To(Equal(expectedGalleryURL))
+				Expect(parts[4]).To(Equal(expectedImageDefinition))
+
+				// Need to reset env since we are doing these nested tests
+				cluster.Reset()
+				azureEnv.Reset()
+			},
+			Entry("Gen2, Gen1 instance type with AKSUbuntu image family",
+				"Standard_D2_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2CommunityImage, imagefamily.AKSUbuntuPublicGalleryURL),
+			Entry("Gen1 instance type with AKSUbuntu image family",
+				"Standard_D2_v3", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen1CommunityImage, imagefamily.AKSUbuntuPublicGalleryURL),
+			Entry("ARM instance type with AKSUbuntu image family",
+				"Standard_D16plds_v5", v1alpha2.Ubuntu2204ImageFamily, imagefamily.Ubuntu2204Gen2ArmCommunityImage, imagefamily.AKSUbuntuPublicGalleryURL),
+			Entry("Gen2 instance type with AzureLinux image family",
+				"Standard_D2_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2CommunityImage, imagefamily.AKSAzureLinuxPublicGalleryURL),
+			Entry("Gen1 instance type with AzureLinux image family",
+				"Standard_D2_v3", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen1CommunityImage, imagefamily.AKSAzureLinuxPublicGalleryURL),
+			Entry("ARM instance type with AzureLinux image family",
+				"Standard_D16plds_v5", v1alpha2.AzureLinuxImageFamily, imagefamily.AzureLinuxGen2ArmCommunityImage, imagefamily.AKSAzureLinuxPublicGalleryURL),
+		)
+	})
 	Context("Instance Types", func() {
 		It("should support provisioning with no labels", func() {
 			ExpectApplied(ctx, env.Client, nodePool, nodeClass)