BREAKING CHANGE(aksnodeclass): removing imageVersion from CRD (#531)

* remove imageVersion from CRD * changes removing imageVersion field from code * minor additional cleanup --------- Co-authored-by: Charlie McBride <[email protected]>
Azure · Oct 18, 2024 · 4ce57d9 · 4ce57d9
1 parent b8fa120
commit 4ce57d9
Show file tree

Hide file tree

Showing 8 changed files with 17 additions and 89 deletions.
diff --git a/pkg/apis/crds/karpenter.azure.com_aksnodeclasses.yaml b/pkg/apis/crds/karpenter.azure.com_aksnodeclasses.yaml
@@ -53,9 +53,6 @@ spec:
                 - Ubuntu2204
                 - AzureLinux
                 type: string
-              imageVersion:
-                description: ImageVersion is the image version that instances use.
-                type: string
               osDiskSizeGB:
                 default: 128
                 description: osDiskSizeGB is the size of the OS disk in GB.

diff --git a/pkg/apis/v1alpha2/aksnodeclass.go b/pkg/apis/v1alpha2/aksnodeclass.go
@@ -43,9 +43,6 @@ type AKSNodeClassSpec struct {
 	// +kubebuilder:default=Ubuntu2204
 	// +kubebuilder:validation:Enum:={Ubuntu2204,AzureLinux}
 	ImageFamily *string `json:"imageFamily,omitempty"`
-	// ImageVersion is the image version that instances use.
-	// +optional
-	ImageVersion *string `json:"imageVersion,omitempty"`
 	// Tags to be applied on Azure resources like instances.
 	// +optional
 	Tags map[string]string `json:"tags,omitempty"`

diff --git a/pkg/apis/v1alpha2/aksnodeclass_utils.go b/pkg/apis/v1alpha2/aksnodeclass_utils.go
diff --git a/pkg/apis/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/v1alpha2/zz_generated.deepcopy.go
diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go
@@ -199,7 +199,7 @@ func (c *CloudProvider) IsDrifted(ctx context.Context, nodeClaim *corev1beta1.No
 	if k8sVersionDrifted != "" {
 		return k8sVersionDrifted, nil
 	}
-	imageVersionDrifted, err := c.isImageVersionDrifted(ctx, nodeClaim, nodeClass)
+	imageVersionDrifted, err := c.isImageVersionDrifted(ctx, nodeClaim)
 	if err != nil {
 		return "", err
 	}

diff --git a/pkg/cloudprovider/drift.go b/pkg/cloudprovider/drift.go
@@ -78,7 +78,7 @@ func (c *CloudProvider) isK8sVersionDrifted(ctx context.Context, nodeClaim *core
 // Feel reassessing this within the future with a potential minor refactor would be best to fix the gocyclo.
 // nolint: gocyclo
 func (c *CloudProvider) isImageVersionDrifted(
-	ctx context.Context, nodeClaim *corev1beta1.NodeClaim, nodeClass *v1alpha2.AKSNodeClass) (cloudprovider.DriftReason, error) {
+	ctx context.Context, nodeClaim *corev1beta1.NodeClaim) (cloudprovider.DriftReason, error) {
 	logger := logging.FromContext(ctx)
 
 	id, err := utils.GetVMName(nodeClaim.Status.ProviderID)
@@ -113,7 +113,7 @@ func (c *CloudProvider) isImageVersionDrifted(
 		return "", err
 	}
 
-	expectedImageID, err := c.imageProvider.GetImageID(ctx, communityImageName, publicGalleryURL, nodeClass.Spec.GetImageVersion())
+	expectedImageID, err := c.imageProvider.GetImageID(ctx, communityImageName, publicGalleryURL)
 	if err != nil {
 		return "", err
 	}

diff --git a/pkg/providers/imagefamily/image.go b/pkg/providers/imagefamily/image.go
@@ -68,7 +68,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1alpha2.AKSNodeClass, in
 	for _, defaultImage := range defaultImages {
 		if err := instanceType.Requirements.Compatible(defaultImage.Requirements, v1alpha2.AllowUndefinedLabels); err == nil {
 			communityImageName, publicGalleryURL := defaultImage.CommunityImage, defaultImage.PublicGalleryURL
-			return p.GetImageID(ctx, communityImageName, publicGalleryURL, nodeClass.Spec.GetImageVersion())
+			return p.GetImageID(ctx, communityImageName, publicGalleryURL)
 		}
 	}
 
@@ -92,29 +92,27 @@ func (p *Provider) KubeServerVersion(ctx context.Context) (string, error) {
 }
 
 // Input versionName == "" to get the latest version
-func (p *Provider) GetImageID(ctx context.Context, communityImageName, publicGalleryURL, versionName string) (string, error) {
-	key := fmt.Sprintf("%s/%s/%s", publicGalleryURL, communityImageName, versionName)
+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 {
 		return imageID.(string), nil
 	}
 
-	if versionName == "" {
-		pager := p.imageVersionsClient.NewListPager(p.location, publicGalleryURL, communityImageName, nil)
-		topImageVersionCandidate := armcompute.CommunityGalleryImageVersion{}
-		for pager.More() {
-			page, err := pager.NextPage(context.Background())
-			if err != nil {
-				return "", err
-			}
-			for _, imageVersion := range page.CommunityGalleryImageVersionList.Value {
-				if lo.IsEmpty(topImageVersionCandidate) || imageVersion.Properties.PublishedDate.After(*topImageVersionCandidate.Properties.PublishedDate) {
-					topImageVersionCandidate = *imageVersion
-				}
+	pager := p.imageVersionsClient.NewListPager(p.location, publicGalleryURL, communityImageName, nil)
+	topImageVersionCandidate := armcompute.CommunityGalleryImageVersion{}
+	for pager.More() {
+		page, err := pager.NextPage(context.Background())
+		if err != nil {
+			return "", err
+		}
+		for _, imageVersion := range page.CommunityGalleryImageVersionList.Value {
+			if lo.IsEmpty(topImageVersionCandidate) || imageVersion.Properties.PublishedDate.After(*topImageVersionCandidate.Properties.PublishedDate) {
+				topImageVersionCandidate = *imageVersion
 			}
 		}
-		versionName = lo.FromPtr(topImageVersionCandidate.Name)
 	}
+	versionName := lo.FromPtr(topImageVersionCandidate.Name)
 
 	selectedImageID := BuildImageID(publicGalleryURL, communityImageName, versionName)
 	if p.cm.HasChanged(key, selectedImageID) {

diff --git a/test/suites/drift/suite_test.go b/test/suites/drift/suite_test.go
@@ -111,41 +111,6 @@ var _ = Describe("Drift", func() {
 		env.EventuallyExpectNotFound(pod, nodeClaim, node)
 		SetDefaultEventuallyTimeout(5 * time.Minute)
 	})
-	It("should upgrade nodes using drift based on node image version change", func() {
-		// TODO: Get these dynamically
-		startingImageVersion := "202404.09.0"
-		upgradedImageVersion := "202404.16.0"
-
-		nodeClass.Spec.ImageVersion = &startingImageVersion
-
-		By(fmt.Sprintf("creating pod %s, nodepool %s, and nodeclass %s", pod.Name, nodePool.Name, nodeClass.Name))
-		env.ExpectCreated(pod, nodeClass, nodePool)
-
-		By(fmt.Sprintf("expect pod %s to be healthy", pod.Name))
-		env.EventuallyExpectHealthy(pod)
-
-		By("expect created node count to be 1")
-		env.ExpectCreatedNodeCount("==", 1)
-
-		nodeClaim := env.EventuallyExpectCreatedNodeClaimCount("==", 1)[0]
-		node := env.EventuallyExpectNodeCount("==", 1)[0]
-
-		By(fmt.Sprintf("waiting for nodeClass %s update", nodeClass.Name))
-		nodeClass.Spec.ImageVersion = &upgradedImageVersion
-		env.ExpectCreatedOrUpdated(nodeClass)
-
-		By(fmt.Sprintf("waiting for nodeclaim %s to be marked as drifted", nodeClaim.Name))
-		env.EventuallyExpectDrifted(nodeClaim)
-
-		By(fmt.Sprintf("waiting for pod %s to to update", pod.Name))
-		delete(pod.Annotations, corev1beta1.DoNotDisruptAnnotationKey)
-		env.ExpectUpdated(pod)
-
-		By(fmt.Sprintf("expect pod %s, nodeclaim %s, and node %s to eventually not exist", pod.Name, nodeClaim.Name, node.Name))
-		SetDefaultEventuallyTimeout(10 * time.Minute)
-		env.EventuallyExpectNotFound(pod, nodeClaim, node)
-		SetDefaultEventuallyTimeout(5 * time.Minute)
-	})
 	It("should mark the nodeclaim as drifted for SubnetDrift if AKSNodeClass subnet id changes", func() {
 		env.ExpectCreated(pod, nodeClass, nodePool)