Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/cli/admin/release/info: Render multi-arch release information #1292

Conversation

wking
Copy link
Member

@wking wking commented Nov 21, 2022

Comparing release text for various arch-specific shards of the multi-arch 4.12.0-rc.0 release:

$ for B in arm64 ppc64le s390x; do diff -u <(curl -s https://mirror.openshift.com/pub/openshift-v4/multi/clients/ocp/4.12.0-rc.0/amd64/release.txt) <(curl -s "https://mirror.openshift.com/pub/openshift-v4/multi/clients/ocp/4.12.0-rc.0/${B}/release.txt"); done
--- /dev/fd/63  2022-11-16 14:57:24.849975880 -0800
+++ /dev/fd/62  2022-11-16 14:57:24.854975880 -0800
@@ -13,7 +13,7 @@
 Name:           4.12.0-rc.0
 Digest:         sha256:7ac78aaca993bf307010e8e92642da2bc28e275fbac241b42f24be46153f12d1
 Created:        2022-11-10T19:12:09Z
-OS/Arch:        linux/amd64
+OS/Arch:        linux/arm64
 Manifests:      643
 Metadata files: 1
    
--- /dev/fd/63  2022-11-16 14:57:26.195975808 -0800
+++ /dev/fd/62  2022-11-16 14:57:26.195975808 -0800
@@ -13,7 +13,7 @@
 Name:           4.12.0-rc.0
 Digest:         sha256:7ac78aaca993bf307010e8e92642da2bc28e275fbac241b42f24be46153f12d1
 Created:        2022-11-10T19:12:09Z
-OS/Arch:        linux/amd64
+OS/Arch:        linux/ppc64le
 Manifests:      643
 Metadata files: 1
    
--- /dev/fd/63  2022-11-16 14:57:26.821975774 -0800
+++ /dev/fd/62  2022-11-16 14:57:26.824975774 -0800
@@ -13,7 +13,7 @@
 Name:           4.12.0-rc.0
 Digest:         sha256:7ac78aaca993bf307010e8e92642da2bc28e275fbac241b42f24be46153f12d1
 Created:        2022-11-10T19:12:09Z
-OS/Arch:        linux/amd64
+OS/Arch:        linux/s390x
 Manifests:      643
 Metadata files: 1

With this change, those OS/Arch properties will be rendered like:

  OS/Arch:        multi (linux/amd64)

and similar, to make it more obvious that the release payload as a whole is multi-architecture.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2022
@wking wking force-pushed the multi-arch-release-arch-rendering branch from eac5120 to aa87d25 Compare November 21, 2022 21:07
@petr-muller
Copy link
Member

/cc

Copy link
Member

@petr-muller petr-muller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retesting e2e-agnostic-ovn-cmd and e2e-aws-ovn, failures seem unrelated.

/retest
/hold

LGTM, feel free to address the nit or unhold.

pkg/cli/admin/release/info.go Show resolved Hide resolved
@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2022
@wking wking force-pushed the multi-arch-release-arch-rendering branch from aa87d25 to ae46096 Compare November 22, 2022 21:59
@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2022
@wking
Copy link
Member Author

wking commented Nov 23, 2022

/retest-required

Comparing release text for various arch-specific shards of the
multi-arch 4.12.0-rc.0 release:

  $ for B in arm64 ppc64le s390x; do diff -u <(curl -s https://mirror.openshift.com/pub/openshift-v4/multi/clients/ocp/4.12.0-rc.0/amd64/release.txt) <(curl -s "https://mirror.openshift.com/pub/openshift-v4/multi/clients/ocp/4.12.0-rc.0/${B}/release.txt"); done
  --- /dev/fd/63  2022-11-16 14:57:24.849975880 -0800
  +++ /dev/fd/62  2022-11-16 14:57:24.854975880 -0800
  @@ -13,7 +13,7 @@
   Name:           4.12.0-rc.0
   Digest:         sha256:7ac78aaca993bf307010e8e92642da2bc28e275fbac241b42f24be46153f12d1
   Created:        2022-11-10T19:12:09Z
  -OS/Arch:        linux/amd64
  +OS/Arch:        linux/arm64
   Manifests:      643
   Metadata files: 1

  --- /dev/fd/63  2022-11-16 14:57:26.195975808 -0800
  +++ /dev/fd/62  2022-11-16 14:57:26.195975808 -0800
  @@ -13,7 +13,7 @@
   Name:           4.12.0-rc.0
   Digest:         sha256:7ac78aaca993bf307010e8e92642da2bc28e275fbac241b42f24be46153f12d1
   Created:        2022-11-10T19:12:09Z
  -OS/Arch:        linux/amd64
  +OS/Arch:        linux/ppc64le
   Manifests:      643
   Metadata files: 1

  --- /dev/fd/63  2022-11-16 14:57:26.821975774 -0800
  +++ /dev/fd/62  2022-11-16 14:57:26.824975774 -0800
  @@ -13,7 +13,7 @@
   Name:           4.12.0-rc.0
   Digest:         sha256:7ac78aaca993bf307010e8e92642da2bc28e275fbac241b42f24be46153f12d1
   Created:        2022-11-10T19:12:09Z
  -OS/Arch:        linux/amd64
  +OS/Arch:        linux/s390x
   Manifests:      643
   Metadata files: 1

With this change, those OS/Arch properties will be rendered like:

  OS/Arch:        multi (linux/amd64)

and similar, to make it more obvious that the release payload as a
whole is multi-architecture.
@wking wking force-pushed the multi-arch-release-arch-rendering branch from ae46096 to 2ea70a4 Compare November 24, 2022 01:40
@@ -736,15 +736,23 @@ func (i *ReleaseInfo) PreferredName() string {
}

func (i *ReleaseInfo) Platform() string {
os := i.Config.OS
if len(os) > 0 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't causing trouble before, because describeReleaseInfo wasn't using this Platform method, but the > was a bug, and I'm fixing it to ==.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 24, 2022

@wking: all tests passed!

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.

Copy link
Member

@petr-muller petr-muller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet 👍

@petr-muller
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 24, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, wking

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:

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

@openshift-merge-robot openshift-merge-robot merged commit c1695d2 into openshift:master Nov 24, 2022
@wking wking deleted the multi-arch-release-arch-rendering branch November 29, 2022 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants