CAPZ upgrade test is failing since migration to Compute Gallery default images

[nojnhuh]

Which jobs are failing:

https://testgrid.k8s.io/sig-cluster-lifecycle-cluster-api-provider-azure#capz-pr-apiversion-upgrade-main

Which tests are failing:

Since when has it been failing:

Testgrid link:

Reason for failure (if possible):

Likely caused by #5167

It seems like the machines are using the default image implicitly, and the upgrade is changing the implicit default and therefore the images on those existing machines, causing them to re-roll nodes, which the test is verifying doesn't occur.

Is this failure isolated to Machines owned by MachinePools?

Anything else we need to know:

/kind failing-test

[One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels]

[nojnhuh]

Some prior discussion in #4575 (comment)

/assign @willie-yao

[willie-yao]

It looks like there's some trouble upgrading nodes from 22.04 to 24.04. Seems like it works but the node gets replaced after the cluster is ready. This might be something we can manually fix in the upgrade template so it also upgrades the Ubuntu version.

I'm testing upgrading to 22.04 first to see if that's the problem in #5228

[willie-yao]

Okay I think the fix is to publish 24.04 marketplace images for the older CAPI versions. Is that something that can be done? @mboersma

[mboersma]

No, we can no longer publish to the Azure Marketplace, only the community gallery.

[willie-yao]

Gotcha. I'm not super familiar with how the image publishing process works so correct me if I'm wrong, but is the upgrade test going to be stuck on 22.04? And if so, do we create 22.04 images for the newer CAPI versions?

[nojnhuh]

Are we sure Ubuntu version is the only thing triggering the rollout? Based on my read of this, it seems like any migration from a reference to a Marketplace image to a Community Gallery one will cause CAPZ to rollout changes, even if the images are functionally identical:

cluster-api-provider-azure/azure/types.go

Lines 125 to 132 in bcca2e2

	// HasModelChanges returns true if the spec fields which will mutate the Azure VMSS model are different.
	func (vmss VMSS) HasModelChanges(other VMSS) bool {
	equal := cmp.Equal(vmss.Image, other.Image) &&
	cmp.Equal(vmss.Identity, other.Identity) &&
	cmp.Equal(vmss.Zones, other.Zones) &&
	cmp.Equal(vmss.Sku, other.Sku)
	return !equal
	}

Is this failure isolated to Machines owned by MachinePools?

Have we figured this out yet? I'm wondering why we wouldn't have hit this when we've modified the default image before, like in #2807. Maybe we need to add back some of the logic from before to say "if you're using k8s 1.30.3, you get a Marketplace image. If you're using 1.30.5, you get a Community Gallery image"?

[mboersma]

Maybe we need to add back some of the logic from before to say "if you're using k8s 1.30.3, you get a Marketplace image. If you're using 1.30.5, you get a Community Gallery image"?

I think that's the only way to fulfill the expectations of the clusterctl upgrade test. I was hoping to jettison all that funky logic when moving to community gallery as default, but hadn't thought of this case (and failed to run the job on that PR evidently). I can take a look at restoring what's needed.

[mboersma]

Based on what's actually available in the Marketplace offers, I think we could add some logic in the GetDefaultXXXImage funcs that would return a Marketplace offer for windows-2019 or ubuntu-2204 in this range:

// Use the Azure Marketplace for specific older versions, to keep clusterctl upgrade from rolling new machines.
marketplaceVersions := "1.27.16 || >=1.28.1 <=1.28.12 || >=1.29.1 <=1.29.7 || >=1.30.0 <=1.30.3"
marketplaceVersionRange := semver.MustParseRange(marketplaceVersions)
if marketplaceVersionRange(v) {
	return &infrav1.Image{
		Marketplace: &infrav1.AzureMarketplaceImage{
			ImagePlan: infrav1.ImagePlan{
				Publisher: publisher,
				Offer:     offer,
				SKU:       skuID,
			},
			Version: v.String(),
	}, nil
}
// otherwise return default community gallery image spec

We would need to update several unit tests' expectations to match.