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.

Sign up for GitHub

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

Jump to bottom

Integrate the JLink workflow and scripts into the Ubi9 container sources #401

Merged
merged 15 commits into from
Dec 13, 2023
Merged

Integrate the JLink workflow and scripts into the Ubi9 container sources #401

merged 15 commits into from
Dec 13, 2023

Conversation

Josh-Matsuoka
Copy link
Contributor

Initial implementation of adding the jlink scripts to the openjdk container sources.

  • Adds a Jlink module containing the scripts and an installation script along the lines of the other modules
  • Adds the jlink module as a dependency of the S2I bash module
  • Invokes the jlink workflow following the maven s2i build, once the artifacts are build we're good to go to perform the dependency analysis and create the JRE.

Related JIRA: https://issues.redhat.com/browse/OPENJDK-2024

@Josh-Matsuoka
Adding Jmods RPM to the list of RPMS to install for the UBI9 images.
8938ae2 
Resolves OPENJDK-2181
@Josh-Matsuoka
Initial implementation of a Jlink Module in the container sources.
e28d02d 
Copies the scripts into the S2I artifacts directory

Resolves OPENJDK-2178
@Josh-Matsuoka
Initial implementation of hooking the JLink scripts into the S2I
3b9787e 
process.

Resolves OPENJDK-2179, OPENJDK-2180
jmtd
jmtd reviewed Sep 21, 2023
View reviewed changes
modules/s2i/core/module.yaml Outdated Show resolved Hide resolved
jmtd
jmtd reviewed Sep 21, 2023
View reviewed changes
ubi9-openjdk-11.yaml Outdated Show resolved Hide resolved
@jmtd

This comment was marked as outdated.

Sign in to view
@jmtd
Copy link
Member

jmtd commented Nov 1, 2023

Great stuff, all my comments addressed!
As of 0812d30, the S2I build fails (at a later stage to last time):

I1101 10:51:13.890717  615424 sti.go:715] /opt/jboss/container/java/jlink/mkdeps.sh: line 21: JAVA_APP_JAR: unbound variable

Fixing this properly will take a bit of thought: JAVA_APP_JAR is currently undocumented, and otherwise only referenced in the run-java.sh script, and there is (almost) no scripts/shell libraries in common between the S2I assemble and the run script. I'll talk about that in more detail on our call. For now, I'm going to explicitly define it the environment during testing.

jmtd
jmtd reviewed Nov 8, 2023
View reviewed changes
modules/util/pathfinder/module.yaml Outdated
as well as library directories"

envs:
- name: JBOSS_CONTAINER_JAVA_S2I_MODULE
Copy link
Member

Choose a reason for hiding this comment

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

This should be JBOSS_CONTAINER_UTIL_PATHFINDER_MODULE

@jmtd
Copy link
Member

jmtd commented Nov 8, 2023 •
edited
Loading

THe new module is going to need a script (conventionally named configure.sh in other modules) to perform a copy of the stuff in artifacts/ into the container image. Also a reference to it in the module.yaml e.g.

+execute:
+- script: configure.sh

Josh-Matsuoka and others added 6 commits November 15, 2023 03:58
@Josh-Matsuoka
Fix ordering of pathfinder, add logging, add configure script
1d5399e
@jmtd
Serially copy JARs when flattening libdir
06357e1 
For some applications, multiple copies of JARs may exist in
JAVA_LIB_DIR. If >1 of those filenames end up collated into the same
`cp` invocation, the result will fail ("cp: will not overwrite
just-created...)

The solution is to serially invoke cp, one per library. This will
cause duplicate filenames to overwrite, which is fine if they are
the same library, but might mask a different problem if they aren't.

Signed-off-by: Jonathan Dowland <[email protected]>
@jmtd
fix syntax/typo around shell pipeline
b881fff 
The first line is a shell redirection so the first processes's
stdin corresponds to a file; the second line is the first
process. Remove the stray pipeline between them.

Signed-off-by: Jonathan Dowland <[email protected]>
@jmtd
jlink/mkdeps: report on non-0 return code from jdeps
8472ec3 
Signed-off-by: Jonathan Dowland <[email protected]>
@jmtd
jlink: install binutils for objcopy
687e59b 
Signed-off-by: Jonathan Dowland <[email protected]>
@jmtd
Define S2I_JLINK_OUTPUT_PATH for jlinked runtime destination
9433d1a 
Signed-off-by: Jonathan Dowland <[email protected]>
jmtd
jmtd approved these changes Dec 13, 2023
View reviewed changes
Copy link
Member

@jmtd jmtd left a comment

Choose a reason for hiding this comment

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

I think this is ready to be merged to the dev branch, thank you for your hard work!

@jmtd jmtd merged commit 249397b into rh-openjdk:ubi9 Dec 13, 2023
2 of 6 checks passed
jmtd added a commit that referenced this pull request Dec 13, 2023
@jmtd
Revert "Merge pull request #401 from Josh-Matsuoka/ubi9-jlink"
ae652d8 
This reverts commit 249397b, reversing
changes made to 4bc78f0.
@Josh-Matsuoka Josh-Matsuoka mentioned this pull request Apr 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmtd jmtd jmtd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Josh-Matsuoka @jmtd