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

Add tests to verify JITServer with SSL #18262

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

SajinaKandy
Copy link
Contributor

Add tests to the existing jitserver tests under cmdLineTest for checking/verifying SSL connections with JITServer. This is part 2 for the work done in #17985 .

Closes: ##17967

@SajinaKandy
Copy link
Contributor Author

@mpirvu Can you please review?

<command>bash $SCRIPPATH$ $TEST_RESROOT$ $TEST_JDK_BIN$ "$DEFAULT_JITSERVER_OPTIONS$" "$ENABLE_JITSERVER$ $JITSERVER_VERBOSE$ $JITSERVER_SSL1$" false</command>
<output type="success" caseSensitive="no" regex="yes" javaUtilPattern="yes">(java|openjdk|semeru) version</output>
<output type="required" caseSensitive="no" regex="no">JITServer Client Mode.</output>
<output type="success" caseSensitive="no" regex="no">Successfully initialized SSL context</output>
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember discussing that the test succeeds when any of the type="success" criteria is met.
If that is the case, then "Successfully initialized SSL context" is sufficient to make the test pass, but in reality that is not enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this test we can make that condition type="required" for "Successfully initialized SSL context" which will enforce the test to look for this condition and make the test pass only if the condition is met.
However the test will only search for the condition "Successfully initialized SSL context" in the output of the test which includes logs from the server and the client and the test will pass if either client or server logs contain the text.

I can remove the -Xjit:verbose={JITServer} from the server side to avoid checking the logs from the server and change few conditions in the test which currently expects server side logs.

Or a better way might be to make the condition JITServer::StreamFailure: Failed to SSL_connect as required condition in the failure scenario and for success scenario make the Connected to a server as required condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the server side verbose logging and modified the conditions to check if the connection was success/failure (based on the test criteria) as a required condition.

@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Oct 10, 2023
@mpirvu mpirvu self-assigned this Oct 10, 2023
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM

@mpirvu
Copy link
Contributor

mpirvu commented Oct 11, 2023

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit,alinux64jit jdk17

@SajinaKandy
Copy link
Contributor Author

The ppc_le test failed with the error:

22:04:58  compile:
22:04:58       [echo] Ant version is Apache Ant(TM) version 1.10.5 compiled on July 10 2018
22:04:58       [echo] ============COMPILER SETTINGS============
22:04:58       [echo] ===fork:                         yes
22:04:58       [echo] ===executable:                   /home/jenkins/workspace/Test_openjdk17_j9_sanity.functional_ppc64le_linux_jit_Personal_testList_1/openjdkbinary/j2sdk-image/bin/javac
22:04:58       [echo] ===debug:                        on
22:04:58       [echo] ===destdir:                      /home/jenkins/workspace/Test_openjdk17_j9_sanity.functional_ppc64le_linux_jit_Personal_testList_1/aqa-tests/TKG/../../jvmtest/functional/cmdLineTests/criu
22:04:58       [echo] ===addExports:        --add-exports java.base/openj9.internal.criu=ALL-UNNAMED --add-exports java.base/jdk.internal.misc=ALL-UNNAMED
22:04:58       [echo] ===enablePreview:     
22:04:58      [javac] Compiling 16 source files to /home/jenkins/workspace/Test_openjdk17_j9_sanity.functional_ppc64le_linux_jit_Personal_testList_1/aqa-tests/functional/cmdLineTests/criu/bin
22:05:05      [javac] /home/jenkins/workspace/Test_openjdk17_j9_sanity.functional_ppc64le_linux_jit_Personal_testList_1/aqa-tests/functional/cmdLineTests/criu/src/org/openj9/criu/TimeChangeTest.java:116: error: cannot find symbol
22:05:05      [javac] 		long lastRestoreTime = InternalCRIUSupport.getLastRestoreTime();
22:05:05      [javac] 		                                          ^
22:05:05      [javac]   symbol:   method getLastRestoreTime()
22:05:05      [javac]   location: class InternalCRIUSupport
22:05:05      [javac] 1 error
22:05:05  
22:05:05  BUILD FAILED

Seems due to #18184.

The other failure is on x86_64 which fails with a timeout error while copying files:

23:33:40  Exception: java.io.IOException: Failed to copy file:/var/lib/jenkins/jobs/test.getDependency/builds/882/archive/jython-standalone.jar to /home/jenkins/workspace/Test_openjdk17_j9_sanity.functional_x86-64_linux_jit_Personal_testList_1/aqa-tests/TKG/lib/jython-standalone.jar
[Pipeline] echo
23:33:40  Cannot run copyArtifacts from test.getDependency. Skipping copyArtifacts...

@mpirvu
Copy link
Contributor

mpirvu commented Oct 12, 2023

jenkins test sanity xlinuxjit jdk17

@JasonFengJ9
Copy link
Member

Seems due to #18184.

Pls rebase to fix the compilation failure.

@mpirvu
Copy link
Contributor

mpirvu commented Oct 12, 2023

jenkins test sanity xlinuxjit jdk17

@mpirvu
Copy link
Contributor

mpirvu commented Oct 13, 2023

jenkins test sanity plinuxjit jdk17

@SajinaKandy
Copy link
Contributor Author

The tests on ppcle (Hostname: cent7-ppcle-3) are failing with the error:
CRIU needs to have the CAP_SYS_ADMIN or the CAP_CHECKPOINT_RESTORE capability
This is an issue with CRIU capability on the machine discussed here: #18144

@mpirvu
Copy link
Contributor

mpirvu commented Oct 13, 2023

@SajinaKandy Could you please fix the " Eclipse Foundation Contributor Agreement validation"? Maybe you have to revalidate again?

Add tests to the existing jitserver tests under cmdLineTest for checking
/verifying SSL connections with JITServer.
This is part 2 for the work done in eclipse-openj9#17985 .

Closes: #eclipse-openj9#17967
Signed-off-by: SajinaKandy <[email protected]>
@SajinaKandy
Copy link
Contributor Author

@mpirvu the eclipse eca validation is successful now.

@mpirvu
Copy link
Contributor

mpirvu commented Oct 16, 2023

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit,alinux64jit jdk17

@SajinaKandy
Copy link
Contributor Author

Fails again with the same error on ppc_le CRIU needs to have the CAP_SYS_ADMIN or the CAP_CHECKPOINT_RESTORE :

[2023-10-16T18:39:30.485Z]  [OUT] Total checkpoint(s) 1:
[2023-10-16T18:39:30.485Z]  [OUT] Pre-checkpoint
[2023-10-16T18:39:30.485Z]  [OUT] Performing CRIUSupport.checkpointJVM(), current thread name: main, Mon Oct 16 18:39:29 UTC 2023, System.currentTimeMillis(): 1697481569361, System.nanoTime(): 953125349648965
[2023-10-16T18:39:30.485Z]  [OUT] CRIU needs to have the CAP_SYS_ADMIN or the CAP_CHECKPOINT_RESTORE capability: 
[2023-10-16T18:39:30.485Z]  [OUT] setcap cap_checkpoint_restore+eip criu
[2023-10-16T18:39:30.485Z]  [OUT] Exception in thread "main" org.eclipse.openj9.criu.SystemCheckpointException: Could not dump the JVM processes, err=-52
[2023-10-16T16:34:44.233Z] dynamicAgents: [fyre]
[Pipeline] node
[2023-10-16T16:34:44.247Z] Running on ub16p8j93 in /home/jenkins/workspace/Test_openjdk17_j9_sanity.functional_ppc64le_linux_jit_Personal

@mpirvu
Copy link
Contributor

mpirvu commented Oct 16, 2023

Merging based on the comment here: #18262 (comment)

@mpirvu mpirvu merged commit f418680 into eclipse-openj9:master Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jitserver Artifacts related to JIT-as-a-Service project
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants