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

8265690: Use the latest Ubuntu base image version in Docker testing #3605

Closed
wants to merge 1 commit into from

Conversation

@shipilev
Copy link
Contributor

@shipilev shipilev commented Apr 21, 2021

Use latest Ubuntu base image to get latest glibc, so that host-built JDK has a higher chance to pass. Current "oraclelinux:7.6" image apparently has glibc 2.27. Please see the bug for more discussion.

Additional testing:

  • hotspot_containers tests on Ubuntu 20.04 (glibc 2.31) -- used to fail, now passes
  • hotspot_containers tests on Debian 9 (glibc 2.24) -- continues to pass
  • hotspot_containers tests on Centos 7 (glibc 2.17) -- continues to pass

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8265690: Use the latest Ubuntu base image version in Docker testing

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3605/head:pull/3605
$ git checkout pull/3605

Update a local copy of the PR:
$ git checkout pull/3605
$ git pull https://git.openjdk.java.net/jdk pull/3605/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3605

View PR using the GUI difftool:
$ git pr show -t 3605

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/3605.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Apr 21, 2021

👋 Welcome back shade! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Apr 21, 2021

@shipilev The following label will be automatically applied to this pull request:

  • hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

Loading

@shipilev shipilev marked this pull request as ready for review Apr 21, 2021
@openjdk openjdk bot added the rfr label Apr 21, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 21, 2021

Webrevs

Loading

Copy link
Contributor

@mgkwill mgkwill left a comment

Seems reasonable to me.

My approval means little however. 😏

Loading

@shipilev shipilev marked this pull request as draft Apr 22, 2021
@shipilev
Copy link
Contributor Author

@shipilev shipilev commented Apr 22, 2021

Turns out, the tests are throwing SkippedException on error. This masked the failure to resolve oraclelinux:latest... Looking at it a bit more. Edit: it seems ubuntu:latest is the way to go.

Loading

@openjdk openjdk bot removed the rfr label Apr 22, 2021
@shipilev shipilev changed the title 8265690: Use latest base image version in Docker testing 8265690: Use the latest Ubuntu base image version in Docker testing Apr 22, 2021
@shipilev shipilev force-pushed the JDK-8265690-docker-images branch from d36717b to ad904a7 Apr 22, 2021
@shipilev shipilev marked this pull request as ready for review Apr 22, 2021
@openjdk openjdk bot added the rfr label Apr 22, 2021
Copy link
Contributor

@jerboaa jerboaa left a comment

This looks fine to me. It makes docker tests compatible with more systems by default. On the other hand, it'll require for the Oracle config to set -Djdk.test.docker.image.name=oraclelinux and -Djdk.test.docker.image.version=(latest|7.6) depending on architecture. Paging @iignatev @dholmes-ora for input.

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Apr 26, 2021

@shipilev This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8265690: Use the latest Ubuntu base image version in Docker testing

Reviewed-by: sgehwolf, iignatyev, mseledtsov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 91 new commits pushed to the master branch:

  • b2628d1: 8263972: C2: LoadVector/StoreVector type mismatch in MemNode::can_see_stored_value()
  • 377b346: 8264752: SIGFPE crash with option FlightRecorderOptions:threadbuffersize=30M
  • dc323a9: 8263421: Module image file is opened twice during VM startup
  • fbfd4ea: 8265914: Duplicated NotANode and not_a_node
  • 9481fad: 8163367: Test javax/swing/JComboBox/8033069/bug8033069NoScrollBar.java javax/swing/JComboBox/8033069/bug8033069ScrollBar.java fails intermittently
  • 9adbf15: 8265995: Shenandoah: Move ShenandoahInitMarkRootsClosure close to its use
  • 879a77f: 8265757: stack-use-after-scope in perfMemory_posix.cpp get_user_name_slow()
  • e4be968: 8265980: Fix systemDictionary and loaderConstraints printing
  • f6e26f6: 8265756: AArch64: initialize memory allocated for locals according to Windows AArch64 stack page growth requirement in template interpreter
  • 0a4c338: 8263432: javac may report an invalid package/class clash on case insensitive filesystems
  • ... and 81 more: https://git.openjdk.java.net/jdk/compare/a93d911954555c957b7a134df0268de500f2ade3...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

Loading

@openjdk openjdk bot added the ready label Apr 26, 2021
@iignatev
Copy link
Member

@iignatev iignatev commented Apr 26, 2021

Severin, thanks for bringing this to our attention.

As far as I can see, we explicitly specify jdk.test.docker.image.(name|version) properties in all Oracle testing, so the change impacts only local uses, and there it would seem to make it actually easier for people to use. with that being said, I'd like to get an OK from Misha (@mseledts) as he oversees our container testing.

Cheers,
-- Igor

Loading

@mseledts
Copy link
Member

@mseledts mseledts commented Apr 26, 2021

As Igor has pointed out, we at Oracle explicitly use jdk.test.docker.image.(name|version) to specify base image+version for container test execution. Hence, in principle I have no objection to this change. Couple of notes though:

  • this change might potentially affect other parties to OpenJDK that may rely on default base image being OracleLinux, who are not currently using -Ddk.test.docker.image
  • one of recommended container best practices is to use an explicit version rather than "latest", for more deterministic outcomes

Loading

@shipilev
Copy link
Contributor Author

@shipilev shipilev commented Apr 27, 2021

Well, I think latest is fine here, and it matches what other arches are doing in the same test. This also helps us not to update the test every time new development environments appear. Stable CIs should probably just set the test properties for the environments that CI servers run. That would also be the way out for those who may potentially run in incompatibilities. I've checked that the more or less wide spectrum of environments is fine.

OK for me to push this?

Loading

@jerboaa
Copy link
Contributor

@jerboaa jerboaa commented Apr 27, 2021

OK for me to push this?

Given the feedback we've got, I'd say yes.

Loading

@mseledts
Copy link
Member

@mseledts mseledts commented Apr 27, 2021

Change looks good to me.

Loading

@shipilev
Copy link
Contributor Author

@shipilev shipilev commented Apr 27, 2021

Thanks all!

/integrate

Loading

@openjdk openjdk bot closed this Apr 27, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Apr 27, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 27, 2021

@shipilev Since your change was applied there have been 91 commits pushed to the master branch:

  • b2628d1: 8263972: C2: LoadVector/StoreVector type mismatch in MemNode::can_see_stored_value()
  • 377b346: 8264752: SIGFPE crash with option FlightRecorderOptions:threadbuffersize=30M
  • dc323a9: 8263421: Module image file is opened twice during VM startup
  • fbfd4ea: 8265914: Duplicated NotANode and not_a_node
  • 9481fad: 8163367: Test javax/swing/JComboBox/8033069/bug8033069NoScrollBar.java javax/swing/JComboBox/8033069/bug8033069ScrollBar.java fails intermittently
  • 9adbf15: 8265995: Shenandoah: Move ShenandoahInitMarkRootsClosure close to its use
  • 879a77f: 8265757: stack-use-after-scope in perfMemory_posix.cpp get_user_name_slow()
  • e4be968: 8265980: Fix systemDictionary and loaderConstraints printing
  • f6e26f6: 8265756: AArch64: initialize memory allocated for locals according to Windows AArch64 stack page growth requirement in template interpreter
  • 0a4c338: 8263432: javac may report an invalid package/class clash on case insensitive filesystems
  • ... and 81 more: https://git.openjdk.java.net/jdk/compare/a93d911954555c957b7a134df0268de500f2ade3...master

Your commit was automatically rebased without conflicts.

Pushed as commit b67b2b1.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Loading

@shipilev shipilev deleted the JDK-8265690-docker-images branch May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants