OCPBUGS-33508: capi/aws: fix setting custom AMI

When I introduced the AMI copying functionality, I inadvertently broke
the custom AMI logic, causing the code to always use the default RHCOS
AMI.

This PR fixes the issue and adds some comments to explain the AMI
selection logic which spans multiple files and is not easy to follow.

Author: r4f4

pkg/infrastructure/aws/clusterapi/ami.go

@@ -10,23 +10,15 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/openshift/installer/pkg/asset/installconfig"
+	"github.com/openshift/installer/pkg/asset/rhcos"
 )
 
 // copyAMIToRegion copies the AMI to the region configured in the installConfig if needed.
-func copyAMIToRegion(ctx context.Context, installConfig *installconfig.InstallConfig, infraID, rhcosImage string) (string, error) {
-	osImage := strings.SplitN(rhcosImage, ",", 2)
-	amiID := osImage[0]
-	amiRegion := installConfig.Config.AWS.Region
-	if len(osImage) > 1 {
-		amiRegion = osImage[1]
-	}
-
-	// Already in target region, nothing to do
-	if amiRegion == installConfig.Config.AWS.Region {
-		return amiID, nil
-	}
+func copyAMIToRegion(ctx context.Context, installConfig *installconfig.InstallConfig, infraID string, rhcosImage *rhcos.Image) (string, error) {
+	osImage := strings.SplitN(string(*rhcosImage), ",", 2)
+	amiID, amiRegion := osImage[0], osImage[1]
 
-	logrus.Infof("Copying AMI to region %s", installConfig.AWS.Region)
+	logrus.Infof("Copying AMI %s to region %s", amiID, installConfig.AWS.Region)
 
 	session, err := installConfig.AWS.Session(ctx)
 	if err != nil {

pkg/infrastructure/aws/clusterapi/aws.go

@@ -45,13 +45,29 @@ func (*Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInp
 		return fmt.Errorf("failed to create IAM roles: %w", err)
 	}
 
-	amiID, err := copyAMIToRegion(ctx, in.InstallConfig, in.InfraID, string(*in.RhcosImage))
+	// The AWSMachine manifests might already have the AMI ID set from the machine pool which takes into account the
+	// ways in which the AMI can be specified: the default rhcos if already in the target region, a custom AMI ID set in
+	// platform.aws.amiID, and a custom AMI ID specified in the controlPlane stanza. So we just get the value from the
+	// first awsmachine manifest we find, instead of duplicating all the inheriting logic here.
+	for i := range in.MachineManifests {
+		if awsMachine, ok := in.MachineManifests[i].(*capa.AWSMachine); ok {
+			// Default/custom AMI already in target region, nothing else to do
+			if ptr.Deref(awsMachine.Spec.AMI.ID, "") != "" {
+				return nil
+			}
+		}
+	}
+
+	// Notice that we have to use the default RHCOS value because we set the AMI.ID to empty if the default RHCOS is not
+	// in the target region and it needs to be copied over. See pkg/asset/machines/clusterapi.go
+	amiID, err := copyAMIToRegion(ctx, in.InstallConfig, in.InfraID, in.RhcosImage)
 	if err != nil {
 		return fmt.Errorf("failed to copy AMI: %w", err)
 	}
+	// Update manifests with the new ID
 	for i := range in.MachineManifests {
 		if awsMachine, ok := in.MachineManifests[i].(*capa.AWSMachine); ok {
-			awsMachine.Spec.AMI = capa.AMIReference{ID: ptr.To(amiID)}
+			awsMachine.Spec.AMI.ID = ptr.To(amiID)
 		}
 	}
 	return nil