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

Enable Semeru Cloud Compiler for InstantOn #440

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Conversation

leochr
Copy link
Member

@leochr leochr commented Jun 19, 2023

What this PR does / why we need it?:

Does this PR introduce a user-facing change?

  • User guide
  • CHANGELOG.md

Which issue(s) this PR fixes:

Fixes #

Signed-off-by: Leo Christy Jesuraj <leojc@ca.ibm.com>
@leochr leochr requested review from arturdzm and tjwatson June 19, 2023 13:17
@tjwatson
Copy link
Member

@anjumfatima90 can you try out the operator with InstantOn?

@tjwatson
Copy link
Member

Fixes #438

@leochr
Copy link
Member Author

leochr commented Jul 11, 2023

We didn't feel comfortable releasing something that wasn't tested. So this enhancement is only available from a pre-release driver. Follow the instructions in our wiki to install the driver: https://github.com/OpenLiberty/open-liberty-operator/wiki

Adding the CatalogSource details here as well (for Step 2.2) in case our Wiki is changed to some other driver:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: olo-catalog
  namespace: openshift-marketplace
spec:
  displayName: Open Liberty instanton
  image: 'icr.io/appcafe/open-liberty-operator-catalog@sha256:5298bed424f87f480cb0756d3dbf4cc309af1b0c9459034c814dbcc55680b808'
  sourceType: grpc

"export OPENJ9_JAVA_OPTIONS=\"$OPENJ9_JAVA_OPTIONS " +
jitSeverOptions +
"export OPENJ9_JAVA_OPTIONS=\"$OPENJ9_JAVA_OPTIONS " + jitSeverOptions +
"\" && export OPENJ9_RESTORE_JAVA_OPTIONS=\"$OPENJ9_RESTORE_JAVA_OPTIONS " + jitSeverOptions +
Copy link
Member

Choose a reason for hiding this comment

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

@NottyCode points out that this handles OPENJ9_JAVA_OPTIONS and OPENJ9_RESTORE_JAVA_OPTIONS values being set in kubernetes config but it doesn't handle cases where the container image has already set the env. For example, using ENV in their Dockerfile. Is this a concern? If so it has been an issue ever since the operator started auto setting the OPENJ9_JAVA_OPTIONS

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not a concern. The commands will be executed at runtime inside the container. So the jitServerOptions will be appended to OPENJ9_JAVA_OPTIONS value (the value could have come from container ENV or k8s env). In fact, we took this approach in the first place to preserve the OPENJ9_JAVA_OPTIONS ENV specified in the Liberty container image:

ENV RANDFILE=/tmp/.rnd \
    OPENJ9_JAVA_OPTIONS="-XX:+IgnoreUnrecognizedVMOptions -XX:+IdleTuningGcOnIdle -Xshareclasses:name=openj9_system_scc,cacheDir=/opt/java/.scc,readonly,nonFatal -Dosgi.checkConfiguration=false"

@tjwatson
Copy link
Member

Can we get this change merged soon?

@leochr
Copy link
Member Author

leochr commented Sep 28, 2023

@tjwatson We've been holding off on merging this until the fix for the issue mentioned here is released and validated. Has that happened?

@tjwatson
Copy link
Member

@tjwatson We've been holding off on merging this until the fix for the issue mentioned here is released and validated. Has that happened?

Can we merge this fix ahead of the available Semeru containers with the fix available (expected mid October)? Perhaps it would be better to have that error mentioned occur now instead of silently ignoring the JIT Server configuration from the operator.

@leochr
Copy link
Member Author

leochr commented Sep 28, 2023

An Operator release prior to mid-October is not in the plan. So we'll proceed to merge. It'll still be good to validate once the Semeru fix is available.

@leochr
Copy link
Member Author

leochr commented Sep 28, 2023

@arturdzm please review and merge when you get a chance. Thanks

@leochr
Copy link
Member Author

leochr commented Oct 23, 2023

@arturdzm please review. Thanks

@leochr leochr merged commit 7e78ef7 into main Oct 26, 2023
@leochr leochr deleted the instanton-support-lcj branch October 26, 2023 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants