[GSC] Always use absolute paths inside the Docker container #2151
Conversation
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)
a discussion (no related file):
FYI: This issue was detected by people in offline communication. They will verify that this fixes their use case.
a discussion (no related file):
@vahldiek Could you check that this PR makes sense to you?
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required) (waiting on @dimakuv)
a discussion (no related file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
@vahldiek Could you check that this PR makes sense to you?
Looks fine to me. A bit surprised, since workdir has worked before, but might be an artifact of the recent general rewrite. It might be worth adding a test for workdir to avoid seeing this issue again. The nodejs test case includes a workdir, but we're only testing test 1-3 which are the simpler python-based tests due to limited testing infrastructure. I'm wandering, if we should simply add another python image or change the trusted arguments image to include a working dir as well. What do you think?
|
|
Previously, some GSC templates and scripts that execute inside Docker containers contained relative file paths. This led to failures if a base Docker image contained WORKDIR different from root (`/`), since all GSC scripts put Graphene-related files under root dir. Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
dimakuv
force-pushed
the
dimakuv/gsc-use-absolute-paths
branch
from
72fc138 to
31f08ab
Compare
|
Jenkins, retest Jenkins-Debug please ( |
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved
There was a problem hiding this comment.
Reviewable status:
a discussion (no related file):
Previously, vahldiek (Anjo Vahldiek-Oberwagner) wrote…
Looks fine to me. A bit surprised, since workdir has worked before, but might be an artifact of the recent general rewrite. It might be worth adding a test for workdir to avoid seeing this issue again. The nodejs test case includes a workdir, but we're only testing test 1-3 which are the simpler python-based tests due to limited testing infrastructure. I'm wandering, if we should simply add another python image or change the trusted arguments image to include a working dir as well. What do you think?
FYI: This bug happened because we changed Graphene to use the libos.entrypoint which doesn't care about where the binary is located. So in the previous PR, I modified GSC such that it always puts the manifest + token + sig files in the root dir. But forgot to update all the paths to the absolute ones.
Description of the changes
Previously, some GSC templates and scripts that execute inside Docker containers contained relative file paths. This led to failures if a base Docker image contained WORKDIR different from root (
/), since all GSC scripts put Graphene-related files under root dir.How to test this PR?
GSC tests and tutorials (e.g. https://graphene.readthedocs.io/en/latest/manpages/gsc.html#example) must work.
This change is