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

Add CRIU security hooks #14824

Merged
merged 1 commit into from
May 16, 2022
Merged

Add CRIU security hooks #14824

merged 1 commit into from
May 16, 2022

Conversation

ZainabF92
Copy link

@ZainabF92 ZainabF92 commented Mar 30, 2022 •
edited by keithc-ca

Ensures that CRIUSEC is the only provider available during checkpoint.
See PR: ibmruntimes/openj9-openjdk-jdk11#510

Signed-off-by: Zainab Fatmi zainab@ibm.com

@tajila tajila requested a review from JasonFengJ9 March 30, 2022 19:48
@ZainabF92 ZainabF92 force-pushed the criu-sec branch 4 times, most recently from 6160842 to f57497b Compare April 1, 2022 14:30
@ZainabF92 ZainabF92 mentioned this pull request Apr 6, 2022
Closed
@ZainabF92 ZainabF92 force-pushed the criu-sec branch 4 times, most recently from 0ce4e3d to 33a7c36 Compare April 7, 2022 13:07
keithc-ca
keithc-ca reviewed Apr 7, 2022
View reviewed changes
jcl/src/java.base/share/classes/module-info.java.extra Outdated Show resolved Hide resolved
jcl/src/openj9.criu/share/classes/org/eclipse/openj9/criu/CRIUSupport.java Outdated Show resolved Hide resolved
jcl/src/openj9.criu/share/classes/org/eclipse/openj9/criu/SecurityProviders.java Outdated Show resolved Hide resolved
jcl/src/openj9.criu/share/classes/org/eclipse/openj9/criu/SecurityProviders.java Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/criuSecurityScript.sh Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/src/CRIUSecurityTest.java Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/src/CRIUSecurityTest.java Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/src/CRIUSecurityTest.java Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/src/CRIUSecurityTest.java Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/src/CRIUSecurityTest.java Outdated Show resolved Hide resolved
@tajila
Copy link
Contributor

tajila commented May 2, 2022

@ZainabF92 please rebase on master.

@ZainabF92 ZainabF92 force-pushed the criu-sec branch 3 times, most recently from 726064d to 53df7b1 Compare May 2, 2022 18:15
@ZainabF92
Copy link
Author

rebased and added another test case

@ZainabF92 ZainabF92 force-pushed the criu-sec branch 2 times, most recently from a5892ed to c3caab3 Compare May 10, 2022 18:54
@tajila
Copy link
Contributor

tajila commented May 11, 2022

Any further comments @keithc-ca ?

@keithc-ca
Copy link
Contributor

This will need to be adjusted to account for the changes in #15047 (and ibmruntimes/openj9-openjdk-jdk11#523).

@keithc-ca keithc-ca self-assigned this May 12, 2022
@JasonFengJ9 JasonFengJ9 mentioned this pull request May 12, 2022
Merged
@ZainabF92 ZainabF92 force-pushed the criu-sec branch 2 times, most recently from 8aed91f to fd6b2ec Compare May 12, 2022 18:49
@tajila
Copy link
Contributor

tajila commented May 12, 2022

@keithc-ca This is ready for another look

@keithc-ca
Copy link
Contributor

jenkins test sanity alinux64,xlinuxcriu jdk11

@keithc-ca
Copy link
Contributor

There are CRIU test failures.

Add security hooks and test cases
bc9e377 
Signed-off-by: Zainab Fatmi <zainab@ibm.com>
@keithc-ca
Copy link
Contributor

@ZainabF92 @tajila What about the other failure mode in https://openj9-jenkins.osuosl.org/job/Test_openjdk11_j9_sanity.functional_x86-64_linux_criu_Personal/15:

Test result: FAILED
Output from test:
 [OUT] start running script
 [OUT] Three checkpoints:
 [OUT] Pre-checkpoint
 [OUT] Post-checkpoint 1
 [OUT] finished script
 [ERR] /home/jenkins/workspace/Test_openjdk11_j9_sanity.functional_x86-64_linux_criu_Personal_testList_1/aqa-tests/TKG/../../jvmtest/functional/cmdLineTests/criu/criuScript.sh: line 26:  3710 Killed                  $2 -XX:+EnableCRIUSupport $3 -cp "$1/criu.jar" CRIUSimpleTest $4 > testOutput 2>&1
 [ERR] pie: 3710: Error (criu/pie/restorer.c:1790): Unable to create a thread: 3739
 [ERR] pie: 3710: Error (criu/pie/restorer.c:1925): Restorer fail 3710
 [ERR] Error (criu/cr-restore.c:2447): Restoring FAILED.
 [ERR] pie: 3710: Error (criu/pie/restorer.c:1790): Unable to create a thread: 3739
 [ERR] pie: 3710: Error (criu/pie/restorer.c:1925): Restorer fail 3710
 [ERR] Error (criu/cr-restore.c:2447): Restoring FAILED.

Does #14974 cover that?

@JasonFengJ9
Copy link
Member

Does #14974 cover that?

I think so, it was intermittent as per the grinder at ibmruntimes/openj9-openjdk-jdk18#39 (comment)

@tajila
Copy link
Contributor

tajila commented May 13, 2022

@keithc-ca
Yes #14974 tracks that failure

@tajila
Copy link
Contributor

tajila commented May 13, 2022

jenkins test sanity xlinuxcriu jdk11

@tajila
Copy link
Contributor

tajila commented May 16, 2022

@keithc-ca any more comments?

keithc-ca
keithc-ca approved these changes May 16, 2022
View reviewed changes
Copy link
Contributor

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

I'm not excited that we have (likely more) intermittent failures with this, but we do have an issue to track fixing those problems and the scope is limited to the CRIU feature.

@keithc-ca keithc-ca merged commit 6e6bc65 into eclipse-openj9:master May 16, 2022
@LongyuZhang LongyuZhang mentioned this pull request May 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tajila tajila tajila approved these changes

@JasonFengJ9 JasonFengJ9 JasonFengJ9 approved these changes

@keithc-ca keithc-ca keithc-ca approved these changes

Assignees

@keithc-ca keithc-ca

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ZainabF92 @tajila @keithc-ca @JasonFengJ9