diff --git a/azure/converters/vmss.go b/azure/converters/vmss.go index b588c31bf08..7ad7b50c3c0 100644 --- a/azure/converters/vmss.go +++ b/azure/converters/vmss.go @@ -68,7 +68,7 @@ func SDKToVMSS(sdkvmss armcompute.VirtualMachineScaleSet, sdkinstances []armcomp sdkvmss.Properties.VirtualMachineProfile.StorageProfile != nil && sdkvmss.Properties.VirtualMachineProfile.StorageProfile.ImageReference != nil { imageRef := sdkvmss.Properties.VirtualMachineProfile.StorageProfile.ImageReference - vmss.Image = SDKImageToImage(imageRef, sdkvmss.Plan != nil) + vmss.Image = SDKImageToImage(imageRef, sdkvmss.Plan) } return vmss @@ -95,7 +95,7 @@ func SDKVMToVMSSVM(sdkInstance armcompute.VirtualMachine, mode infrav1.Orchestra if sdkInstance.Properties.StorageProfile != nil && sdkInstance.Properties.StorageProfile.ImageReference != nil { imageRef := sdkInstance.Properties.StorageProfile.ImageReference - instance.Image = SDKImageToImage(imageRef, sdkInstance.Plan != nil) + instance.Image = SDKImageToImage(imageRef, sdkInstance.Plan) } if len(sdkInstance.Zones) > 0 { @@ -147,7 +147,7 @@ func SDKToVMSSVM(sdkInstance armcompute.VirtualMachineScaleSetVM) *azure.VMSSVM if sdkInstance.Properties.StorageProfile != nil && sdkInstance.Properties.StorageProfile.ImageReference != nil { imageRef := sdkInstance.Properties.StorageProfile.ImageReference - instance.Image = SDKImageToImage(imageRef, sdkInstance.Plan != nil) + instance.Image = SDKImageToImage(imageRef, sdkInstance.Plan) } if len(sdkInstance.Zones) > 0 { @@ -159,20 +159,33 @@ func SDKToVMSSVM(sdkInstance armcompute.VirtualMachineScaleSetVM) *azure.VMSSVM } // SDKImageToImage converts a SDK image reference to infrav1.Image. -func SDKImageToImage(sdkImageRef *armcompute.ImageReference, isThirdPartyImage bool) infrav1.Image { - if sdkImageRef.ID != nil { - return IDImageRefToImage(*sdkImageRef.ID) +func SDKImageToImage(sdkImageRef *armcompute.ImageReference, sdkPlan *armcompute.Plan) infrav1.Image { + var image infrav1.Image + switch { + case sdkImageRef.ID != nil: + image = IDImageRefToImage(*sdkImageRef.ID) + case sdkImageRef.CommunityGalleryImageID != nil: + image = cgImageRefToImage(*sdkImageRef.CommunityGalleryImageID) + case sdkImageRef.SharedGalleryImageID != nil: + image = sgImageRefToImage(*sdkImageRef.SharedGalleryImageID) + default: + image = mpImageRefToImage(sdkImageRef, sdkPlan != nil) } - // community gallery image - if sdkImageRef.CommunityGalleryImageID != nil { - return cgImageRefToImage(*sdkImageRef.CommunityGalleryImageID) - } - // shared gallery image - if sdkImageRef.SharedGalleryImageID != nil { - return sgImageRefToImage(*sdkImageRef.SharedGalleryImageID) + + if sdkPlan != nil { + if image.ComputeGallery != nil { + image.ComputeGallery.Plan = &infrav1.ImagePlan{ + Publisher: ptr.Deref(sdkPlan.Publisher, ""), + Offer: ptr.Deref(sdkPlan.Product, ""), + SKU: ptr.Deref(sdkPlan.Name, ""), + } + } else if image.SharedGallery != nil { + image.SharedGallery.Publisher = sdkPlan.Publisher + image.SharedGallery.Offer = sdkPlan.Product + image.SharedGallery.SKU = sdkPlan.Name + } } - // marketplace image - return mpImageRefToImage(sdkImageRef, isThirdPartyImage) + return image } // GetOrchestrationMode returns the compute.OrchestrationMode for the given infrav1.OrchestrationModeType. diff --git a/azure/converters/vmss_test.go b/azure/converters/vmss_test.go index d137962ef62..d0099057a21 100644 --- a/azure/converters/vmss_test.go +++ b/azure/converters/vmss_test.go @@ -111,6 +111,53 @@ func Test_SDKToVMSS(t *testing.T) { g.Expect(actual).To(gomega.Equal(expected)) }, }, + { + Name: "ShouldPopulateImageWithPlan", + SubjectFactory: func(g *gomega.GomegaWithT) (armcompute.VirtualMachineScaleSet, []armcompute.VirtualMachineScaleSetVM) { + return armcompute.VirtualMachineScaleSet{ + ID: ptr.To("vmssIDWithPlan"), + Name: ptr.To("vmssNameWithPlan"), + Properties: &armcompute.VirtualMachineScaleSetProperties{ + ProvisioningState: ptr.To("Succeeded"), + VirtualMachineProfile: &armcompute.VirtualMachineScaleSetVMProfile{ + StorageProfile: &armcompute.VirtualMachineScaleSetStorageProfile{ + ImageReference: &armcompute.ImageReference{ + ID: ptr.To("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/galleries/gallery/images/image/versions/version"), + }, + }, + }, + }, + Plan: &armcompute.Plan{ + Publisher: ptr.To("publisher"), + Product: ptr.To("product"), + Name: ptr.To("sku"), + }, + }, nil + }, + Expect: func(g *gomega.GomegaWithT, actual azure.VMSS) { + expected := azure.VMSS{ + ID: "vmssIDWithPlan", + Name: "vmssNameWithPlan", + State: "Succeeded", + Image: infrav1.Image{ + ComputeGallery: &infrav1.AzureComputeGalleryImage{ + Gallery: "gallery", + Name: "image", + Version: "version", + SubscriptionID: ptr.To("subscription"), + ResourceGroup: ptr.To("rg"), + Plan: &infrav1.ImagePlan{ + Publisher: "publisher", + Offer: "product", + SKU: "sku", + }, + }, + }, + } + + g.Expect(actual).To(gomega.Equal(expected)) + }, + }, } for _, c := range cases { @@ -202,6 +249,42 @@ func Test_SDKToVMSSVM(t *testing.T) { State: "Creating", }, }, + { + Name: "VM with shared gallery storage and plan", + SDKInstance: armcompute.VirtualMachineScaleSetVM{ + ID: ptr.To("/subscriptions/foo/resourceGroups/MY_RESOURCE_GROUP/providers/bar"), + Properties: &armcompute.VirtualMachineScaleSetVMProperties{ + OSProfile: &armcompute.OSProfile{ComputerName: ptr.To("instance-000003")}, + StorageProfile: &armcompute.StorageProfile{ + ImageReference: &armcompute.ImageReference{ + SharedGalleryImageID: ptr.To("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/galleries/gallery/images/image/versions/version"), + }, + }, + }, + Plan: &armcompute.Plan{ + Publisher: ptr.To("publisher"), + Product: ptr.To("product"), + Name: ptr.To("sku"), + }, + }, + VMSSVM: &azure.VMSSVM{ + ID: "/subscriptions/foo/resourceGroups/my_resource_group/providers/bar", + Name: "instance-000003", + Image: infrav1.Image{ + SharedGallery: &infrav1.AzureSharedGalleryImage{ + SubscriptionID: "subscription", + ResourceGroup: "rg", + Gallery: "gallery", + Name: "image", + Version: "version", + Publisher: ptr.To("publisher"), + Offer: ptr.To("product"), + SKU: ptr.To("sku"), + }, + }, + State: "Creating", + }, + }, } for _, c := range cases { @@ -215,17 +298,16 @@ func Test_SDKToVMSSVM(t *testing.T) { func Test_SDKImageToImage(t *testing.T) { cases := []struct { - Name string - SDKImageRef *armcompute.ImageReference - IsThirdParty bool - Image infrav1.Image + Name string + SDKImageRef *armcompute.ImageReference + SDKPlan *armcompute.Plan + Image infrav1.Image }{ { Name: "id image", SDKImageRef: &armcompute.ImageReference{ ID: ptr.To("imageID"), }, - IsThirdParty: false, Image: infrav1.Image{ ID: ptr.To("imageID"), }, @@ -238,7 +320,11 @@ func Test_SDKImageToImage(t *testing.T) { SKU: ptr.To("sku"), Version: ptr.To("version"), }, - IsThirdParty: true, + SDKPlan: &armcompute.Plan{ + Publisher: ptr.To("publisher"), + Product: ptr.To("offer"), + Name: ptr.To("sku"), + }, Image: infrav1.Image{ Marketplace: &infrav1.AzureMarketplaceImage{ ImagePlan: infrav1.ImagePlan{ @@ -266,6 +352,29 @@ func Test_SDKImageToImage(t *testing.T) { }, }, }, + { + Name: "shared gallery image with plan", + SDKImageRef: &armcompute.ImageReference{ + SharedGalleryImageID: ptr.To("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/galleries/gallery/images/image/versions/version"), + }, + SDKPlan: &armcompute.Plan{ + Publisher: ptr.To("publisher"), + Product: ptr.To("product"), + Name: ptr.To("sku"), + }, + Image: infrav1.Image{ + SharedGallery: &infrav1.AzureSharedGalleryImage{ + SubscriptionID: "subscription", + ResourceGroup: "rg", + Gallery: "gallery", + Name: "image", + Version: "version", + Publisher: ptr.To("publisher"), + Offer: ptr.To("product"), + SKU: ptr.To("sku"), + }, + }, + }, { Name: "community gallery image", SDKImageRef: &armcompute.ImageReference{ @@ -294,6 +403,31 @@ func Test_SDKImageToImage(t *testing.T) { }, }, }, + { + Name: "compute gallery image with plan", + SDKImageRef: &armcompute.ImageReference{ + ID: ptr.To("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/galleries/gallery/images/image/versions/version"), + }, + SDKPlan: &armcompute.Plan{ + Publisher: ptr.To("publisher"), + Product: ptr.To("product"), + Name: ptr.To("sku"), + }, + Image: infrav1.Image{ + ComputeGallery: &infrav1.AzureComputeGalleryImage{ + Gallery: "gallery", + Name: "image", + Version: "version", + SubscriptionID: ptr.To("subscription"), + ResourceGroup: ptr.To("rg"), + Plan: &infrav1.ImagePlan{ + Publisher: "publisher", + Offer: "product", + SKU: "sku", + }, + }, + }, + }, { Name: "compute gallery image not formatted as expected", SDKImageRef: &armcompute.ImageReference{ @@ -316,7 +450,7 @@ func Test_SDKImageToImage(t *testing.T) { t.Run(c.Name, func(t *testing.T) { t.Parallel() g := gomega.NewGomegaWithT(t) - g.Expect(converters.SDKImageToImage(c.SDKImageRef, c.IsThirdParty)).To(gomega.Equal(c.Image)) + g.Expect(converters.SDKImageToImage(c.SDKImageRef, c.SDKPlan)).To(gomega.Equal(c.Image)) }) } } @@ -378,6 +512,44 @@ func Test_SDKVMToVMSSVM(t *testing.T) { State: "Creating", }, }, + { + Name: "VM with shared gallery storage and plan", + Subject: armcompute.VirtualMachine{ + ID: ptr.To("vmID5"), + Properties: &armcompute.VirtualMachineProperties{ + OSProfile: &armcompute.OSProfile{ + ComputerName: ptr.To("vmwithplan"), + }, + StorageProfile: &armcompute.StorageProfile{ + ImageReference: &armcompute.ImageReference{ + SharedGalleryImageID: ptr.To("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/galleries/gallery/images/image/versions/version"), + }, + }, + }, + Plan: &armcompute.Plan{ + Publisher: ptr.To("publisher"), + Product: ptr.To("product"), + Name: ptr.To("sku"), + }, + }, + Expected: &azure.VMSSVM{ + ID: "vmID5", + Image: infrav1.Image{ + SharedGallery: &infrav1.AzureSharedGalleryImage{ + SubscriptionID: "subscription", + ResourceGroup: "rg", + Gallery: "gallery", + Name: "image", + Version: "version", + Publisher: ptr.To("publisher"), + Offer: ptr.To("product"), + SKU: ptr.To("sku"), + }, + }, + Name: "vmwithplan", + State: "Creating", + }, + }, { Name: "VM with provisioning state", Subject: armcompute.VirtualMachine{ diff --git a/azure/scope/machinepoolmachine.go b/azure/scope/machinepoolmachine.go index 6fabff22ed1..682c7b41730 100644 --- a/azure/scope/machinepoolmachine.go +++ b/azure/scope/machinepoolmachine.go @@ -439,7 +439,13 @@ func (s *MachinePoolMachineScope) hasLatestModelApplied(ctx context.Context) (bo // this means the ID was a compute gallery image ID if newImage.ComputeGallery != nil { - return reflect.DeepEqual(s.instance.Image, newImage), nil + // The desired image from the spec has no plan metadata (the user + // cannot express it via image.ID), but the observed instance image + // may carry plan data populated by the SDK converter. Strip the + // plan from the observed copy so the comparison stays correct. + instanceImageCopy := s.instance.Image + instanceImageCopy.ComputeGallery.Plan = nil + return reflect.DeepEqual(instanceImageCopy, newImage), nil } } diff --git a/azure/scope/machinepoolmachine_test.go b/azure/scope/machinepoolmachine_test.go index 1ce8816f8fd..113381fd808 100644 --- a/azure/scope/machinepoolmachine_test.go +++ b/azure/scope/machinepoolmachine_test.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" v1beta1conditions "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" @@ -499,6 +500,132 @@ func getNotReadyNode() *corev1.Node { } } +func TestMachinePoolMachineScope_hasLatestModelApplied(t *testing.T) { + galleryID := "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/galleries/gallery/images/image/versions/1.0.0" + + cases := []struct { + name string + instanceImage infrav1.Image + specImage *infrav1.Image + expected bool + }{ + { + name: "spec uses raw ID matching compute gallery, instance has plan - should match", + specImage: &infrav1.Image{ + ID: ptr.To(galleryID), + }, + instanceImage: infrav1.Image{ + ComputeGallery: &infrav1.AzureComputeGalleryImage{ + Gallery: "gallery", + Name: "image", + Version: "1.0.0", + SubscriptionID: ptr.To("sub"), + ResourceGroup: ptr.To("rg"), + Plan: &infrav1.ImagePlan{ + Publisher: "publisher", + Offer: "product", + SKU: "sku", + }, + }, + }, + expected: true, + }, + { + name: "spec uses raw ID matching compute gallery, instance has no plan - should match", + specImage: &infrav1.Image{ + ID: ptr.To(galleryID), + }, + instanceImage: infrav1.Image{ + ComputeGallery: &infrav1.AzureComputeGalleryImage{ + Gallery: "gallery", + Name: "image", + Version: "1.0.0", + SubscriptionID: ptr.To("sub"), + ResourceGroup: ptr.To("rg"), + }, + }, + expected: true, + }, + { + name: "spec uses raw ID matching compute gallery, instance has different version - should not match", + specImage: &infrav1.Image{ + ID: ptr.To(galleryID), + }, + instanceImage: infrav1.Image{ + ComputeGallery: &infrav1.AzureComputeGalleryImage{ + Gallery: "gallery", + Name: "image", + Version: "2.0.0", + SubscriptionID: ptr.To("sub"), + ResourceGroup: ptr.To("rg"), + Plan: &infrav1.ImagePlan{ + Publisher: "publisher", + Offer: "product", + SKU: "sku", + }, + }, + }, + expected: false, + }, + { + name: "spec uses structured ComputeGallery with plan - should match", + specImage: &infrav1.Image{ + ComputeGallery: &infrav1.AzureComputeGalleryImage{ + Gallery: "gallery", + Name: "image", + Version: "1.0.0", + SubscriptionID: ptr.To("sub"), + ResourceGroup: ptr.To("rg"), + Plan: &infrav1.ImagePlan{ + Publisher: "publisher", + Offer: "product", + SKU: "sku", + }, + }, + }, + instanceImage: infrav1.Image{ + ComputeGallery: &infrav1.AzureComputeGalleryImage{ + Gallery: "gallery", + Name: "image", + Version: "1.0.0", + SubscriptionID: ptr.To("sub"), + ResourceGroup: ptr.To("rg"), + Plan: &infrav1.ImagePlan{ + Publisher: "publisher", + Offer: "product", + SKU: "sku", + }, + }, + }, + expected: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + g := NewWithT(t) + s := &MachinePoolMachineScope{ + instance: &azure.VMSSVM{ + Image: c.instanceImage, + }, + MachinePoolScope: &MachinePoolScope{ + AzureMachinePool: &infrav1exp.AzureMachinePool{ + Spec: infrav1exp.AzureMachinePoolSpec{ + Template: infrav1exp.AzureMachinePoolMachineTemplate{ + Image: c.specImage, + }, + }, + }, + }, + } + + result, err := s.hasLatestModelApplied(t.Context()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).To(Equal(c.expected)) + }) + } +} + // asserts whether a condition of type is set on the Getter object // when the condition is true, asserting the reason/severity/message // for the condition are avoided.