Merge pull request openshift#7349 from jeffdyoung/OCPBUGS-15941

OCPBUGS-15941: ABI - Validate release image arch, add cpu_architectures to RELEASE_IMAGES

Author: openshift-merge-bot[bot]

pkg/asset/agent/common.go

@@ -1,6 +1,8 @@
 package agent
 
 import (
+	"github.com/sirupsen/logrus"
+
 	hiveext "github.com/openshift/assisted-service/api/hiveextension/v1beta1"
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/baremetal"
@@ -56,3 +58,25 @@ func IsSupportedPlatform(platform hiveext.PlatformType) bool {
 	}
 	return false
 }
+
+// DetermineReleaseImageArch returns the arch of the release image.
+func DetermineReleaseImageArch(pullSecret, pullSpec string) (string, error) {
+	templateFilter := "-o=go-template={{if and .metadata.metadata (index . \"metadata\" \"metadata\" \"release.openshift.io/architecture\")}}{{index . \"metadata\" \"metadata\" \"release.openshift.io/architecture\"}}{{else}}{{.config.architecture}}{{end}}"
+
+	var getReleaseArch = []string{
+		"oc",
+		"adm",
+		"release",
+		"info",
+		pullSpec,
+		templateFilter,
+	}
+
+	releaseArch, err := ExecuteOC(pullSecret, getReleaseArch)
+	if err != nil {
+		logrus.Errorf("Release Image arch could not be found: %s", err)
+		return "", err
+	}
+	logrus.Debugf("Release Image arch is: %s", releaseArch)
+	return releaseArch, nil
+}

pkg/asset/agent/image/ignition.go

@@ -21,6 +21,7 @@ import (
 	"github.com/openshift/assisted-service/api/v1beta1"
 	"github.com/openshift/assisted-service/models"
 	"github.com/openshift/installer/pkg/asset"
+	agentcommon "github.com/openshift/installer/pkg/asset/agent"
 	"github.com/openshift/installer/pkg/asset/agent/agentconfig"
 	"github.com/openshift/installer/pkg/asset/agent/manifests"
 	"github.com/openshift/installer/pkg/asset/agent/mirror"
@@ -135,8 +136,20 @@ func (a *Ignition) Generate(dependencies asset.Parents) error {
 	if infraEnv.Spec.CpuArchitecture != "" {
 		archName = infraEnv.Spec.CpuArchitecture
 	}
-
-	releaseImageList, err := releaseImageList(agentManifests.ClusterImageSet.Spec.ReleaseImage, archName)
+	// Examine the release payload to see if its multi
+	releaseArch, err := agentcommon.DetermineReleaseImageArch(agentManifests.GetPullSecretData(), agentManifests.ClusterImageSet.Spec.ReleaseImage)
+	if err != nil {
+		logrus.Warnf("Unable to validate the release image architecture, using infraEnv.Spec.CpuArchitecture for the release image arch")
+		releaseArch = archName
+	} else {
+		releaseArch = arch.RpmArch(releaseArch)
+		logrus.Debugf("Found Release Image Architecture: %s", releaseArch)
+	}
+	releaseArchs := []string{releaseArch}
+	if releaseArch == "multi" {
+		releaseArchs = []string{arch.RpmArch(types.ArchitectureARM64), arch.RpmArch(types.ArchitectureAMD64), arch.RpmArch(types.ArchitecturePPC64LE), arch.RpmArch(types.ArchitectureS390X)}
+	}
+	releaseImageList, err := releaseImageList(agentManifests.ClusterImageSet.Spec.ReleaseImage, releaseArch, releaseArchs)
 	if err != nil {
 		return err
 	}

pkg/asset/agent/image/ignition_test.go

@@ -65,7 +65,7 @@ func TestIgnition_getTemplateData(t *testing.T) {
 	haveMirrorConfig := true
 	publicContainerRegistries := "quay.io,registry.ci.openshift.org"
 
-	releaseImageList, err := releaseImageList(clusterImageSet.Spec.ReleaseImage, "x86_64")
+	releaseImageList, err := releaseImageList(clusterImageSet.Spec.ReleaseImage, "x86_64", []string{"86_64"})
 	assert.NoError(t, err)
 
 	arch := "x86_64"

pkg/asset/agent/image/oc.go pkg/asset/agent/image/releaseextract.gopkg/asset/agent/image/oc.go renamed to pkg/asset/agent/image/releaseextract.go

@@ -24,6 +24,7 @@ import (
 	"sigs.k8s.io/yaml"
 
 	operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1"
+	"github.com/openshift/installer/pkg/asset/agent"
 	"github.com/openshift/installer/pkg/asset/agent/mirror"
 	"github.com/openshift/installer/pkg/rhcos"
 	"github.com/openshift/installer/pkg/rhcos/cache"
@@ -34,19 +35,19 @@ const (
 	coreOsFileName       = "/coreos/coreos-%s.iso"
 	coreOsSha256FileName = "/coreos/coreos-%s.iso.sha256"
 	coreOsStreamFileName = "/coreos/coreos-stream.json"
-	//OcDefaultTries is the number of times to execute the oc command on failues
+	// OcDefaultTries is the number of times to execute the oc command on failures.
 	OcDefaultTries = 5
-	// OcDefaultRetryDelay is the time between retries
+	// OcDefaultRetryDelay is the time between retries.
 	OcDefaultRetryDelay = time.Second * 5
 )
 
-// Config is used to set up the retries for extracting the base ISO
+// Config is used to set up the retries for extracting the base ISO.
 type Config struct {
 	MaxTries   uint
 	RetryDelay time.Duration
 }
 
-// Release is the interface to use the oc command to the get image info
+// Release is the interface to use the oc command to the get image info.
 type Release interface {
 	GetBaseIso(architecture string) (string, error)
 	GetBaseIsoVersion(architecture string) (string, error)
@@ -60,7 +61,7 @@ type release struct {
 	mirrorConfig []mirror.RegistriesConfig
 }
 
-// NewRelease is used to set up the executor to run oc commands
+// NewRelease is used to set up the executor to run oc commands.
 func NewRelease(config Config, releaseImage string, pullSecret string, mirrorConfig []mirror.RegistriesConfig) Release {
 	return &release{
 		config:       config,
@@ -70,13 +71,6 @@ func NewRelease(config Config, releaseImage string, pullSecret string, mirrorCon
 	}
 }
 
-const (
-	templateGetImage             = "oc adm release info --image-for=%s --filter-by-os=linux/%s --insecure=%t %s"
-	templateGetImageWithIcsp     = "oc adm release info --image-for=%s --filter-by-os=linux/%s --insecure=%t --icsp-file=%s %s"
-	templateImageExtract         = "oc image extract --path %s:%s --filter-by-os=linux/%s --confirm %s"
-	templateImageExtractWithIcsp = "oc image extract --path %s:%s --filter-by-os=linux/%s --confirm --icsp-file=%s %s"
-)
-
 // ExtractFile extracts the specified file from the given image name, and store it in the cache dir.
 func (r *release) ExtractFile(image string, filename string, architecture string) ([]string, error) {
 	imagePullSpec, err := r.getImageFromRelease(image, architecture)
@@ -171,7 +165,6 @@ func (r *release) GetBaseIsoVersion(architecture string) (string, error) {
 func (r *release) getImageFromRelease(imageName string, architecture string) (string, error) {
 	// This requires the 'oc' command so make sure its available
 	_, err := exec.LookPath("oc")
-	var cmd string
 	if err != nil {
 		if len(r.mirrorConfig) > 0 {
 			logrus.Warning("Unable to validate mirror config because \"oc\" command is not available")
@@ -182,21 +175,32 @@ func (r *release) getImageFromRelease(imageName string, architecture string) (st
 	}
 
 	archName := arch.GoArch(architecture)
-
+	imagefor := "--image-for=" + imageName
+	filterbyos := "--filter-by-os=linux/" + archName
+	insecure := "--insecure=true"
+
+	var cmd = []string{
+		"oc",
+		"adm",
+		"release",
+		"info",
+		imagefor,
+		filterbyos,
+		insecure,
+	}
 	if len(r.mirrorConfig) > 0 {
 		logrus.Debugf("Using mirror configuration")
 		icspFile, err := getIcspFileFromRegistriesConfig(r.mirrorConfig)
 		if err != nil {
 			return "", err
 		}
 		defer removeIcspFile(icspFile)
-		cmd = fmt.Sprintf(templateGetImageWithIcsp, imageName, archName, true, icspFile, r.releaseImage)
-	} else {
-		cmd = fmt.Sprintf(templateGetImage, imageName, archName, true, r.releaseImage)
+		icspfile := "--icsp-file=" + icspFile
+		cmd = append(cmd, icspfile)
 	}
-
+	cmd = append(cmd, r.releaseImage)
 	logrus.Debugf("Fetching image from OCP release (%s)", cmd)
-	image, err := execute(r.pullSecret, cmd)
+	image, err := agent.ExecuteOC(r.pullSecret, cmd)
 	if err != nil {
 		if strings.Contains(err.Error(), "unknown flag: --icsp-file") {
 			logrus.Warning("Using older version of \"oc\" that does not support mirroring")
@@ -208,26 +212,36 @@ func (r *release) getImageFromRelease(imageName string, architecture string) (st
 }
 
 func (r *release) extractFileFromImage(image, file, cacheDir string, architecture string) ([]string, error) {
-	var cmd string
 	archName := arch.GoArch(architecture)
+	extractpath := "--path=" + file + ":" + cacheDir
+	filterbyos := "--filter-by-os=linux/" + archName
+
+	var cmd = []string{
+		"oc",
+		"image",
+		"extract",
+		extractpath,
+		filterbyos,
+		"--confirm",
+	}
+
 	if len(r.mirrorConfig) > 0 {
 		icspFile, err := getIcspFileFromRegistriesConfig(r.mirrorConfig)
 		if err != nil {
 			return nil, err
 		}
 		defer removeIcspFile(icspFile)
-		cmd = fmt.Sprintf(templateImageExtractWithIcsp, file, cacheDir, archName, icspFile, image)
-	} else {
-		cmd = fmt.Sprintf(templateImageExtract, file, cacheDir, archName, image)
+		icspfile := "--icsp-file=" + icspFile
+		cmd = append(cmd, icspfile)
 	}
 	path := filepath.Join(cacheDir, path.Base(file))
 	// Remove file if it exists
 	if err := removeCacheFile(path); err != nil {
 		return nil, err
 	}
-
+	cmd = append(cmd, image)
 	logrus.Debugf("extracting %s to %s, %s", file, cacheDir, cmd)
-	_, err := retry.Do(r.config.MaxTries, r.config.RetryDelay, execute, r.pullSecret, cmd)
+	_, err := retry.Do(r.config.MaxTries, r.config.RetryDelay, agent.ExecuteOC, r.pullSecret, cmd)
 	if err != nil {
 		return nil, err
 	}
@@ -244,7 +258,7 @@ func (r *release) extractFileFromImage(image, file, cacheDir string, architectur
 	return matches, nil
 }
 
-// Get hash from rhcos.json
+// Get hash from rhcos.json.
 func getHashFromInstaller(architecture string) (bool, string) {
 	// Get hash from metadata in the installer
 	ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second)
@@ -277,7 +291,7 @@ func matchingHash(imageSha []byte, sha string) bool {
 	return false
 }
 
-// Check if there is a different base ISO in the release payload
+// Check if there is a different base ISO in the release payload.
 func (r *release) verifyCacheFile(image, file, architecture string) (bool, error) {
 	// Get hash of cached file
 	f, err := os.Open(file)
@@ -343,44 +357,7 @@ func removeCacheFile(path string) error {
 	return nil
 }
 
-func execute(pullSecret, command string) (string, error) {
-	ps, err := os.CreateTemp("", "registry-config")
-	if err != nil {
-		return "", err
-	}
-	defer func() {
-		ps.Close()
-		os.Remove(ps.Name())
-	}()
-	_, err = ps.Write([]byte(pullSecret))
-	if err != nil {
-		return "", err
-	}
-	// flush the buffer to ensure the file can be read
-	ps.Close()
-	executeCommand := command[:] + " --registry-config=" + ps.Name()
-	args := strings.Split(executeCommand, " ")
-
-	var stdoutBytes, stderrBytes bytes.Buffer
-	cmd := exec.Command(args[0], args[1:]...) //nolint:gosec
-	cmd.Stdout = &stdoutBytes
-	cmd.Stderr = &stderrBytes
-
-	err = cmd.Run()
-	if err == nil {
-		return strings.TrimSpace(stdoutBytes.String()), nil
-	}
-
-	var exitErr *exec.ExitError
-	if errors.As(err, &exitErr) {
-		err = fmt.Errorf("command '%s' exited with non-zero exit code %d: %s\n%s", executeCommand, exitErr.ExitCode(), stdoutBytes.String(), stderrBytes.String())
-	} else {
-		err = fmt.Errorf("command '%s' failed: %w", executeCommand, err)
-	}
-	return "", err
-}
-
-// Create a temporary file containing the ImageContentPolicySources
+// Create a temporary file containing the ImageContentPolicySources.
 func getIcspFileFromRegistriesConfig(mirrorConfig []mirror.RegistriesConfig) (string, error) {
 	contents, err := getIcspContents(mirrorConfig)
 	if err != nil {
@@ -406,7 +383,7 @@ func getIcspFileFromRegistriesConfig(mirrorConfig []mirror.RegistriesConfig) (st
 	return icspFile.Name(), nil
 }
 
-// Convert the data in registries.conf into ICSP format
+// Convert the data in registries.conf into ICSP format.
 func getIcspContents(mirrorConfig []mirror.RegistriesConfig) ([]byte, error) {
 	icsp := operatorv1alpha1.ImageContentSourcePolicy{
 		TypeMeta: metav1.TypeMeta{

pkg/asset/agent/image/releaseimage.go

@@ -10,18 +10,18 @@ import (
 )
 
 type releaseImage struct {
-	ReleaseVersion string `json:"openshift_version"`
-	Arch           string `json:"cpu_architecture"`
-	PullSpec       string `json:"url"`
-	Tag            string `json:"version"`
+	ReleaseVersion string   `json:"openshift_version"`
+	Arch           string   `json:"cpu_architecture"`
+	Archs          []string `json:"cpu_architectures"`
+	PullSpec       string   `json:"url"`
+	Tag            string   `json:"version"`
 }
 
 func isDigest(pullspec string) bool {
 	return regexp.MustCompile(`.*sha256:[a-fA-F0-9]{64}$`).MatchString(pullspec)
 }
 
-func releaseImageFromPullSpec(pullSpec, arch string) (releaseImage, error) {
-
+func releaseImageFromPullSpec(pullSpec, arch string, archs []string) (releaseImage, error) {
 	// When the pullspec it's a digest let's use the current version
 	// stored in the installer
 	if isDigest(pullSpec) {
@@ -33,6 +33,7 @@ func releaseImageFromPullSpec(pullSpec, arch string) (releaseImage, error) {
 		return releaseImage{
 			ReleaseVersion: versionString,
 			Arch:           arch,
+			Archs:          archs,
 			PullSpec:       pullSpec,
 			Tag:            versionString,
 		}, nil
@@ -54,14 +55,14 @@ func releaseImageFromPullSpec(pullSpec, arch string) (releaseImage, error) {
 	return releaseImage{
 		ReleaseVersion: relVersion,
 		Arch:           arch,
+		Archs:          archs,
 		PullSpec:       pullSpec,
 		Tag:            tag,
 	}, nil
 }
 
-func releaseImageList(pullSpec, arch string) (string, error) {
-
-	relImage, err := releaseImageFromPullSpec(pullSpec, arch)
+func releaseImageList(pullSpec, arch string, archs []string) (string, error) {
+	relImage, err := releaseImageFromPullSpec(pullSpec, arch, archs)
 	if err != nil {
 		return "", err
 	}

pkg/asset/agent/image/releaseimage_test.go

@@ -17,37 +17,37 @@ func TestReleaseImageList(t *testing.T) {
 			name:     "4.10rc",
 			pullSpec: "quay.io/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64",
 			arch:     "x86_64",
-			result:   "[{\"openshift_version\":\"4.10\",\"cpu_architecture\":\"x86_64\",\"url\":\"quay.io/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64\",\"version\":\"4.10.0-rc.1\"}]",
+			result:   "[{\"openshift_version\":\"4.10\",\"cpu_architecture\":\"x86_64\",\"cpu_architectures\":[\"x86_64\"],\"url\":\"quay.io/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64\",\"version\":\"4.10.0-rc.1\"}]",
 		},
 		{
 			name:     "pull-spec-includes-port-number",
 			pullSpec: "quay.io:433/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64",
 			arch:     "x86_64",
-			result:   "[{\"openshift_version\":\"4.10\",\"cpu_architecture\":\"x86_64\",\"url\":\"quay.io:433/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64\",\"version\":\"4.10.0-rc.1\"}]",
+			result:   "[{\"openshift_version\":\"4.10\",\"cpu_architecture\":\"x86_64\",\"cpu_architectures\":[\"x86_64\"],\"url\":\"quay.io:433/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64\",\"version\":\"4.10.0-rc.1\"}]",
 		},
 		{
 			name:     "arm",
 			pullSpec: "quay.io/openshift-release-dev/ocp-release:4.10.0-rc.1-aarch64",
 			arch:     "aarch64",
-			result:   "[{\"openshift_version\":\"4.10\",\"cpu_architecture\":\"aarch64\",\"url\":\"quay.io/openshift-release-dev/ocp-release:4.10.0-rc.1-aarch64\",\"version\":\"4.10.0-rc.1\"}]",
+			result:   "[{\"openshift_version\":\"4.10\",\"cpu_architecture\":\"aarch64\",\"cpu_architectures\":[\"aarch64\"],\"url\":\"quay.io/openshift-release-dev/ocp-release:4.10.0-rc.1-aarch64\",\"version\":\"4.10.0-rc.1\"}]",
 		},
 		{
 			name:     "4.11ci",
 			pullSpec: "registry.ci.openshift.org/ocp/release:4.11.0-0.ci-2022-05-16-202609",
 			arch:     "x86_64",
-			result:   "[{\"openshift_version\":\"4.11\",\"cpu_architecture\":\"x86_64\",\"url\":\"registry.ci.openshift.org/ocp/release:4.11.0-0.ci-2022-05-16-202609\",\"version\":\"4.11.0-0.ci-2022-05-16-202609\"}]",
+			result:   "[{\"openshift_version\":\"4.11\",\"cpu_architecture\":\"x86_64\",\"cpu_architectures\":[\"x86_64\"],\"url\":\"registry.ci.openshift.org/ocp/release:4.11.0-0.ci-2022-05-16-202609\",\"version\":\"4.11.0-0.ci-2022-05-16-202609\"}]",
 		},
 		{
 			name:     "CI-ephemeral",
 			pullSpec: "registry.build04.ci.openshift.org/ci-op-m7rfgytz/release@sha256:ebb203f24ee060d61bdb466696a9c20b3841f9929badf9b81fc99cbedc2a679e",
 			arch:     "x86_64",
-			result:   "[{\"openshift_version\":\"was not built correctly\",\"cpu_architecture\":\"x86_64\",\"url\":\"registry.build04.ci.openshift.org/ci-op-m7rfgytz/release@sha256:ebb203f24ee060d61bdb466696a9c20b3841f9929badf9b81fc99cbedc2a679e\",\"version\":\"was not built correctly\"}]",
+			result:   "[{\"openshift_version\":\"was not built correctly\",\"cpu_architecture\":\"x86_64\",\"cpu_architectures\":[\"x86_64\"],\"url\":\"registry.build04.ci.openshift.org/ci-op-m7rfgytz/release@sha256:ebb203f24ee060d61bdb466696a9c20b3841f9929badf9b81fc99cbedc2a679e\",\"version\":\"was not built correctly\"}]",
 		},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			output, err := releaseImageList(tc.pullSpec, tc.arch)
+			output, err := releaseImageList(tc.pullSpec, tc.arch, []string{tc.arch})
 			assert.NoError(t, err)
 			if err == nil {
 				assert.Equal(t, tc.result, output)
@@ -65,7 +65,7 @@ func TestReleaseImageListErrors(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc, func(t *testing.T) {
-			_, err := releaseImageList(tc, "x86_64")
+			_, err := releaseImageList(tc, "x86_64", []string{"x86_64"})
 			assert.Error(t, err)
 		})
 	}