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

Issue:
MGMT-17503: Fix the cpu architectures retrieved in ../openshift-versions endpoint
Currently when requesting the latest openshift versions at `../openshift-versions?only_latest=true`, you get at most one CPU architecture in the list. This PR fixes that issue.

<!--
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?

-->

## 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)
- [x] Cloud
- [ ] Operator Managed Deployments
- [ ] None

## How was this code tested?

<!-- Please, select one or more if needed: -->

- [x] 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)
- [x] 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



Discussion / hints:

Fix patch:
diff --git a/internal/common/version.go b/internal/common/version.go
index 3f167787f15..4b3ebc07bd7 100644
--- a/internal/common/version.go
+++ b/internal/common/version.go
@@ -69,6 +69,18 @@ func BaseVersionEqual(version1, versionMayEqual string) (bool, error) {
 	return *majorMinorVersion1 == *majorMinorVersionMayEqual, nil
 }
 
+func MajorMinorPatchEqual(firstVersion, secondVersion string) (*bool, error) {
+	if GetVersionFormat(firstVersion) != MajorMinorPatchVersion {
+		return nil, fmt.Errorf("version '%s' is not in the form of major.minor.patch or major.minor.patch-<prerelease>", firstVersion)
+	}
+
+	if GetVersionFormat(secondVersion) != MajorMinorPatchVersion {
+		return nil, fmt.Errorf("version '%s' is not in the form of major.minor.patch or major.minor.patch-<prerelease>", secondVersion)
+	}
+
+	return swag.Bool(strings.Split(firstVersion, "-")[0] == strings.Split(secondVersion, "-")[0]), nil
+}
+
 func GetMajorMinorVersion(version string) (*string, error) {
 	version = strings.Split(version, "-")[0]
 	splittedVersion := strings.Split(version, ".")
diff --git a/internal/versions/api.go b/internal/versions/api.go
index 56fd02ce5c2..c5a12d4af46 100644
--- a/internal/versions/api.go
+++ b/internal/versions/api.go
@@ -14,7 +14,6 @@ import (
 	"github.com/openshift/assisted-service/restapi"
 	operations "github.com/openshift/assisted-service/restapi/operations/versions"
 	"github.com/pkg/errors"
-	"github.com/samber/lo"
 	"github.com/sirupsen/logrus"
 	"github.com/thoas/go-funk"
 )
@@ -77,62 +76,73 @@ func GetModelVersions(v Versions) models.Versions {
 	}
 }
 
-// getLatestReleaseImageForMajorMinor returns the latest release image matching a given major.minor version,
+// getLatestReleaseImagesForMajorMinor returns the latest release images (all the CPU architectures) matching a given major.minor version,
 // or error if none of the release images match. The latest release image is considered the latest none beta release image,
 // or if all matching release images are beta then just the latest.
-func getLatestReleaseImageForMajorMinor(releaseImages models.ReleaseImages, majorMinorVersion string, log logrus.FieldLogger) (*models.ReleaseImage, error) {
-	var latestReleaseImage *models.ReleaseImage
+func getLatestReleaseImagesForMajorMinor(releaseImages models.ReleaseImages, majorMinorVersion string, log logrus.FieldLogger) (models.ReleaseImages, error) {
+	var latestReleaseImages models.ReleaseImages
 	for _, releaseImage := range releaseImages {
 		// Typically, releaseImage.OpenshiftVersion follow the major.minor format, though exceptions exist with static release images.
-		currentMajorMinorVersion, err := common.GetMajorMinorVersion(*releaseImage.OpenshiftVersion)
+		// We want to consider only release images matching the given major.minor version.
+		isMajorMinorEqual, err := common.BaseVersionEqual(*releaseImage.Version, majorMinorVersion)
 		if err != nil {
 			return nil, err
 		}
 
-		if *currentMajorMinorVersion != majorMinorVersion {
+		if !isMajorMinorEqual {
 			continue
 		}
 
-		if latestReleaseImage == nil {
-			latestReleaseImage = releaseImage
+		if latestReleaseImages == nil {
+			latestReleaseImages = models.ReleaseImages{releaseImage}
 		}
 
-		isNewest, err := common.BaseVersionLessThan(*releaseImage.Version, *latestReleaseImage.Version)
+		// If it is the same version but different CPU architecture, add the atchitecture to the list.
+		isEqual, err := common.MajorMinorPatchEqual(*releaseImage.Version, *latestReleaseImages[0].Version)
+		if err != nil {
+			return nil, err
+		}
+		if *isEqual && !funk.Contains(latestReleaseImages, releaseImage) {
+			latestReleaseImages = append(latestReleaseImages, releaseImage)
+			continue
+		}
+
+		isNewest, err := common.BaseVersionLessThan(*releaseImage.Version, *latestReleaseImages[0].Version)
 		if err != nil {
 			return nil, err
 		}
 
 		// none-beta > beta, later-beta > beta
-		if latestReleaseImage.SupportLevel == models.OpenshiftVersionSupportLevelBeta {
+		if latestReleaseImages[0].SupportLevel == models.OpenshiftVersionSupportLevelBeta {
 			if isNewest || releaseImage.SupportLevel != models.OpenshiftVersionSupportLevelBeta {
-				latestReleaseImage = releaseImage
+				latestReleaseImages = models.ReleaseImages{releaseImage}
 			}
 		} else { // non-beta-later > non-beta
 			if isNewest && releaseImage.SupportLevel != models.OpenshiftVersionSupportLevelBeta {
-				latestReleaseImage = releaseImage
+				latestReleaseImages = models.ReleaseImages{releaseImage}
 			}
 		}
-	}
-
-	log.Debugf("Found latest release image version '%s' for majorMinorVersion '%s'", *latestReleaseImage.Version, majorMinorVersion)
-
-	return latestReleaseImage, nil
-}
 
-// getMultiArchReleaseImage retrieves multi arch release image matching a given x.y.z version.
-func getMultiArchReleaseImage(releaseImages models.ReleaseImages, version string, log logrus.FieldLogger) *models.ReleaseImage {
-	if !strings.HasSuffix(version, "-multi") {
-		version = fmt.Sprintf("%s-multi", version)
 	}
 
-	if releaseImage, ok := lo.Find(releaseImages, func(releaseImage *models.ReleaseImage) bool {
-		return swag.StringValue(releaseImage.Version) == version
-	}); ok {
-		log.Debugf("Found multi-arch release image for version '%s'", version)
-		return releaseImage
+	if len(latestReleaseImages) == 0 {
+		return nil, errors.Errorf("No release image found for version '%s'", majorMinorVersion)
 	}
 
-	return nil
+	cpuArchitecturesLiteral := funk.Reduce(latestReleaseImages, func(agg string, releaseImage *models.ReleaseImage) string {
+		if agg == "" {
+			return *releaseImage.CPUArchitecture
+		}
+		return fmt.Sprintf("%s, %s", agg, *releaseImage.CPUArchitecture)
+	}, "").(string)
+	log.Debugf(
+		"Found latest release images version '%s' and CPU architectures '%s' for majorMinorVersion '%s'",
+		*latestReleaseImages[0].Version,
+		cpuArchitecturesLiteral,
+		majorMinorVersion,
+	)
+
+	return latestReleaseImages, nil
 }
 
 func filterReleaseImagesByOnlyLatest(releaseImages models.ReleaseImages, onlyLatest *bool, log logrus.FieldLogger) (models.ReleaseImages, error) {
@@ -154,16 +164,11 @@ func filterReleaseImagesByOnlyLatest(releaseImages models.ReleaseImages, onlyLat
 
 	filteredReleaseImages := models.ReleaseImages{}
 	for majorMinorVersion := range releaseImagesMajorMinorVersionsSet {
-		latestReleaseImageForMajorMinor, err := getLatestReleaseImageForMajorMinor(releaseImages, majorMinorVersion, log)
+		latestReleaseImagesForMajorMinor, err := getLatestReleaseImagesForMajorMinor(releaseImages, majorMinorVersion, log)
 		if err != nil {
-			return nil, errors.Wrapf(err, "error occurred while trying to get the latest release image for version: %s", majorMinorVersion)
-		}
-		filteredReleaseImages = append(filteredReleaseImages, latestReleaseImageForMajorMinor)
-		// Add the matching multi-arch release image if exists as well
-		matchingMultiArchReleaseImage := getMultiArchReleaseImage(releaseImages, *latestReleaseImageForMajorMinor.Version, log)
-		if matchingMultiArchReleaseImage != nil {
-			filteredReleaseImages = append(filteredReleaseImages, matchingMultiArchReleaseImage)
+			return nil, errors.Wrapf(err, "error occurred while trying to get the latest release images for version: %s", majorMinorVersion)
 		}
+		filteredReleaseImages = append(filteredReleaseImages, latestReleaseImagesForMajorMinor...)
 	}
 
 	return filteredReleaseImages, nil


Test patch:
diff --git a/internal/common/version_test.go b/internal/common/version_test.go
index 2c55792736a..81e760e666c 100644
--- a/internal/common/version_test.go
+++ b/internal/common/version_test.go
@@ -143,3 +143,16 @@ var _ = DescribeTable("GetVersionFormat", func(input string, expectedVersionForm
 	Entry("empty version", "", NoneVersion),
 	Entry("non-version string", "non-version", NoneVersion),
 )
+
+var _ = DescribeTable("MajorMinorPatchEqual", func(firstVersion string, secondVersion string, ExpectedResult *bool) {
+	isEqual, _ := MajorMinorPatchEqual(firstVersion, secondVersion)
+	Expect(isEqual).To(Equal(ExpectedResult))
+},
+	Entry("two equal x.y.z versions", "4.11.0", "4.11.0", swag.Bool(true)),
+	Entry("two not unequal x.y.z versions", "4.11.0", "4.11.1", swag.Bool(false)),
+	Entry("one x.y.z version and one x.y.z-something version, equal", "4.11.0", "4.11.0-alpha", swag.Bool(true)),
+	Entry("one x.y.z version and one x.y.z-something version, unequal", "4.11.0", "4.12.0-alpha", swag.Bool(false)),
+	Entry("majorMinorVersions", "4.11", "4.11", nil),
+	Entry("majorVersions", "4", "4", nil),
+	Entry("malformed versions", "foobar", "foobar", nil),
+)
diff --git a/internal/versions/api_test.go b/internal/versions/api_test.go
index 31b8a38a66e..eabb270a5ad 100644
--- a/internal/versions/api_test.go
+++ b/internal/versions/api_test.go
@@ -482,6 +482,14 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				Version:          swag.String("4.11.1"),
 				SupportLevel:     models.ReleaseImageSupportLevelProduction,
 			},
+			{
+				CPUArchitecture:  swag.String(common.ARM64CPUArchitecture),
+				CPUArchitectures: []string{common.ARM64CPUArchitecture},
+				OpenshiftVersion: swag.String("4.11"),
+				URL:              swag.String("quay.io/openshift-release-dev/ocp-release:4.11.1-aarch64"),
+				Version:          swag.String("4.11.1"),
+				SupportLevel:     models.ReleaseImageSupportLevelProduction,
+			},
 			{
 				CPUArchitecture:  swag.String(common.X86CPUArchitecture),
 				CPUArchitectures: []string{common.X86CPUArchitecture},
@@ -506,6 +514,14 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				Version:          swag.String("4.12.1"),
 				SupportLevel:     models.ReleaseImageSupportLevelProduction,
 			},
+			{
+				CPUArchitecture:  swag.String(common.ARM64CPUArchitecture),
+				CPUArchitectures: []string{common.ARM64CPUArchitecture},
+				OpenshiftVersion: swag.String("4.12"),
+				URL:              swag.String("quay.io/openshift-release-dev/ocp-release:4.12.1-aarch64"),
+				Version:          swag.String("4.12.1"),
+				SupportLevel:     models.ReleaseImageSupportLevelProduction,
+			},
 			{
 				CPUArchitecture:  swag.String(common.X86CPUArchitecture),
 				CPUArchitectures: []string{common.X86CPUArchitecture},
@@ -522,6 +538,14 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				Version:          swag.String("4.13.1"),
 				SupportLevel:     models.OpenshiftVersionSupportLevelBeta,
 			},
+			{
+				CPUArchitecture:  swag.String(common.ARM64CPUArchitecture),
+				CPUArchitectures: []string{common.ARM64CPUArchitecture},
+				OpenshiftVersion: swag.String("4.13"),
+				URL:              swag.String("quay.io/openshift-release-dev/ocp-release:4.13.1-aarch64"),
+				Version:          swag.String("4.13.1"),
+				SupportLevel:     models.OpenshiftVersionSupportLevelBeta,
+			},
 			{
 				CPUArchitecture:  swag.String(common.X86CPUArchitecture),
 				CPUArchitectures: []string{common.X86CPUArchitecture},
@@ -530,6 +554,14 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				Version:          swag.String("4.13.2"),
 				SupportLevel:     models.OpenshiftVersionSupportLevelBeta,
 			},
+			{
+				CPUArchitecture:  swag.String(common.ARM64CPUArchitecture),
+				CPUArchitectures: []string{common.ARM64CPUArchitecture},
+				OpenshiftVersion: swag.String("4.13"),
+				URL:              swag.String("quay.io/openshift-release-dev/ocp-release:4.13.2-aarch64"),
+				Version:          swag.String("4.13.2"),
+				SupportLevel:     models.OpenshiftVersionSupportLevelBeta,
+			},
 			{
 				CPUArchitecture:  swag.String(common.MultiCPUArchitecture),
 				CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture},
@@ -559,6 +591,12 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				URL:              swag.String("https://mirror.openshift.com/pub/openshift-v4/x86_64/dependencies/rhcos/4.12/4.12.48/rhcos-4.12.48-x86_64-live.x86_64.iso"),
 				Version:          swag.String("412.86.202308081056-0"),
 			},
+			{
+				OpenshiftVersion: swag.String("4.12"),
+				CPUArchitecture:  swag.String(common.ARM64CPUArchitecture),
+				URL:              swag.String("https://mirror.openshift.com/pub/openshift-v4/aarch64/dependencies/rhcos/4.12/4.12.48/rhcos-4.12.48-aarch64-live.aarch64.iso"),
+				Version:          swag.String("412.86.202308081056-0"),
+			},
 			{
 				OpenshiftVersion: swag.String("4.13"),
 				CPUArchitecture:  swag.String(common.X86CPUArchitecture),
@@ -588,7 +626,7 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 			expectedPayload := models.OpenshiftVersions{
 				"4.11.1": {
 					DisplayName:      swag.String("4.11.1"),
-					CPUArchitectures: []string{common.X86CPUArchitecture},
+					CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture},
 					SupportLevel:     swag.String(models.ReleaseImageSupportLevelProduction),
 					Default:          false,
 				},
@@ -606,7 +644,7 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				},
 				"4.12.1": {
 					DisplayName:      swag.String("4.12.1"),
-					CPUArchitectures: []string{common.X86CPUArchitecture},
+					CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture},
 					SupportLevel:     swag.String(models.ReleaseImageSupportLevelProduction),
 					Default:          false,
 				},
@@ -618,13 +656,13 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				},
 				"4.13.1": {
 					DisplayName:      swag.String("4.13.1"),
-					CPUArchitectures: []string{common.X86CPUArchitecture},
+					CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture},
 					SupportLevel:     swag.String(models.OpenshiftVersionSupportLevelBeta),
 					Default:          false,
 				},
 				"4.13.2": {
 					DisplayName:      swag.String("4.13.2"),
-					CPUArchitectures: []string{common.X86CPUArchitecture},
+					CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture},
 					SupportLevel:     swag.String(models.OpenshiftVersionSupportLevelBeta),
 					Default:          false,
 				},
@@ -660,13 +698,13 @@ var _ = Describe("V2ListSupportedOpenshiftVersions", func() {
 				},
 				"4.12.1": {
 					DisplayName:      swag.String("4.12.1"),
-					CPUArchitectures: []string{common.X86CPUArchitecture},
+					CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture},
 					SupportLevel:     swag.String(models.ReleaseImageSupportLevelProduction),
 					Default:          false,
 				},
 				"4.13.2": {
 					DisplayName:      swag.String("4.13.2"),
-					CPUArchitectures: []string{common.X86CPUArchitecture},
+					CPUArchitectures: []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture},
 					SupportLevel:     swag.String(models.OpenshiftVersionSupportLevelBeta),
 					Default:          false,
 				},