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

OpenFL-Gramine fixes for SGX execution [reupload] #570

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

Giemp95
Copy link

@Giemp95 Giemp95 commented Nov 3, 2022

[This PR is a copy of a previous one closed by error due to some branching reorganization]

This PR aims to make the OpenFL-Gramine workflow smoother for the user.

I have modified the openfl.manifest.template to include the mount of the /tmp directory since the lack of it was causing errors in the execution of the container.
Furthermore, I have updated the Dockerfile.gramine file so that now it will download and install the last development branch of the OpenFL software. This way, the version of OpenFL installed inside the container will be the same one used for development, and there will be no mismatch between OpenFL versions inside and outside the container. Once a new stable version of OpenFL is released, this will need to be changed to point to the last stable version.
Lastly, I've extended the Manual.md file to include many small things I learned while trying to make the OpenFL-Gramine example work. Now it should be easier for a newbie to run this application.

I have tested all the changes I made, and they are working as they are.
The only difference to this PR is that, in my tests, the Docker image contained my project fork; in this PR I have changed that pointer to the developmental branch of OpenFL so that, if integrated, it will work out-of-the-box.

[Comments]
DL8: "are you sure that's the case? as far as i understand, the docker image is used, so aggregator and collaborators using it don't really need openfl installed"
Answer: "Yes, you're right that the training will be carried out with the Docker container, in which OpenFL will already be present. However, the user will still need the OpenFL functionalities available outside the Docker container to carry out the exchange and validation of the certificates, as reported in step 7 of the actual OpenFL-gramine workflow reported in Manual.md."

DL8: "does gramine support a tmpfs-style mount? if /tmp is used only for temporary files, it may be better to tell gramine that /tmp is in memory, so there will be no leftovers after its execution"
DL8: "i looked in gramine docs and found the tmpfs filesystem type. i believe it can be more appropriate in this context"
Answer: "I've changed the mount type for /tmp from 'chmod' to 'tmpfs'"

igor-davidyuk: "Some manifest updates were done here #537"

@mansishr
Copy link
Collaborator

mansishr commented Nov 3, 2022

Thanks @Giemp95 for submitting this PR! There is some overlap between this PR and #537 that we should resolve. Regarding ensuring that we use the same version of OpenFL for workspace and for docker image, the team needs to reach a consensus on the thread going on in Issues #554.

openfl-gramine/MANUAL.md Outdated Show resolved Hide resolved
openfl-gramine/openfl.manifest.template Outdated Show resolved Hide resolved
@igor-davidyuk
Copy link
Contributor

Thank you for the extensions of the manual! I know that working on gramine was hard lately because of many non-merged fixes required to run an example.
Moreover, managing OpenFL versions locally on the building machine and in the Docker container may be tricky and confusing. So this temporary stub for installing develop branch will not solve the problem as I tried it before 🥲, I started to think that we need some Developer mode for the graminize command that will allow us to replicate local OpenFL build in Docker images.

@DL8
Copy link
Contributor

DL8 commented Nov 9, 2022

@igor-davidyuk I like the developer mode idea. To extend it further, I would also want it to be possible to choose which repository and branch/revision to clone in the image build. The motivation is this: for development purposes, I have a private fork of OpenFL with a couple of branches I'm working on. To make it work, I had to manually hard-code my fork's address and the branch I'm using (and remember to switch the branch name as needed).

Regarding certification and PKI work, I think an example of how to do it without installing OpenFL should at the very least be documented. I also encountered some technical challenges with it, so I think it's worth a separate issue.

Copy link
Author

@Giemp95 Giemp95 left a comment

Choose a reason for hiding this comment

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

Review accepted, manifest rolled back to latest develop commit

Comment on lines +8 to +9
git clone --single-branch --branch develop https://github.com/intel/openfl.git && \
pip install ./openfl
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this fix as we will introduce a flexible development mode instead

@@ -68,7 +71,6 @@ sgx.trusted_files = [
]

sgx.allowed_files = [
"file:/tmp",
"file:/etc",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also already fixed

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.

4 participants