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

jeffdyoung · 2023-07-20T00:02:43Z

Adding additional validation to check that the release payload architecture matches the cluster's architecture.
Also fixes OCPBUGS-27787 - single arch ABI installs w/multi payload, adding cpu_architectures to RELEASE_IMAGES

RELEASE_IMAGES:
Single arch -

'[{"openshift_version":"4.15","cpu_architecture":"x86_64","cpu_architectures":["x86_64"],"url":"quay.io/openshift-release-dev/ocp-release:4.15.0-rc.4-x86_64","support_level":"beta","version":"4.15.0-rc.4"}]

Multi -

[{"openshift_version":"4.15-multi","cpu_architecture":"multi","cpu_architectures":["x86_64","arm64","ppc64le","s390x"],"url":"quay.io/openshift-release-dev/ocp-release:4.15.0-rc.4-multi","support_level":"beta","version":"4.15.0-rc.4-multi"}]'

openshift-ci-robot · 2023-07-20T00:02:46Z

@jeffdyoung: This pull request references Jira Issue OCPBUGS-15941, which is invalid:

expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Adding additional validation to check that the release payload architecture matches the cluster's architecture. Will also throw an error if the 'multi' payload is detected.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jeffdyoung · 2023-07-20T00:03:30Z

This will also undo the work done in #7276
because this test is creating an agent.aarch64.iso with the default x86 payload

jeffdyoung · 2023-07-22T17:58:16Z

/retest

jeffdyoung · 2023-07-23T01:51:56Z

/retest

jeffdyoung · 2023-07-23T22:41:16Z

/assign @zaneb

hack/go-lint.sh

jeffdyoung · 2023-07-26T03:32:40Z

/retest

jeffdyoung · 2023-07-26T12:28:07Z

/retest

jeffdyoung · 2023-07-26T17:45:38Z

/retest

jeffdyoung · 2023-07-27T00:52:21Z

/retest

pawanpinjarkar

PTAL

cmd/openshift-install/testdata/agent/image/validations/sno_invalid_arch.txt

pawanpinjarkar

Minor changes

pkg/asset/agent/image/oc.go

pawanpinjarkar · 2023-08-08T00:23:53Z

/retest-required

pkg/asset/agent/image/oc.go

cmd/openshift-install/testdata/agent/image/validations/sno_invalid_arch.txt

pkg/asset/agent/manifests/releaseimagearch.go

andfasano · 2024-02-20T17:58:42Z

pkg/asset/agent/image/releaseimage.go

@@ -33,6 +33,7 @@ func releaseImageFromPullSpec(pullSpec, arch string) (releaseImage, error) {
 		return releaseImage{
 			ReleaseVersion: versionString,
 			Arch:           arch,
+			Archs:          archs,


@jeffdyoung this point it's not clear to me. What's the requirement to have separate fields Archs and Arch? Would make sense to have just one field Archs to cover all the cases?

I think it's just because Archs was added later.
openshift/assisted-service#4248

Looks like we'll need both for now:

level=fatal msg="failed to create Versions handlers" func=main.main.func1 file="/src/cmd/main.go:237" error="Missing value in ReleaseImage for 'cpu_architecture' field"

This isn't an exhaustive list, but I see both used in various parts of the assisted-service:
https://github.com/openshift/assisted-service/blob/master/models/infra_env.go#L39
https://github.com/openshift/assisted-service/blob/master/models/infra_env_create_params.go#L40
https://github.com/openshift/assisted-service/blob/master/models/os_image.go#L26
https://github.com/openshift/assisted-service/blob/master/models/release_image.go#L27-L30
https://github.com/openshift/assisted-service/blob/master/models/openshift_version.go#L25

zaneb

This would be easier to review (and faster to re-review) if the logically separate changes were in separate commits. I can see at least these 5 logical changes:

Use more realistic pull secret in unit tests
Refactor execute() function to not do arg splitting
Move execute() function to a common location
Support multi-arch payload in releaseImage data
Validate release arch when possible

zaneb · 2024-02-25T22:36:03Z

pkg/asset/agent/image/releaseimage.go

@@ -33,6 +33,7 @@ func releaseImageFromPullSpec(pullSpec, arch string) (releaseImage, error) {
 		return releaseImage{
 			ReleaseVersion: versionString,
 			Arch:           arch,
+			Archs:          archs,


I think it's just because Archs was added later.
openshift/assisted-service#4248

pkg/asset/agent/image/releaseextract.go

pkg/asset/agent/image/ignition.go

zaneb · 2024-02-26T00:06:47Z

pkg/asset/agent/common.go

+		pullSpec,
+		templateFilter,
+	}
+	releaseArch, err := ExecuteOC(pullSecret, getReleaseArch)


This is ok for this PR, but particularly with the heterogeneous cluster support coming in I'd like to see a follow-up switching to direct docker client calls as suggested in #7349 (comment).
I think validation of the release payload arch is going to be critical for heterogeneous clusters in particular.

@zaneb I agree, that something* needs to be done to validate the release image(s) for all IPI/assisted/agent installs. Not sure how that suggestion will work with sparse manifest list? But yeah, want to figure this out for heterogeneous in future releases.

pkg/asset/agent/oc.go

pkg/asset/agent/image/ignition.go

zaneb

/approve

pkg/asset/agent/oc.go

openshift-ci · 2024-02-29T02:23:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zaneb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/asset/agent/OWNERS~~ [zaneb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jeffdyoung · 2024-02-29T19:34:45Z

/retest

jeffdyoung · 2024-02-29T22:55:38Z

/retest

jeffdyoung · 2024-03-01T04:23:21Z

/retest

jeffdyoung · 2024-03-03T23:20:04Z

/retest

jeffdyoung · 2024-03-05T15:59:52Z

/retest

jeffdyoung · 2024-03-06T16:44:38Z

/retest

jeffdyoung · 2024-03-11T01:57:43Z

/retest

jeffdyoung · 2024-03-14T13:26:21Z

/retest

andfasano · 2024-03-14T14:40:10Z

/lgtm

openshift-ci · 2024-03-14T15:10:57Z

@jeffdyoung: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-e2e-agent-sno-ipv6	`e421b24`	link	false	`/test okd-scos-e2e-agent-sno-ipv6`
ci/prow/okd-scos-e2e-agent-compact-ipv4	`e421b24`	link	false	`/test okd-scos-e2e-agent-compact-ipv4`
ci/prow/okd-e2e-aws-ovn-upgrade	`e421b24`	link	false	`/test okd-e2e-aws-ovn-upgrade`
ci/prow/okd-scos-e2e-aws-ovn	`e421b24`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/okd-e2e-aws-ovn	`e421b24`	link	false	`/test okd-e2e-aws-ovn`
ci/prow/terraform-verify-vendor	`e421b24`	link	true	`/test terraform-verify-vendor`
ci/prow/terraform-images	`e421b24`	link	true	`/test terraform-images`
ci/prow/okd-e2e-agent-compact-ipv4	`19f298a`	link	false	`/test okd-e2e-agent-compact-ipv4`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2024-03-23T01:33:58Z

@jeffdyoung: Jira Issue OCPBUGS-15941: All pull requests linked via external trackers have merged:

openshift/installer#7349

Jira Issue OCPBUGS-15941 has been moved to the MODIFIED state.

In response to this:

Adding additional validation to check that the release payload architecture matches the cluster's architecture.
Also fixes OCPBUGS-27787 - single arch ABI installs w/multi payload, adding cpu_architectures to RELEASE_IMAGES

RELEASE_IMAGES:
Single arch -
'[{"openshift_version":"4.15","cpu_architecture":"x86_64","cpu_architectures":["x86_64"],"url":"quay.io/openshift-release-dev/ocp-release:4.15.0-rc.4-x86_64","support_level":"beta","version":"4.15.0-rc.4"}]
Multi -
[{"openshift_version":"4.15-multi","cpu_architecture":"multi","cpu_architectures":["x86_64","arm64","ppc64le","s390x"],"url":"quay.io/openshift-release-dev/ocp-release:4.15.0-rc.4-multi","support_level":"beta","version":"4.15.0-rc.4-multi"}]'

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 20, 2023

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Jul 20, 2023

openshift-ci bot requested review from andfasano and lranjbar July 20, 2023 00:04

jeffdyoung force-pushed the OCPBUGS-15941 branch from 6fa6e96 to 3e652af Compare July 21, 2023 15:09

openshift-ci bot assigned zaneb Jul 23, 2023

r4f4 reviewed Jul 24, 2023

View reviewed changes

hack/go-lint.sh Outdated Show resolved Hide resolved

jeffdyoung force-pushed the OCPBUGS-15941 branch from 28d52c4 to 10647b2 Compare July 25, 2023 12:29

pawanpinjarkar suggested changes Aug 7, 2023

View reviewed changes

pawanpinjarkar reviewed Aug 7, 2023

View reviewed changes

cmd/openshift-install/testdata/agent/image/validations/sno_invalid_arch.txt Outdated Show resolved Hide resolved

pawanpinjarkar reviewed Aug 7, 2023

View reviewed changes

pkg/asset/agent/image/oc.go Outdated Show resolved Hide resolved

jeffdyoung force-pushed the OCPBUGS-15941 branch from 007a4de to 4526e8f Compare August 7, 2023 19:33

zaneb reviewed Aug 8, 2023

View reviewed changes

pkg/asset/agent/image/oc.go Outdated Show resolved Hide resolved

pkg/asset/agent/image/oc.go Outdated Show resolved Hide resolved

pkg/asset/agent/image/oc.go Outdated Show resolved Hide resolved

andfasano reviewed Aug 8, 2023

View reviewed changes

pkg/asset/agent/image/oc.go Outdated Show resolved Hide resolved

pkg/asset/agent/image/oc.go Outdated Show resolved Hide resolved

cmd/openshift-install/testdata/agent/image/validations/sno_invalid_arch.txt Outdated Show resolved Hide resolved

jeffdyoung changed the title ~~OCPBUGS-15941: Validate release image arch when creating agent.iso~~ WIP OCPBUGS-15941: Validate release image arch when creating agent.iso Aug 17, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 17, 2023

jeffdyoung changed the title ~~WIP OCPBUGS-15941: Validate release image arch when creating agent.iso~~ [WIP] OCPBUGS-15941: Validate release image arch when creating agent.iso Aug 17, 2023

jeffdyoung force-pushed the OCPBUGS-15941 branch from 3603937 to 5fd1791 Compare August 17, 2023 14:20

jeffdyoung commented Aug 17, 2023

View reviewed changes

pkg/asset/agent/manifests/releaseimagearch.go Outdated Show resolved Hide resolved

jeffdyoung requested a review from andfasano February 20, 2024 12:47

andfasano reviewed Feb 20, 2024

View reviewed changes

zaneb reviewed Feb 26, 2024

View reviewed changes

jeffdyoung force-pushed the OCPBUGS-15941 branch from acf3aa3 to 2f8fdaf Compare February 29, 2024 01:27

zaneb reviewed Feb 29, 2024

View reviewed changes

pkg/asset/agent/oc.go Outdated Show resolved Hide resolved

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 29, 2024

jeffdyoung force-pushed the OCPBUGS-15941 branch from d1402b7 to 0fca7bb Compare February 29, 2024 11:46

jeffdyoung added 5 commits February 29, 2024 07:09

Use more realistic pull secret in unit tests

e97ce22

Refactor execute() function to not do arg splitting

2f8d547

Move execute() function to a common location

b768fca

Support multi-arch payload in releaseImage data

d0b5481

Validate release arch when possible

19f298a

jeffdyoung force-pushed the OCPBUGS-15941 branch from 0fca7bb to 19f298a Compare February 29, 2024 12:18

jeffdyoung requested review from zaneb and andfasano March 5, 2024 20:01

openshift-ci bot assigned andfasano Mar 14, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2024

openshift-merge-bot bot merged commit 8710137 into openshift:master Mar 23, 2024
29 of 30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jeffdyoung commented Jul 20, 2023 •

edited

openshift-ci-robot commented Jul 20, 2023

jeffdyoung commented Jul 20, 2023

jeffdyoung commented Jul 22, 2023

jeffdyoung commented Jul 23, 2023

jeffdyoung commented Jul 23, 2023

jeffdyoung commented Jul 26, 2023

jeffdyoung commented Jul 26, 2023

jeffdyoung commented Jul 26, 2023

jeffdyoung commented Jul 27, 2023

pawanpinjarkar left a comment

pawanpinjarkar left a comment

pawanpinjarkar commented Aug 8, 2023

andfasano Feb 20, 2024

zaneb Feb 25, 2024

jeffdyoung Feb 29, 2024

jeffdyoung Feb 29, 2024

zaneb left a comment

zaneb Feb 25, 2024

zaneb Feb 26, 2024

jeffdyoung Feb 29, 2024

zaneb left a comment

openshift-ci bot commented Feb 29, 2024

jeffdyoung commented Feb 29, 2024

jeffdyoung commented Feb 29, 2024

jeffdyoung commented Mar 1, 2024

jeffdyoung commented Mar 3, 2024

jeffdyoung commented Mar 5, 2024

jeffdyoung commented Mar 6, 2024

jeffdyoung commented Mar 11, 2024

jeffdyoung commented Mar 14, 2024

andfasano commented Mar 14, 2024

openshift-ci bot commented Mar 14, 2024

openshift-ci-robot commented Mar 23, 2024

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

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

Conversation

jeffdyoung commented Jul 20, 2023 • edited

openshift-ci-robot commented Jul 20, 2023

jeffdyoung commented Jul 20, 2023

jeffdyoung commented Jul 22, 2023

jeffdyoung commented Jul 23, 2023

jeffdyoung commented Jul 23, 2023

jeffdyoung commented Jul 26, 2023

jeffdyoung commented Jul 26, 2023

jeffdyoung commented Jul 26, 2023

jeffdyoung commented Jul 27, 2023

pawanpinjarkar left a comment

Choose a reason for hiding this comment

pawanpinjarkar left a comment

Choose a reason for hiding this comment

pawanpinjarkar commented Aug 8, 2023

andfasano Feb 20, 2024

Choose a reason for hiding this comment

zaneb Feb 25, 2024

Choose a reason for hiding this comment

jeffdyoung Feb 29, 2024

Choose a reason for hiding this comment

jeffdyoung Feb 29, 2024

Choose a reason for hiding this comment

zaneb left a comment

Choose a reason for hiding this comment

zaneb Feb 25, 2024

Choose a reason for hiding this comment

zaneb Feb 26, 2024

Choose a reason for hiding this comment

jeffdyoung Feb 29, 2024

Choose a reason for hiding this comment

zaneb left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Feb 29, 2024

jeffdyoung commented Feb 29, 2024

jeffdyoung commented Feb 29, 2024

jeffdyoung commented Mar 1, 2024

jeffdyoung commented Mar 3, 2024

jeffdyoung commented Mar 5, 2024

jeffdyoung commented Mar 6, 2024

jeffdyoung commented Mar 11, 2024

jeffdyoung commented Mar 14, 2024

andfasano commented Mar 14, 2024

openshift-ci bot commented Mar 14, 2024

openshift-ci-robot commented Mar 23, 2024

jeffdyoung commented Jul 20, 2023 •

edited