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

Bug 1793128: enable s390x bootimage support #2933

Merged
merged 6 commits into from
Jan 21, 2020
Merged

Bug 1793128: enable s390x bootimage support #2933

merged 6 commits into from
Jan 21, 2020

Conversation

crawford
Copy link
Contributor

This is a backport of #2885 along with two other commits that enable and add s390x images. The latter two weren't included in master because there are no 4.4 builds for s390x.

cgwalters and others added 2 commits January 15, 2020 19:21
Don't allow people to point to e.g. an RHT-internal endpoint.

See: #2462
Not everyone has python3 installed into /usr/bin/.
@openshift-ci-robot
Copy link
Contributor

@crawford: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

*: enable s390x bootimage support

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.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 16, 2020
@crawford
Copy link
Contributor Author

crawford commented Jan 16, 2020

/hold

Waiting to know that this is the correct version for s390x.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 16, 2020
@wking
Copy link
Member

wking commented Jan 16, 2020

$ git --no-pager grep buildid origin/pr/2933 -- data/data
origin/pr/2933:data/data/rhcos-amd64.json:    "buildid": "42.80.20191002.0",
origin/pr/2933:data/data/rhcos-s390x.json:    "buildid": "42s390x.81.20191224.0",
origin/pr/2933:data/data/rhcos.json:    "buildid": "42.80.20191002.0",

We're ok with this divergence? Should we be bumping AWS to a December build at the same time? Or are our AMIs frozen out? Do we have a summary of what chanced between October and December?

@miabbott
Copy link
Member

miabbott commented Jan 16, 2020

Should we be bumping AWS to a December build at the same time? Or are our AMIs frozen out?

We haven't made new AMIs for 4.2 since the GA release.

Do we have a summary of what chanced between October and December?

The biggest change is a bump from RHEL 8.0 to RHEL 8.1 content. Using the x86_64 metadata as a guide (the differ script doesn't support multi-arch, yet):

https://gist.github.com/miabbott/1e790139031706c4b47eec10f2870e40

@andymcc
Copy link
Member

andymcc commented Jan 16, 2020

The latest build for s390x is here:
https://releases-rhcos-art.cloud.privileged.psi.redhat.com/?stream=releases/rhcos-4.2-s390x&release=42s390x.81.20200115.0#42s390x.81.20200115.0

That said no changes have gone in to redhat-coreos or coreos-assembler for the 4.2-multiarch branch since 2019/12/19 so I don't think there would be any benefit to changing that version to the latest from the 2019/12/24 build.

@crawford
Copy link
Contributor Author

/retest

@wking
Copy link
Member

wking commented Jan 16, 2020

You'll need a bug to get the bugzilla/valid-bug label.

@sdodson
Copy link
Member

sdodson commented Jan 16, 2020

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sdodson

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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 16, 2020
@wking
Copy link
Member

wking commented Jan 16, 2020

Comparing with the master-ward PR:

  • 45de9b9 is from hack/update-rhcos-bootimage: Require ART endpoint #2463 (also landed in master).

  • efaeb0c is a clean cherry-pick of 8428471.

  • I dunno if we care about fixing this shift:

    diff -U3 <(git show b1726d2c5) <(git show 9895178ce) | tail -n15
      args = parser.parse_args()
      
    -+metadata_dir = os.path.join(os.path.dirname(sys.argv[0]), "../data/data")
    -+
      if not args.meta.startswith(RHCOS_RELEASES_APP):
          raise SystemExit("URL must start with: " + RHCOS_RELEASES_APP)
      
    ++metadata_dir = os.path.join(os.path.dirname(sys.argv[0]), "../data/data")
    ++
    + with urllib.request.urlopen(args.meta) as f:
    +     string_f = codecs.getreader('utf-8')(f)  # support for Python < 3.6
    +     meta = json.load(string_f)
     @@ -32,5 +33,14 @@ newmeta['amis'] = {
          for entry in meta['amis']
      }

    Otherwise b1726d2 -> 9895178 only differs in the choice of bootimage, which is fine.

  • 2583ba4 -> 13a0e55 has a lot of context noise, but is a clean cherrypick.

  • d7f0664 and 1362d78 are new (because the masterward PR wasn't adding s390x, as discussed earlier in this PR).

@wking
Copy link
Member

wking commented Jan 16, 2020

Oh, hanging a bug on this will need a 4.3 backport of #2885 as well. Which I guess will not include these s390x commits?

@wking
Copy link
Member

wking commented Jan 16, 2020

Two nits: the metadata_dir position and the reactive-bump-logic suggestion. I'm fine with this landing without either of those being addressed though.

This splits the RHCOS build metadata into architecture-specific files.
This will allow the metadata to contain information about bootimages of
multiple architectures. In order to preserve backward compatibility
(there are a few users, including certain CI jobs, that pull rhcos.json
from GitHub directly), I've opted to use separate files for each
architecture. Normally, we could have just symlinked the legacy metadata
file, but when hosted on raw.githubcontent.com, the symlinks aren't
followed.

When updating the RHCOS bootimages, this script will need to be run once
for each architecture that is being updated.

The build metadata was resynced with the following:

./hack/update-rhcos-bootimage.py https://releases-art-rhcos.svc.ci.openshift.org/art/storage/releases/rhcos-4.2/42.80.20191002.0/meta.json amd64
This adds an architecture parameter to the RHCOS image lookup process
and a corresponding field to MachinePool. This is a backward-compatible
change, defaulting the architecture to AMD64 if none has been specified.
This also enforces that the control plane and compute nodes share an
architecture, since we don't support heterogeneous clusters today.
These images differ in version from the AMD64 variants because the s390x
variants don't exist. Support for s390x was added after 4.2 was
released. The version used here has been the one tested in CI.

This was generated with the following:

./hack/update-rhcos-bootimage.py https://releases-art-rhcos.svc.ci.openshift.org/art/storage/releases/rhcos-4.2-s390x/42s390x.81.20191224.0/meta.json s390x
@crawford
Copy link
Contributor Author

@wking Good catches. I've updated the commits to address both.

@crawford crawford changed the title *: enable s390x bootimage support Bug 1793128: enable s390x bootimage support Jan 20, 2020
@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jan 20, 2020
@openshift-ci-robot
Copy link
Contributor

@crawford: This pull request references Bugzilla bug 1793128, which is invalid:

  • expected Bugzilla bug 1793128 to depend on a bug in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but no dependents were found

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

In response to this:

Bug 1793128: enable s390x bootimage support

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.

@crawford
Copy link
Contributor Author

We've come to an agreement that this is the desired image.

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2020
@crawford crawford removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jan 20, 2020
@crawford crawford added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jan 20, 2020
@crawford
Copy link
Contributor Author

I'm manually changing the BZ validity label because this change cannot (at this time) be verified in the 4.3 branch. Once Z ships, we will then start on forward-porting these changes to the release-4.3 branch. A little backward, I know.

@patrickdillon
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@crawford crawford added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jan 20, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@crawford
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 5bc9a4f into openshift:release-4.2 Jan 21, 2020
@openshift-ci-robot
Copy link
Contributor

@crawford: Bugzilla bug 1793128 is in an unrecognized state (MODIFIED) and will not be moved to the MODIFIED state.

In response to this:

Bug 1793128: enable s390x bootimage support

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.

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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants