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 Gunicorn example #80

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

sahason
Copy link
Contributor

@sahason sahason commented Oct 10, 2023

Add Gunicorn example


This change is Reviewable

Signed-off-by: Sonali Saha <[email protected]>
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @sahason)


gunicorn/gunicorn.manifest.template line 40 at r1 (raw file):


# BSD (flock) locks are currently experimental
sys.experimental__enable_flock = true

Does it really require flock locks? If you remove this option, Gunicorn fails?


gunicorn/README.md line 3 at r1 (raw file):

# Gunicorn example

This directory contains an example for running Gunicorn in Gramine, including the

Can you add official links to Gunicorn, and add a paragraph on what Gunicorn is? And why is it beneficial to put it inside Gramine?

Also, what is the relationship between Gunicorn and Flask?


gunicorn/README.md line 29 at r1 (raw file):

Without SGX:

gramine-direct gunicorn --workers 1 --timeout 600 main:app

Why not put these arguments as loader.argv (see https://gramine.readthedocs.io/en/stable/manifest-syntax.html#command-line-arguments)

Then you won't need to use an insecure loader.insecure__use_cmdline_argv.

Also, please describe why you must have workers == 1 and timeout == 600. Why not have the default settings?

Signed-off-by: Sonali Saha <[email protected]>
Copy link
Contributor Author

@sahason sahason left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv)


gunicorn/gunicorn.manifest.template line 40 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Does it really require flock locks? If you remove this option, Gunicorn fails?

For the sample application it is not required. But if any file is getting download from endpoints flock is required. I have removed this line.


gunicorn/README.md line 3 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Can you add official links to Gunicorn, and add a paragraph on what Gunicorn is? And why is it beneficial to put it inside Gramine?

Also, what is the relationship between Gunicorn and Flask?

Done.


gunicorn/README.md line 29 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why not put these arguments as loader.argv (see https://gramine.readthedocs.io/en/stable/manifest-syntax.html#command-line-arguments)

Then you won't need to use an insecure loader.insecure__use_cmdline_argv.

Also, please describe why you must have workers == 1 and timeout == 600. Why not have the default settings?

Modified the manifest file.

To workaround the timeout issue which happens due to gramine not supporting sharing file metadata (st_ctime) information between child and parent processes, the timeout is set to a higher value.

More details are here. I found a gap between Gunicorn execution in native vs gramine (Both gramine-direct and gramine-sgx) irrespective of the value of timeout. In native the flow works fine but in gramine the worker is up for some time ~(timeout-20) seconds then exit happens, and the worker is restarted again after timeout.
gunicorn-garmine.png
In Gunicorn timeout happens when time.time() - worker.tmp.last_update() <= self.timeout
https://github.com/benoitc/gunicorn/blob/430dcdd997b955fa1a8569a07d9cdcbac686fb35/gunicorn/arbiter.py#L498 & https://github.com/benoitc/gunicorn/blob/430dcdd997b955fa1a8569a07d9cdcbac686fb35/gunicorn/workers/workertmp.py#L49C24-L49C53. After every timeout/2 seconds the st_ctime of a tmpfs file is updated by fchmod https://github.com/benoitc/gunicorn/blob/430dcdd997b955fa1a8569a07d9cdcbac686fb35/gunicorn/workers/workertmp.py#L46. So, last updated time will get updated after each timeout/2 seconds.  The fchmod happens in a forked process and Gunicorn checks updated time using fstat in parent process in each one second interval. In gramine the file metadata information (st_ctime in this case) is not propagated from child to parent if there is any change in child process due to fchmod. I have found similar issues already reported in gramine Forked processes do not get updated file sizes when modified in a multi-process scenario · Issue #1134 · gramineproject/gramine (github.com)gramineproject/gramine#254. There is a work item to handle the shared tmpfs files gramineproject/gramine#584. Also note that we have to handle st_ctime modification for tmpfs files when fchmod is called when this fix is in.

Signed-off-by: Sonali Saha <[email protected]>
@anjalirai-intel
Copy link
Contributor

I tested this example currently on Ubuntu 22.04 system, I was able to launch the gunicorn app with both gramine-direct and gramine-sgx, but I am not able to kill the process. Everytime I have to hard kill both the child and parent process.

Starting GUNICORN app

$ gramine-sgx gunicorn & echo $! > server.PID
[2] 2979595

$ Gramine is starting. Parsing TOML manifest file, this may take some time...
[2023-11-08 11:05:50 +0000] [1] [INFO] Starting gunicorn 20.1.0
[2023-11-08 11:05:50 +0000] [1] [INFO] Listening at: http://127.0.0.1:8000 (1)
[2023-11-08 11:05:50 +0000] [1] [INFO] Using worker: sync
[2023-11-08 11:05:59 +0000] [2] [INFO] Booting worker with pid: 2

Get the process details and kill it
In below steps, we can see how the process is not getting killed, even after trying it multiple times using kill command. Trying to kill both parent process and child process together also does not help. If we just kill parent process, the child process remains active, it does not get killed

$ ps aux | grep gramine
intel    2979595 81.6  0.5 1074314344 44704 pts/1 Sl  16:35   0:10 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so init gunicorn
intel    2979605  100  0.3  26752 24664 pts/1    R    16:35   0:02 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so child 29
intel    2979607  0.0  0.0   9076  2492 pts/2    S+   16:35   0:00 grep --color=auto gramine

$ cat server.PID
2979595

$ kill $(cat server.PID)

$ ps aux | grep gramine
intel    2979595 25.2  0.5 1074314344 44700 pts/1 Sl  16:35   0:10 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so init gunicorn
intel    2979605 30.2  0.4 1074301800 32164 pts/1 Sl  16:35   0:09 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so child 29
intel    2979620  0.0  0.0   9076  2460 pts/2    S+   16:36   0:00 grep --color=auto gramine

$ kill $(cat server.PID)

$ ps aux | grep gramine
intel    2979595 19.0  0.5 1074314344 44700 pts/1 Sl  16:35   0:10 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so init gunicorn
intel    2979605 21.0  0.4 1074301800 32164 pts/1 Sl  16:35   0:09 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so child 29
intel    2979623  0.0  0.0   9076  2480 pts/2    S+   16:36   0:00 grep --color=auto gramine


$ kill 2979595 2979605
$ ps aux | grep gramine
intel    2979595  9.7  0.5 1074314344 44700 pts/1 Sl  16:35   0:10 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so init gunicorn
intel    2979605  9.5  0.4 1074301800 32164 pts/1 Sl  16:35   0:09 /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/loader /home/intel/anjali/gramine/gramine_install/usr/lib/x86_64-linux-gnu/gramine/sgx/libpal.so child 29
intel    2979625  0.0  0.0   9076  2492 pts/2    S+   16:37   0:00 grep --color=auto gramine
$

Signed-off-by: Sonali Saha <[email protected]>
Copy link
Contributor Author

@sahason sahason left a comment

Choose a reason for hiding this comment

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

Please check now. Added "sys.enable_sigterm_injection = true" to the manifest file.

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

@anjalirai-intel
Copy link
Contributor

@sahason It works now. I will continue to further test on other distros

@anjalirai-intel
Copy link
Contributor

We tested across distros, and it is working fine, no further issues to report

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @sahason)

a discussion (no related file):
Add the explicit BSD-3 license, see #90


Signed-off-by: Sonali Saha <[email protected]>
Copy link
Contributor Author

@sahason sahason left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Add the explicit BSD-3 license, see #90

Done.


Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r2, 1 of 2 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @sahason)


gunicorn/gunicorn.manifest.template line 9 at r5 (raw file):

loader.log_level = "{{ log_level }}"

loader.argv = ["gunicorn", "--timeout", "600", "main:app"]

Please add a quick comment on why we need --timeout 600. Like this:

# we need `--timeout 600` to work around the problem of a constantly restarting worker process:
# the worker process is supposed to update the ctime of the shared file, and the parent process
# verifies that the worker process is alive by checking the ctime of this file every second.
# This ctime file metadata sharing between processes is currently not supported by Gramine;
# see also https://github.com/gramineproject/gramine/issues/1134.

gunicorn/gunicorn.manifest.template line 13 at r5 (raw file):

sys.enable_sigterm_injection = true

loader.env.LD_LIBRARY_PATH = "/gramine_lib:{{ arch_libdir }}:/usr/{{ arch_libdir }}"

Any reason you're using /gramine_lib? We typically use /lib:


gunicorn/gunicorn.manifest.template line 16 at r5 (raw file):


fs.mounts = [
  { path = "{{ arch_libdir }}", uri = "file:{{ arch_libdir }}" },

Please move this entry right-before /usr/{{ arch_libdir }}, for readability


gunicorn/gunicorn.manifest.template line 18 at r5 (raw file):

  { path = "{{ arch_libdir }}", uri = "file:{{ arch_libdir }}" },
  { path = "{{ entrypoint }}", uri = "file:{{ entrypoint }}" },
  { path = "/gramine_lib", uri = "file:{{ gramine.runtimedir() }}" },

Any reason you're using /gramine_lib? We typically use /lib:


gunicorn/gunicorn.manifest.template line 22 at r5 (raw file):

  { path = "{{ path }}", uri = "file:{{ path }}" },
{% endfor %}
  { path = "/usr/bin", uri = "file:/usr/bin" },

Is this really needed? What happens if you remove it?


gunicorn/gunicorn.manifest.template line 31 at r5 (raw file):

sgx.max_threads = 64

sgx.use_exinfo = true

Is this really needed? What happens if you remove it?


gunicorn/gunicorn.manifest.template line 34 at r5 (raw file):


sgx.trusted_files = [
  "file:{{ arch_libdir }}/",

ditto (move to another place)


gunicorn/gunicorn.manifest.template line 38 at r5 (raw file):

  "file:{{ gramine.libos }}",
  "file:{{ gramine.runtimedir() }}/",
  "file:main.py",

Can you move it as the very-last item?


gunicorn/gunicorn.manifest.template line 42 at r5 (raw file):

  "file:{{ path }}{{ '/' if path.is_dir() else '' }}",
{% endfor %}
  "file:/usr/bin/",

ditto (do you really need it)


gunicorn/main.py line 1 at r5 (raw file):

from flask import Flask, jsonify, request

@sahason Can you please add the license lines also here? If you wrote this yourself, then just add the exact same as for manifest.template and Makefile (BSD license). If you took it from somewhere, please check what license is used for this file in the source website, and add the SPDX with this license here.


gunicorn/README.md line 29 at r1 (raw file):

Previously, sahason wrote…

Modified the manifest file.

To workaround the timeout issue which happens due to gramine not supporting sharing file metadata (st_ctime) information between child and parent processes, the timeout is set to a higher value.

More details are here. I found a gap between Gunicorn execution in native vs gramine (Both gramine-direct and gramine-sgx) irrespective of the value of timeout. In native the flow works fine but in gramine the worker is up for some time ~(timeout-20) seconds then exit happens, and the worker is restarted again after timeout.
gunicorn-garmine.png
In Gunicorn timeout happens when time.time() - worker.tmp.last_update() <= self.timeout
https://github.com/benoitc/gunicorn/blob/430dcdd997b955fa1a8569a07d9cdcbac686fb35/gunicorn/arbiter.py#L498 & https://github.com/benoitc/gunicorn/blob/430dcdd997b955fa1a8569a07d9cdcbac686fb35/gunicorn/workers/workertmp.py#L49C24-L49C53. After every timeout/2 seconds the st_ctime of a tmpfs file is updated by fchmod https://github.com/benoitc/gunicorn/blob/430dcdd997b955fa1a8569a07d9cdcbac686fb35/gunicorn/workers/workertmp.py#L46. So, last updated time will get updated after each timeout/2 seconds.  The fchmod happens in a forked process and Gunicorn checks updated time using fstat in parent process in each one second interval. In gramine the file metadata information (st_ctime in this case) is not propagated from child to parent if there is any change in child process due to fchmod. I have found similar issues already reported in gramine Forked processes do not get updated file sizes when modified in a multi-process scenario · Issue #1134 · gramineproject/gramine (github.com)gramineproject/gramine#254. There is a work item to handle the shared tmpfs files gramineproject/gramine#584. Also note that we have to handle st_ctime modification for tmpfs files when fchmod is called when this fix is in.

Thanks for a thorough explanation!


gunicorn/README.md line 4 at r5 (raw file):

This directory contains an example for running Gunicorn in Gramine, including the Makefile and a
template for generating the manifest. Gunicorn is a webserver to deploy Flask application in

application -> applications


gunicorn/README.md line 5 at r5 (raw file):

This directory contains an example for running Gunicorn in Gramine, including the Makefile and a
template for generating the manifest. Gunicorn is a webserver to deploy Flask application in
Python. Although, Flask comes with an internal webserver, this is widely considered to be not

Please remove , after Although:

Although Flask comes ...

gunicorn/README.md line 9 at r5 (raw file):

communicates via the WSGI protocol. A common choice for that webserver is Gunicorn. Users can
protect their confidentiality and integrity of the Python based ML APIs and models using Gramine
for a more secure production deployment using Gunicorn. For more documentation, refer to

I'm not sure why you added this sentence about Python based ML APIs. I assume that was the final goal of Gunicorn, to use it as a web interface to some AI/ML runtime.

Anyway, since this PR is very generic and only has the HelloWorld example, please remove this whole sentence (Users can protect ...).


gunicorn/README.md line 41 at r5 (raw file):

gramine-sgx gunicorn

But aren't you supposed to also show some client connecting to Gunicorn and receiving the Hello World reply?

Please add a way to quickly test Gunicorn, similar to this: https://github.com/gramineproject/gramine/tree/master/CI-Examples/lighttpd#running-lighttpd

(Just one of wget or curl commands will be enough)

Signed-off-by: Sonali Saha <[email protected]>
Copy link
Contributor Author

@sahason sahason left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 14 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)


gunicorn/gunicorn.manifest.template line 9 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please add a quick comment on why we need --timeout 600. Like this:

# we need `--timeout 600` to work around the problem of a constantly restarting worker process:
# the worker process is supposed to update the ctime of the shared file, and the parent process
# verifies that the worker process is alive by checking the ctime of this file every second.
# This ctime file metadata sharing between processes is currently not supported by Gramine;
# see also https://github.com/gramineproject/gramine/issues/1134.

Done.


gunicorn/gunicorn.manifest.template line 13 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Any reason you're using /gramine_lib? We typically use /lib:

We are enabling this example for all supported distros. In Alpine Distro, ARCH_LIBDIR is /lib and by default gramine.runtimedir() was also mounted to /lib which caused conflicts and failed for Alpine Distro. The solution is inspired from https://github.com/gramineproject/gramine/blob/36676a75e9897c9b7b183157a9d9fcdf2f668a13/CI-Examples/blender/blender.manifest.template#L11


gunicorn/gunicorn.manifest.template line 16 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please move this entry right-before /usr/{{ arch_libdir }}, for readability

Done.


gunicorn/gunicorn.manifest.template line 18 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Any reason you're using /gramine_lib? We typically use /lib:

We are enabling this example for all supported distros. In Alpine Distro, ARCH_LIBDIR is /lib and by default gramine.runtimedir() was also mounted to /lib which caused conflicts and failed for Alpine Distro. The solution is inspired from (https://github.com/gramineproject/gramine/blob/36676a75e9897c9b7b183157a9d9fcdf2f668a13/CI-Examples/blender/blender.manifest.template#L18)


gunicorn/gunicorn.manifest.template line 22 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Is this really needed? What happens if you remove it?

If I remove it I get below error while running make and the build fails in Ubuntu 20.04.

(libos_rtld.c:710:load_and_check_shebang) [P1:T1:] debug: Interpreter to be used for execve: /usr/bin/python3
(libos_init.c:436:libos_init) [P1:T1:] error: libos_init() failed in init_exec_handle: No such file or directory (ENOENT)

We could have added the path /usr/bin/python3 if the example was not generic to suppoted all distros. As in RHEL8 and CentOS distros the path is /usr/bin/python3.6 based on the gunicorn installation had to add this way.


gunicorn/gunicorn.manifest.template line 31 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Is this really needed? What happens if you remove it?

Gunicorn failed with Gramine-SGX with below error for CentOS Stream 8 and RHEL 8

"error: Tried to handle a memory fault with no faulting address reported by SGX. Please consider enabling 'sgx.use_exinfo' in the manifest."

To fix this the line is added.


gunicorn/gunicorn.manifest.template line 34 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

ditto (move to another place)

Done.


gunicorn/gunicorn.manifest.template line 38 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Can you move it as the very-last item?

Done.


gunicorn/gunicorn.manifest.template line 42 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

ditto (do you really need it)

If I remove it I get below error while running make and the build fails in Ubuntu 20.04.

(libos_rtld.c:710:load_and_check_shebang) [P1:T1:] debug: Interpreter to be used for execve: /usr/bin/python3
(libos_init.c:436:libos_init) [P1:T1:] error: libos_init() failed in init_exec_handle: No such file or directory (ENOENT)

We could have added the path /usr/bin/python3 if the example was not generic to suppoted all distros. As in RHEL8 and CentOS distros the path is /usr/bin/python3.6 based on the gunicorn installation had to add the path this way.


gunicorn/main.py line 1 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

@sahason Can you please add the license lines also here? If you wrote this yourself, then just add the exact same as for manifest.template and Makefile (BSD license). If you took it from somewhere, please check what license is used for this file in the source website, and add the SPDX with this license here.

Done.


gunicorn/README.md line 4 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

application -> applications

Done.


gunicorn/README.md line 5 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove , after Although:

Although Flask comes ...

Done.


gunicorn/README.md line 9 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I'm not sure why you added this sentence about Python based ML APIs. I assume that was the final goal of Gunicorn, to use it as a web interface to some AI/ML runtime.

Anyway, since this PR is very generic and only has the HelloWorld example, please remove this whole sentence (Users can protect ...).

Done.


gunicorn/README.md line 41 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

But aren't you supposed to also show some client connecting to Gunicorn and receiving the Hello World reply?

Please add a way to quickly test Gunicorn, similar to this: https://github.com/gramineproject/gramine/tree/master/CI-Examples/lighttpd#running-lighttpd

(Just one of wget or curl commands will be enough)

Done.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @sahason)


gunicorn/gunicorn.manifest.template line 13 at r5 (raw file):

Previously, sahason wrote…

We are enabling this example for all supported distros. In Alpine Distro, ARCH_LIBDIR is /lib and by default gramine.runtimedir() was also mounted to /lib which caused conflicts and failed for Alpine Distro. The solution is inspired from https://github.com/gramineproject/gramine/blob/36676a75e9897c9b7b183157a9d9fcdf2f668a13/CI-Examples/blender/blender.manifest.template#L11

Thanks for the explanation! Yes, I forgot about Alpine's conflicting /lib dir. We'll probably need to fix it everywhere in our examples at some point?


gunicorn/gunicorn.manifest.template line 22 at r5 (raw file):

Previously, sahason wrote…

If I remove it I get below error while running make and the build fails in Ubuntu 20.04.

(libos_rtld.c:710:load_and_check_shebang) [P1:T1:] debug: Interpreter to be used for execve: /usr/bin/python3
(libos_init.c:436:libos_init) [P1:T1:] error: libos_init() failed in init_exec_handle: No such file or directory (ENOENT)

We could have added the path /usr/bin/python3 if the example was not generic to suppoted all distros. As in RHEL8 and CentOS distros the path is /usr/bin/python3.6 based on the gunicorn installation had to add this way.

Still I don't like that we allow all the installed executables on a system.

Can't we use {{ python_exe_path }}? We already have this variable (to get the Python system paths) here, so maybe can use this?


gunicorn/gunicorn.manifest.template line 42 at r5 (raw file):

Previously, sahason wrote…

If I remove it I get below error while running make and the build fails in Ubuntu 20.04.

(libos_rtld.c:710:load_and_check_shebang) [P1:T1:] debug: Interpreter to be used for execve: /usr/bin/python3
(libos_init.c:436:libos_init) [P1:T1:] error: libos_init() failed in init_exec_handle: No such file or directory (ENOENT)

We could have added the path /usr/bin/python3 if the example was not generic to suppoted all distros. As in RHEL8 and CentOS distros the path is /usr/bin/python3.6 based on the gunicorn installation had to add the path this way.

Still I don't like that we allow all the installed executables on a system.

Can't we use {{ python_exe_path }}? We already have this variable (to get the Python system paths) here, so maybe can use this?

Copy link
Contributor Author

@sahason sahason left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)


gunicorn/gunicorn.manifest.template line 22 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Still I don't like that we allow all the installed executables on a system.

Can't we use {{ python_exe_path }}? We already have this variable (to get the Python system paths) here, so maybe can use this?

In ubuntu20.04 using python_exe_path fails as this path translates to /usr/bn/python3.8 and the interpreter version in /usr/bin/gunicorn i.e. /usr/bin/python3 does not match. So there will be ENOENT failure.
ubuntu_20_04_gunicorn_binary.png
Now tried to set python_exe_path to $(shell sh -c "command -v python3") which translates to /usr/bin/python3 which works fine in ubuntu 20.04. But this will not work in RHEL or CentOS. The interpreter version in /usr/bin/gunicorn in RHEL is/usr/bin/python3.6.
rhel_gunicorn_binary.png

Also tried to set default version of python3 to python3.6 in ubuntu 20.04 and install gunicorn but the interpreter version remains same as /usr/bin/python3 irrespective of default python version.


gunicorn/gunicorn.manifest.template line 42 at r5 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Still I don't like that we allow all the installed executables on a system.

Can't we use {{ python_exe_path }}? We already have this variable (to get the Python system paths) here, so maybe can use this?

In ubuntu20.04 using python_exe_path fails as this path translates to /usr/bn/python3.8 and the interpreter version in /usr/bin/gunicorn i.e. /usr/bin/python3 does not match. So there will be ENOENT failure.
ubuntu_20_04_gunicorn_binary.png
Now tried to set python_exe_path to $(shell sh -c "command -v python3") which translates to /usr/bin/python3 which works fine in ubuntu 20.04. But this will not work in RHEL or CentOS. The interpreter version in /usr/bin/gunicorn in RHEL is/usr/bin/python3.6.
rhel_gunicorn_binary.png

Also tried to set default version of python3 to python3.6 in ubuntu 20.04 and install gunicorn but the interpreter version remains same as /usr/bin/python3 irrespective of default python version.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners


gunicorn/gunicorn.manifest.template line 22 at r5 (raw file):

Previously, sahason wrote…

In ubuntu20.04 using python_exe_path fails as this path translates to /usr/bn/python3.8 and the interpreter version in /usr/bin/gunicorn i.e. /usr/bin/python3 does not match. So there will be ENOENT failure.
ubuntu_20_04_gunicorn_binary.png
Now tried to set python_exe_path to $(shell sh -c "command -v python3") which translates to /usr/bin/python3 which works fine in ubuntu 20.04. But this will not work in RHEL or CentOS. The interpreter version in /usr/bin/gunicorn in RHEL is/usr/bin/python3.6.
rhel_gunicorn_binary.png

Also tried to set default version of python3 to python3.6 in ubuntu 20.04 and install gunicorn but the interpreter version remains same as /usr/bin/python3 irrespective of default python version.

Ok, I'm unblocking as indeed this will require some complications.

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