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

CON-1203: Gradle plugin using conclave-build container for GraalVM on Linux #101

Merged
merged 45 commits into from
Dec 7, 2022

Conversation

anien
Copy link
Contributor

@anien anien commented Nov 21, 2022

Originally, enclaves were built in the docker container only on macOS and Windows. Now, all GraalVM enclaves are built in the conclave-build container regardless the OS. Gramine enclaves are still built outside this container, therefore python, Gramine, etc still do need to be installed on the machines.

Docker is now installed in the integration-tests docker container. Conclave-build container is now run with the same user as the integration-tests container - this is to prevent permission issues when deleting files.

@anien anien marked this pull request as draft November 21, 2022 13:04
@anien anien marked this pull request as ready for review November 23, 2022 11:49
@shamsasari
Copy link
Contributor

shamsasari commented Nov 23, 2022

Caused by: net.rubygrapefruit.platform.NativeException: Could not start 'docker'
14:59:01     at net.rubygrapefruit.platform.internal.DefaultProcessLauncher.start(DefaultProcessLauncher.java:27)
14:59:01     at net.rubygrapefruit.platform.internal.WrapperProcessLauncher.start(WrapperProcessLauncher.java:36)
14:59:01     at org.gradle.process.internal.ExecHandleRunner.startProcess(ExecHandleRunner.java:98)
14:59:01     at org.gradle.process.internal.ExecHandleRunner.run(ExecHandleRunner.java:71)
14:59:01     ... 7 more
14:59:01   Caused by: java.io.IOException: Cannot run program "docker" (in directory "/data/buildAgent/work/a4c864046b3e21f/integration-tests/general/threadsafe-enclave"): error=2, No such file or directory
14:59:01     at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1143)
14:59:01     at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1073)
14:59:01     at net.rubygrapefruit.platform.internal.DefaultProcessLauncher.start(DefaultProcessLauncher.java:25)
14:59:01     ... 10 more
14:59:01   Caused by: java.io.IOException: error=2, No such file or directory
14:59:01     at java.base/java.lang.ProcessImpl.forkAndExec(Native Method)
14:59:01     at java.base/java.lang.ProcessImpl.<init>(ProcessImpl.java:314)
14:59:01     at java.base/java.lang.ProcessImpl.start(ProcessImpl.java:244)
14:59:01     at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1110)

It can't find docker, which doesn't make sense, until you realise this is inside the integration-tests container! So perhaps we have install docker inside that container??

@anien
Copy link
Contributor Author

anien commented Nov 24, 2022

Docker is available inside integrations-tests container. We already expose docker socket to the container by having
-v /var/run/docker.sock:/var/run/docker.sock parameter.
According this: https://jpetazzo.github.io/2015/09/03/do-not-use-docker-in-docker-for-ci/, that option allows you to create a 'sibling' containers, instead of a child containers, which is better approach. The Could not start 'docker' error is just hiding the real problem. If you add --privileged to docker command (which the article above recommend as well), the error changes to Permission denied. We do funky things with users in our docker containers, which I don't understand yet. I think we now only need to add some users we have in the docker container to the docker group inside the integration-tests container, so it can start the docker process.

.splitToSequence("\n")
.single { it.startsWith("Location: ") }
.substringAfter("Location: ")
// non-python. https://r3-cev.atlassian.net/browse/CON-1181 and https://r3-cev.atlassian.net/browse/CON-1229.
Copy link
Contributor

Choose a reason for hiding this comment

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

Only the python bits are left from this TODO, so update accordingly. No need for the CON-1181 reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You still need python, pip and jep libraries installed on a local machine for Python enclaves. Updated accordingly.

@shamsasari
Copy link
Contributor

Yay, the integration tests now pass! Now to add that flag and update the error message to point to the GItHub issues page (and merge conflict!)

Copy link
Contributor

@shamsasari shamsasari left a comment

Choose a reason for hiding this comment

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

Some more comments, hopefully the last.

@@ -40,11 +44,14 @@ open class LinuxExec @Inject constructor(objects: ObjectFactory) : ConclaveTask(
conclaveBuildDir
)
} catch (e: Exception) {
logger.info("Docker build of conclave-build failed. Re-run your build with '--info' and " +
"submit a new GitHub issue https://github.com/R3Conclave/conclave-core-sdk/issues/new", e)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to mention submitting a GH issue here. It could be they find out what the issue is by enabling --info and fix the issue themselves. Mentioning it in the exception message is sufficient.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hang on, we've got this wrong! The mention of the --info flag needs to be in the exception below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, that now makes sense :) I'll fix that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully this now make sense.

@anien anien requested a review from shamsasari December 6, 2022 23:04
@shamsasari
Copy link
Contributor

Can you update PR description.

@shamsasari shamsasari changed the title CON-1203: Gradle plugin to use conclave-build container on Linux CON-1203: Gradle plugin using conclave-build container for GraalVM on Linux Dec 7, 2022
Copy link
Contributor

@shamsasari shamsasari left a comment

Choose a reason for hiding this comment

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

Looking good!

@JohnJoser3 remind me that we need to sit down regarding documentation for this and all the Gramine stuff.

@github-actions github-actions bot merged commit 4dc73d2 into master Dec 7, 2022
@github-actions github-actions bot deleted the anna/CON-1203-1 branch December 7, 2022 12:08
github-actions bot pushed a commit that referenced this pull request Dec 19, 2022
#101 accidently made building on MacOS and Windows conditional on the new `buildInDocker` flag. This is not correct as Docker is always required for these environments.
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.

None yet

2 participants