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

feat: Add --no-eval for OCI compatible argument and env var handling, from sylabs 704 #86

Closed
wants to merge 4 commits into from

Conversation

edytuk
Copy link
Collaborator

@edytuk edytuk commented May 5, 2022

This pulls in sylabs PR

The original PR description was:

Address sylabs/singularity#487 by adding a --no-eval actions flag, which does the following:

  1. Quotes environment variable values in the inject-singularity-env.sh generated script using single quotes to
    ensure the values are not evaluated when the injection script is sourced.
    This behavior applies to existing and newly created images.

  2. Uses an alternate path in the generated runscript for images built from OCI sources, that avoids evaluating CMD / ENTRYPOINT parts, and args in the shell.
    This behavior only applies to newly created images, that contain the new form of the generated OCI runscript.

The --no-eval flag is implictly added by the Docker/OCI --compat mode.

Also adds test cases that cover this new functionality, the historic behavior, and general handling of CMD / ENTRYPOINT/ CMD+ENTRYPOINT handling and overrides in Docker/OCI images. This should help us a lot in catching any changes to the 'historic' behaviour, that has been modified inadvertently in the past.

There is no change to the historic behavior of Singularity (run without --no-eval or --compat) at this time. OCI / Docker compatible handling by default may be considered for a future release.

@edytuk edytuk force-pushed the sylabs704 branch 4 times, most recently from 3d6bdac to 2bb35a5 Compare May 6, 2022 10:53
@edytuk edytuk requested a review from DrDaveD May 6, 2022 12:10
e2e/env/env.go Outdated Show resolved Hide resolved
internal/pkg/build/sources/conveyorPacker_oci.go Outdated Show resolved Hide resolved
@edytuk edytuk force-pushed the sylabs704 branch 2 times, most recently from 55987d7 to b094ffd Compare May 9, 2022 15:02
@edytuk
Copy link
Collaborator Author

edytuk commented May 9, 2022

Changes were made according to your comments

Copy link
Collaborator

@DrDaveD DrDaveD 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 sorry, I asked for the wrong things on my previous review. I didn't realize before that the script file was actually going into the container image. The container image has to stay completely backward compatible, meaning that we only refer to SINGULARITY there, not APPTAINER. So please go ahead and change back all the APPTAINER_OCI_RUN to SINGULARITY_OCI_RUN, and remove the extra case checking $APPTAINER_NO_EVAL. I checked and found out that apptainer is indeed setting SINGULARITY_NO_EVAL with the --no-eval option. It is also setting APPTAINER_NO_EVAL but we can't rely on that because we want containers created by apptainer to still run under singularity.

@edytuk
Copy link
Collaborator Author

edytuk commented May 10, 2022

I changed back all the APPTAINER_OCI_RUN to SINGULARITY_OCI_RUN, and removed the extra case checking $APPTAINER_NO_EVAL

Address sylabs/singularity#487 by adding a `--no-eval` actions flag, which does the
following:

1) Quotes environment variable value in the
`inject-singularity-env.sh` generated script using single quotes to
ensure the values are not evaluated when the injection script is
sourced.

This behavior applies to existing and newly created images.

2) Uses an alternate path in the generated runscript for images built
from OCI sources, that avoids evaluating CMD / ENTRYPOINT parts, and
args in the shell.

This behavior only applies to newly created images, that contain the
new form of the generated OCI runscript.

The `--no-eval` flag is implictly added by the Docker/OCI `--compat`
mode.

Signed-off-by: Edita Kizinevic <edita.kizinevic@cern.ch>
Signed-off-by: Edita Kizinevic <edita.kizinevic@cern.ch>
Signed-off-by: Edita Kizinevic <edita.kizinevic@cern.ch>
Signed-off-by: Edita Kizinevic <edita.kizinevic@cern.ch>
@edytuk edytuk closed this May 11, 2022
@edytuk edytuk deleted the sylabs704 branch May 20, 2022 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants