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 optional --app argument to apptainer instance start #1118

Closed
struktured opened this issue Feb 16, 2023 · 8 comments · Fixed by #1125
Closed

Add optional --app argument to apptainer instance start #1118

struktured opened this issue Feb 16, 2023 · 8 comments · Fixed by #1125
Labels
enhancement New feature or request
Milestone

Comments

@struktured
Copy link
Contributor

struktured commented Feb 16, 2023

Version of Apptainer

What version of Apptainer (or Singularity) are you using? Run

apptainer --version (or singularity --version).

apptainer version 1.1.5

Expected behavior

apptainer instance start --app app1 foo.sif foo_app

What did you expect to see when you do...?

INFO: app1 instance started successfully

With the idea that there was an %appstartscript app1 section which gets invoked from the definition file.

Actual behavior

Error for command "start": unknown flag: --app

@struktured struktured changed the title Add optional --app argument to apptainer instance [start|stop] Add optional --app argument to apptainer instance start Feb 16, 2023
@GodloveD
Copy link
Contributor

Seems like a good idea to me. I believe that app support is added by playing fancy tricks with the environment at build time. Would you want to investigate further and perhaps submit a PR?

@GodloveD GodloveD added the enhancement New feature or request label Feb 16, 2023
@struktured
Copy link
Contributor Author

Seems like a good idea to me. I believe that app support is added by playing fancy tricks with the environment at build time. Would you want to investigate further and perhaps submit a PR?

I will investigate further and see if I can manage to implement it.

Thanks!

@struktured
Copy link
Contributor Author

struktured commented Feb 20, 2023

After adding some minor boilerplate in a few critical places I got it working!

However, I need to write a unit test and review the contribution guidelines before I open a pull request. The current diff is here.

@dtrudg
Copy link
Contributor

dtrudg commented Feb 21, 2023

Please could you loop in @vsoch on this proposal, or at least check if they wish to be involved?

The %app... directives are an implementation of a standard called SCIF (https://sci-f.github.io/)

The specification for SCIF defines the recipe / definition file sections, and there is currently no %appstart:

https://sci-f.github.io/spec-v1#sections

Elsewhere in the specification document there are defined apprun filenames and environment variable, that would also need appstart counterparts to be declared.

I believe that adding an %app... to the specification should be considered before it is implemented in apptainer.

I note that elsewhere, Apptainer is considering instance run or similar. It may be the case that an instance run --app with the existing SCIF app runscripts makes more sense then having separate run / start behavior per app. Likely some discussion needs to be had above the level of projects like Apptainer or SingularityCE that implement SCIF, to ensure that a consistent implementation of SCIF can be maintained w.r.t. any additional behaviour.

Thanks!

@vsoch
Copy link
Contributor

vsoch commented Feb 21, 2023

Thanks for pinging me! I agree 100% this should be added to the spec first, and I’d be happy to do that. My 0.02 is that apprun corresponds to the runscript and appstart to the start script.

I’ll need a few days - here is the original paper if anyone is interested! https://academic.oup.com/gigascience/article/7/5/giy023/4931737

It’s truly an honor that the rebranded Singularity name was derived from this spec that I wrote and originally implemented.

@dtrudg when I’m done adding appstart we can add to Singularity too.

it’s 2am here so goodnight!

@vsoch
Copy link
Contributor

vsoch commented Feb 21, 2023

I can get the spec PR in tonight after work - could someone from the Apptainer team please verify this plan is good?

@vsoch
Copy link
Contributor

vsoch commented Feb 22, 2023

Please see sci-f/sci-f.github.io#13

@struktured struktured mentioned this issue Feb 22, 2023
6 tasks
@DrDaveD
Copy link
Contributor

DrDaveD commented Feb 22, 2023

I can get the spec PR in tonight after work - could someone from the Apptainer team please verify this plan is good?

Sounds like a good plan to me.

struktured added a commit to struktured/apptainer that referenced this issue Feb 23, 2023
The apptainer instance start command now accepts an optional --app argument.

Fixes apptainer#1118.

Signed-off-by: Carmelo Piccione <carmelo.piccione@gmail.com>
struktured added a commit to struktured/apptainer that referenced this issue Mar 14, 2023
The apptainer instance start command now accepts an optional --app argument.

Fixes apptainer#1118.

Signed-off-by: Carmelo Piccione <carmelo.piccione@gmail.com>
struktured added a commit to struktured/apptainer that referenced this issue Mar 14, 2023
The apptainer instance start command now accepts an optional --app argument.

Fixes apptainer#1118.

Signed-off-by: Carmelo Piccione <carmelo.piccione@gmail.com>
struktured added a commit to struktured/apptainer that referenced this issue Apr 15, 2023
The apptainer instance start command now accepts an optional --app argument.

Fixes apptainer#1118.

Signed-off-by: Carmelo Piccione <carmelo.piccione@gmail.com>
struktured added a commit to struktured/apptainer that referenced this issue Apr 18, 2023
…gument.

Fixes apptainer#1118.

Signed-off-by: Carmelo Piccione <carmelo.piccione@gmail.com>
@DrDaveD DrDaveD added this to the 1.2.0 milestone Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants