Repository: openshift/assisted-service (language: python)

Issue:
MGMT-16813: Prioritize finding release image with exact CPU architecture
<!--
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

You can refer to [Kubernetes community documentation] on writing good commit messages, which provides good tips and ideas.

Some PRs address specific issues. Please, refer to the [CONTRIBUTING] documentation for more
information on how to link a PR to an existing issue.

It's recommended to take a few extra minutes to provide more information about
how this code was tested. Here are some questions that may be worth answering:

- Should this PR be tested by the reviewer?
- Is this PR relying on CI for an e2e test run?
- Should this PR be tested in a specific environment?
- Any logs, screenshots, etc that can help with the review process?

-->
https://issues.redhat.com/browse/MGMT-16813
Previously, the `getReleaseImageFromCache` would get all release
images that included the CPU architecture and return the first
instance it found that matched the desired Openshift version.

Since multi-arch images include the CPU architecture, it led
to undesireable selection of the multi-arch image over the
exact matching image with the specified CPU architecture.

This fix ensures that the release image will be found with
the following priority:
1. Exact CPU Architecture & OpenShift x.y.z version match
2. Exact CPU Architecture & OpenShift Major Minor (x.y) version match
3. Multi-arch CPU with specified CPU Architecture & OpenShift x.y.z version match
4. Multi-arch CPU with specified CPU Architecture & OpenShift Major Minor (x.y) version match

## List all the issues related to this PR

- [ ] New Feature <!-- new functionality -->
- [ ] Enhancement <!-- refactor, code changes, improvement, that won't add new features -->
- [x] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD <!-- Notice that changes for Dockerfiles/Jenkinsfiles aren't tested in CI due to a known bug. -->

## What environments does this code impact?

- [ ] Automation (CI, tools, etc)
- [ ] Cloud
- [x] Operator Managed Deployments
- [ ] None

## How was this code tested?

<!-- Please, select one or more if needed: -->

- [ ] assisted-test-infra environment
- [ ] dev-scripts environment
- [ ] Reviewer's test appreciated
- [x] Waiting for CI to do a full test run
- [ ] Manual (Elaborate on how it was tested)
- [ ] No tests needed

## Checklist

- [x] Title and description added to both, commit and PR.
- [x] Relevant issues have been associated (see [CONTRIBUTING] guide)
- [x] This change does not require a documentation update (docstring, `docs`, README, etc)
- [ ] Does this change include unit-tests (note that code changes require unit-tests)

## Reviewers Checklist

- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?

[Kubernetes community documentation]: https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#commit-message-guidelines
[CONTRIBUTING]: https://github.com/openshift/assisted-service/blob/master/CONTRIBUTING.md

**note**: will push up code gen once it finishes


Discussion / hints:
@CrystalChun: This pull request references [MGMT-16813](https://issues.redhat.com//browse/MGMT-16813) which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.16.0" version, but no target version was set.

<details>

In response to [this](https://github.com/openshift/assisted-service/pull/6066):

><!--
>Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
>
>You can refer to [Kubernetes community documentation] on writing good commit messages, which provides good tips and ideas.
>
>Some PRs address specific issues. Please, refer to the [CONTRIBUTING] documentation for more
>information on how to link a PR to an existing issue.
>
>It's recommended to take a few extra minutes to provide more information about
>how this code was tested. Here are some questions that may be worth answering:
>
>- Should this PR be tested by the reviewer?
>- Is this PR relying on CI for an e2e test run?
>- Should this PR be tested in a specific environment?
>- Any logs, screenshots, etc that can help with the review process?
>
>-->
>**Problem**: There have been instances where the `AgentClusterInstall` will specify to use a specific `ClusterImageSet`, but assisted will use a completely different image than the one specified.
>
>This is from the cluster deployment initially finding the release image, but only passing the release version and architecture to the `RegisterClusterInternal` function. 
>
>In the `RegisterClusterInternal` function, the assisted-service calls get release image again, but this time will choose it based off of its cache of images, which is not ordered, so it might choose a release image with the same corresponding version, but incorrect architecture. 
>
>**Proposed solution**: Allow the ClusterDeployment controller to pass in a release image that it already found via the `ClusterImageSet` to the `RegisterClusterInternal` function so that the cluster can actually use the release image that was specified. 
>
>## List all the issues related to this PR
>
>- [ ] New Feature <!-- new functionality -->
>- [ ] Enhancement <!-- refactor, code changes, improvement, that won't add new features -->
>- [x] Bug fix
>- [ ] Tests
>- [ ] Documentation
>- [ ] CI/CD <!-- Notice that changes for Dockerfiles/Jenkinsfiles aren't tested in CI due to a known bug. -->
>
>## What environments does this code impact?
>
>- [ ] Automation (CI, tools, etc)
>- [ ] Cloud
>- [x] Operator Managed Deployments
>- [ ] None
>
>## How was this code tested?
>
><!-- Please, select one or more if needed: -->
>
>- [ ] assisted-test-infra environment
>- [ ] dev-scripts environment
>- [ ] Reviewer's test appreciated
>- [x] Waiting for CI to do a full test run
>- [ ] Manual (Elaborate on how it was tested)
>- [ ] No tests needed
>
>## Checklist
>
>- [x] Title and description added to both, commit and PR.
>- [x] Relevant issues have been associated (see [CONTRIBUTING] guide)
>- [x] This change does not require a documentation update (docstring, `docs`, README, etc)
>- [ ] Does this change include unit-tests (note that code changes require unit-tests)
>
>## Reviewers Checklist
>
>- Are the title and description (in both PR and commit) meaningful and clear?
>- Is there a bug required (and linked) for this change?
>- Should this PR be backported?
>
>[Kubernetes community documentation]: https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#commit-message-guidelines
>[CONTRIBUTING]: https://github.com/openshift/assisted-service/blob/master/CONTRIBUTING.md
>
>**note**: will push up code gen once it finishes


Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fassisted-service).  If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
</details>
Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with `/test all`

Fix patch:
diff --git a/internal/versions/versions.go b/internal/versions/versions.go
index 9f6d55fec89..b1dfa947333 100644
--- a/internal/versions/versions.go
+++ b/internal/versions/versions.go
@@ -166,50 +166,80 @@ func (h *handler) GetReleaseImageByURL(ctx context.Context, url, pullSecret stri
 	return h.addReleaseImage(url, pullSecret)
 }
 
+// Finds a release image from the cache that matches the specified version and architecture in the following
+// priority:
+// 1. Exact CPU Architecture & OpenShift x.y.z version match
+// 2. Exact CPU Architecture & OpenShift Major Minor (x.y) version match
+// 3. Multi-arch CPU with specified CPU Architecture & OpenShift x.y.z version match
+// 4. Multi-arch CPU with specified CPU Architecture & OpenShift Major Minor (x.y) version match
 func (h *handler) getReleaseImageFromCache(openshiftVersion, cpuArchitecture string) (*models.ReleaseImage, error) {
 	if cpuArchitecture == "" {
 		// Empty implies default CPU architecture
 		cpuArchitecture = common.DefaultCPUArchitecture
 	}
+
 	// Filter Release images by specified CPU architecture.
-	releaseImages := funk.Filter(h.releaseImages, func(releaseImage *models.ReleaseImage) bool {
-		for _, arch := range releaseImage.CPUArchitectures {
-			if arch == cpuArchitecture {
-				return true
+	exactCPUArchReleaseImages := funk.Filter(h.releaseImages, func(releaseImage *models.ReleaseImage) bool {
+		return swag.StringValue(releaseImage.CPUArchitecture) == cpuArchitecture
+	})
+
+	// Filter Release images by multi-arch images containing the specified CPU architecture
+	multiArchReleaseImages := funk.Filter(h.releaseImages, func(releaseImage *models.ReleaseImage) bool {
+		if *releaseImage.CPUArchitecture == common.MultiCPUArchitecture {
+			for _, arch := range releaseImage.CPUArchitectures {
+				if arch == cpuArchitecture {
+					return true
+				}
 			}
 		}
-		return swag.StringValue(releaseImage.CPUArchitecture) == cpuArchitecture
+		return false
 	})
-	if funk.IsEmpty(releaseImages) {
+
+	if funk.IsEmpty(exactCPUArchReleaseImages) && funk.IsEmpty(multiArchReleaseImages) {
 		return nil, errors.Errorf("The requested CPU architecture (%s) isn't specified in release images list", cpuArchitecture)
 	}
+
+	releaseImage := getReleaseImageByVersion(openshiftVersion, exactCPUArchReleaseImages.([]*models.ReleaseImage))
+	if releaseImage == nil {
+		// Find release image from multi-arch list if one doesn't exist in the exact CPU Arch list
+		releaseImage = getReleaseImageByVersion(openshiftVersion, multiArchReleaseImages.([]*models.ReleaseImage))
+	}
+
+	if releaseImage != nil {
+		return releaseImage, nil
+	}
+
+	return nil, errors.Errorf(
+		"The requested release image for version (%s) and CPU architecture (%s) isn't specified in release images list",
+		openshiftVersion, cpuArchitecture)
+}
+
+func getReleaseImageByVersion(desiredOpenshiftVersion string, releaseImages []*models.ReleaseImage) *models.ReleaseImage {
 	// Search for specified x.y.z openshift version
 	releaseImage := funk.Find(releaseImages, func(releaseImage *models.ReleaseImage) bool {
-		return *releaseImage.OpenshiftVersion == openshiftVersion
+		return *releaseImage.OpenshiftVersion == desiredOpenshiftVersion
 	})
 
 	if releaseImage == nil {
 		// Fallback to x.y version
-		versionKey, err := toMajorMinor(openshiftVersion)
+		majorMinorVersion, err := toMajorMinor(desiredOpenshiftVersion)
 		if err != nil {
-			return nil, err
+			return nil
 		}
+
 		releaseImage = funk.Find(releaseImages, func(releaseImage *models.ReleaseImage) bool {
 			// Starting from OCP 4.12 multi-arch release images do not have "-multi" suffix
 			// reported by the CVO, but we still offer the sufix internally to allow for explicit
 			// selection or single- or multi-arch payload.
 			version := strings.TrimSuffix(*releaseImage.OpenshiftVersion, "-multi")
-			return version == versionKey
+			return version == majorMinorVersion
 		})
 	}
 
 	if releaseImage != nil {
-		return releaseImage.(*models.ReleaseImage), nil
+		return releaseImage.(*models.ReleaseImage)
 	}
-
-	return nil, errors.Errorf(
-		"The requested release image for version (%s) and CPU architecture (%s) isn't specified in release images list",
-		openshiftVersion, cpuArchitecture)
+	return nil
 }
 
 // ValidateReleaseImageForRHCOS validates whether for a specified RHCOS version we have an OCP


Test patch:
diff --git a/internal/versions/versions_test.go b/internal/versions/versions_test.go
index 4532096fafb..0ee429f2dff 100644
--- a/internal/versions/versions_test.go
+++ b/internal/versions/versions_test.go
@@ -210,6 +210,13 @@ var _ = Describe("GetReleaseImage", func() {
 		Expect(*releaseImage.Version).Should(Equal("4.9-candidate"))
 	})
 
+	It("fetch release image by major.minor if given X.Y.Z version", func() {
+		releaseImage, err := h.GetReleaseImage(ctx, "4.9.2", common.DefaultCPUArchitecture, pullSecret)
+		Expect(err).ShouldNot(HaveOccurred())
+		Expect(*releaseImage.OpenshiftVersion).Should(Equal("4.9"))
+		Expect(*releaseImage.Version).Should(Equal("4.9-candidate"))
+	})
+
 	It("get from ReleaseImages", func() {
 		for _, key := range osImages.GetOpenshiftVersions() {
 			for _, architecture := range osImages.GetCPUArchitectures(key) {
@@ -270,6 +277,56 @@ var _ = Describe("GetReleaseImage", func() {
 		Expect(*releaseImage.Version).Should(Equal("4.12.999-rc.4"))
 	})
 
+	It("returns the matching CPU architecture over multi-arch if it is present", func() {
+		h.releaseImages = models.ReleaseImages{
+			&models.ReleaseImage{
+				CPUArchitecture:  swag.String(common.MultiCPUArchitecture),
+				CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture, common.PowerCPUArchitecture},
+				OpenshiftVersion: swag.String("4.12-multi"),
+				URL:              swag.String("release_4.12.999-multi"),
+				Version:          swag.String("4.12.999"),
+			},
+			&models.ReleaseImage{
+				CPUArchitecture:  swag.String(common.X86CPUArchitecture),
+				CPUArchitectures: []string{common.X86CPUArchitecture},
+				OpenshiftVersion: swag.String("4.12"),
+				URL:              swag.String("release_4.12.999-x86_64"),
+				Version:          swag.String("4.12.999"),
+			},
+		}
+
+		releaseImage, err := h.GetReleaseImage(ctx, "4.12.999", common.X86CPUArchitecture, pullSecret)
+		Expect(err).ShouldNot(HaveOccurred())
+		Expect(*releaseImage.OpenshiftVersion).Should(Equal("4.12"))
+		Expect(*releaseImage.Version).Should(Equal("4.12.999"))
+		Expect(*releaseImage.CPUArchitecture).Should(Equal(common.X86CPUArchitecture))
+	})
+
+	It("returns the multi-arch image if matching CPU architecture is not present", func() {
+		h.releaseImages = models.ReleaseImages{
+			&models.ReleaseImage{
+				CPUArchitecture:  swag.String(common.MultiCPUArchitecture),
+				CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture, common.PowerCPUArchitecture},
+				OpenshiftVersion: swag.String("4.12-multi"),
+				URL:              swag.String("release_4.12.999-multi"),
+				Version:          swag.String("4.12.999"),
+			},
+			&models.ReleaseImage{
+				CPUArchitecture:  swag.String(common.X86CPUArchitecture),
+				CPUArchitectures: []string{common.X86CPUArchitecture},
+				OpenshiftVersion: swag.String("4.13"),
+				URL:              swag.String("release_4.13.999-x86_64"),
+				Version:          swag.String("4.13.999"),
+			},
+		}
+
+		releaseImage, err := h.GetReleaseImage(ctx, "4.12.999", common.X86CPUArchitecture, pullSecret)
+		Expect(err).ShouldNot(HaveOccurred())
+		Expect(*releaseImage.OpenshiftVersion).Should(Equal("4.12-multi"))
+		Expect(*releaseImage.Version).Should(Equal("4.12.999"))
+		Expect(*releaseImage.CPUArchitecture).Should(Equal(common.MultiCPUArchitecture))
+	})
+
 	Context("with a kube client", func() {
 		var (
 			client client.Client
diff --git a/subsystem/operators_test.go b/subsystem/operators_test.go
index 407b36c28d5..716bd056ead 100644
--- a/subsystem/operators_test.go
+++ b/subsystem/operators_test.go
@@ -314,7 +314,7 @@ var _ = Describe("Operators endpoint tests", func() {
 				swag.String(models.ClusterCPUArchitectureArm64),
 				nil,
 			)
-			Expect(cluster.Payload.CPUArchitecture).To(Equal(models.ClusterCPUArchitectureMulti))
+			Expect(cluster.Payload.CPUArchitecture).To(Equal(models.ClusterCPUArchitectureArm64))
 			Expect(len(cluster.Payload.MonitoredOperators)).To(Equal(1))
 
 			infraEnvParams := installer.RegisterInfraEnvParams{