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

allow default image to be specified in server start request #304

Closed
rokroskar opened this issue Apr 29, 2020 · 9 comments · Fixed by #452
Closed

allow default image to be specified in server start request #304

rokroskar opened this issue Apr 29, 2020 · 9 comments · Fixed by #452
Assignees
Labels
enhancement New feature or request user

Comments

@rokroskar
Copy link
Member

rokroskar commented Apr 29, 2020

It should be possible to override the default image for a notebook launch in the case that an image built for the particular commit is not available. The use-case would be for situations where it isn't desirable to always build images but instead to provide a single stable image (e.g. courses, tutorials etc.)

This feature should be complemented by a change in the project configuration that allows users to specify a default image.

related to SwissDataScienceCenter/renku#646

@rokroskar rokroskar added the user label Apr 29, 2020
@emmjab
Copy link

emmjab commented Apr 30, 2020

Question: in this case, would it then be possible for projects to not include a Dockerfile?

@rokroskar
Copy link
Member Author

Yes, they wouldn't need the Dockerfile or the .gitlab-ci.yml file.

@emmjab
Copy link

emmjab commented Apr 30, 2020

We could also then create a template for this use case, (e.g. "Course Template") which excludes those files & explains how to use in the readme.

@olevski
Copy link
Member

olevski commented Oct 28, 2020

@ableuler my plan for this is as follows:

  1. get the image name by payload.get('image', None)
  2. if image is None then stick to the current flow
  3. if image is not None then:
    • check if the image that was provided exists and is accessible, if not error out
    • if the image is accessible and can be found then use that image to launch the server

Additional questions/considerations:

  • We should limit the image to being only on gitlab, or should I try to generalize to any image?
  • Should we error out or fall back to the old workflow if the image name passed in the request cannot be found?

@olevski
Copy link
Member

olevski commented Oct 28, 2020

An additional question is whether we should make it possible for a non-public image to be used in the API call. In which case we would have to also ask for the credentials to download/access the image. I think this is doable though.

@ableuler
Copy link
Contributor

  1. get the image name by payload.get('image', None)

  2. if image is None then stick to the current flow

  3. if image is not None then:

    • check if the image that was provided exists and is accessible, if not error out
    • if the image is accessible and can be found then use that image to launch the server

I like this plan. Although it's not exactly a default image that is provided that the notebook-service falls back to (as suggested by the issue title), but rather an image override. But I think that's actually more convenient to work with. What do you think @lorenzo-cavazzi, @rokroskar?

An additional question is whether we should make it possible for a non-public image to be used in the API call. In which case we would have to also ask for the credentials to download/access the image. I think this is doable though.

Yes, I think at least for images from the GitLab registry we should also be able to use non-public ones.

Should we error out or fall back to the old workflow if the image name passed in the request cannot be found?

If the user specifically selects an image which can not be found or accessed I think this should throw an error that the UI can handle adequately.

We should limit the image to being only on gitlab, or should I try to generalize to any image?

Being able to use publicly available images from any registry would be nice, yes.

@lorenzo-cavazzi
Copy link
Member

I like the plan!
Concerning the additional question, I agree with all the answers given by Andreas, but I would personally use a gradual approach by splitting this in 2 PRs:

  1. support only public images from the GitLab registry
  2. support private images from the GitLab registry

I don't know the extra effort required for supporting public images from other registries. If it's easy enough, I would try to implement it after 1. Otherwise, I would postpone it to a later point, unless you think it's a way more disirable feature than 2 for our users.

@rokroskar
Copy link
Member Author

I think private images from a gitlab registry are almost supported by default already. It's just that atm we don't add the image pull secret on projects that are public, so once the changes described here are implemented we would simply add it on every launch. It's less of an issue now since #435 has been merged.

For other registries we would need some other mechanism, e.g. storing the docker password in an environment variable in the project, for example.

@lorenzo-cavazzi
Copy link
Member

lorenzo-cavazzi commented Nov 2, 2020

After discussing with @olevski , we agreed on these steps to implement the feature.

  • accept an optional image parameter when spawning a new environment. This optional parameter contains a complete location for a target image (e.g. registry.dev.renku.ch/lorenzo.cavazzi.tech/test_project:0e8934a). The API invocation should fail early with a 404 response if the image is not accessible/available. The image should be added to the server annotations.
  • when no "image" is provided, the image complete location should still be added to the annotations and an additional boolean variable should be added to specify if the commit-image has been used or if it was necessary to fall back to the default image.
  • adapt the GET /servers api to return the 2 new annotations: [string] image and [bool] fallback_image.

lorenzo-cavazzi added a commit to SwissDataScienceCenter/renku-ui that referenced this issue Nov 4, 2020
lorenzo-cavazzi added a commit to SwissDataScienceCenter/renku-ui that referenced this issue Dec 4, 2020
* support custom registry images for new interactive environments
* show image source on status popover

BREAKING CHANGE: Requires SwissDataScienceCenter/renku-notebooks#304

fix #1105
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants