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 entries to passwd/group if running as user not in passwd file. #559

Merged
merged 1 commit into from
Feb 20, 2018

Conversation

GrahamDumpleton
Copy link
Contributor

Alternate PR to address #552.

See also discussion against prior PR at #553.

This PR does everything in start.sh with only other change being in Dockerfile to make passwd and group files writable by root group.

@minrk
Copy link
Member

minrk commented Feb 20, 2018

Excellent, thank you! I think we can still look at whether it makes sense to consolidate much of this into an entrypoint as you had with the new logic in #553, but I think it's good to have it all together either way.

@parente
Copy link
Member

parente commented Feb 20, 2018

Thanks @minrk for jumping on these reviews in my absence, and for opening #560!

@charliesolomon
Copy link

charliesolomon commented Mar 1, 2018

@GrahamDumpleton I'm testing with OpenShift 3.7, and getting permission errors:

Adding passwd file entry for 1000800000
--
  | Adding group file entry for 1000800000
  | Container must be run with group users to update files
  | Executing the command: jupyter lab
  | Traceback (most recent call last):
  | File "/opt/conda/lib/python3.6/site-packages/traitlets/traitlets.py", line 528, in get
  | value = obj._trait_values[self.name]
  | KeyError: 'runtime_dir'
  | During handling of the above exception, another exception occurred:
  | Traceback (most recent call last):
  | File "/opt/conda/bin/jupyter-lab", line 6, in <module>
  | sys.exit(jupyterlab.labapp.main())
  | File "/opt/conda/lib/python3.6/site-packages/jupyter_core/application.py", line 266, in launch_instance
  | return super(JupyterApp, cls).launch_instance(argv=argv, **kwargs)
  | File "/opt/conda/lib/python3.6/site-packages/traitlets/config/application.py", line 657, in launch_instance
  | app.initialize(argv)
  | File "<decorator-gen-7>", line 2, in initialize
  | File "/opt/conda/lib/python3.6/site-packages/traitlets/config/application.py", line 87, in catch_config_error
  | return method(app, *args, **kwargs)
  | File "/opt/conda/lib/python3.6/site-packages/notebook/notebookapp.py", line 1366, in initialize
  | self.init_configurables()
  | File "/opt/conda/lib/python3.6/site-packages/notebook/notebookapp.py", line 1100, in init_configurables
  | connection_dir=self.runtime_dir,
  | File "/opt/conda/lib/python3.6/site-packages/traitlets/traitlets.py", line 556, in __get__
  | return self.get(obj, cls)
  | File "/opt/conda/lib/python3.6/site-packages/traitlets/traitlets.py", line 535, in get
  | value = self._validate(obj, dynamic_default())
  | File "/opt/conda/lib/python3.6/site-packages/jupyter_core/application.py", line 99, in _runtime_dir_default
  | ensure_dir_exists(rd, mode=0o700)
  | File "/opt/conda/lib/python3.6/site-packages/jupyter_core/utils/__init__.py", line 13, in ensure_dir_exists
  | os.makedirs(path, mode=mode)
  | File "/opt/conda/lib/python3.6/os.py", line 210, in makedirs
  | makedirs(head, mode, exist_ok)
  | File "/opt/conda/lib/python3.6/os.py", line 210, in makedirs
  | makedirs(head, mode, exist_ok)
  | File "/opt/conda/lib/python3.6/os.py", line 210, in makedirs
  | makedirs(head, mode, exist_ok)
  | File "/opt/conda/lib/python3.6/os.py", line 220, in makedirs
  | mkdir(name, mode)
  | PermissionError: [Errno 13] Permission denied: '/home/jovyan/.local'

In my local testing, I create a new image by setting the GID environment var to 0 and running the "fix-permissions" script on the folders that are failing... that image works in OpenShift.

@GrahamDumpleton
Copy link
Contributor Author

When deploying to OpenShift, you must set the supplemental groups in the security context of the deployment config to include UID 100 in the list of groups. For example templates which do the required setup for deployment see:

@GrahamDumpleton
Copy link
Contributor Author

Also look through:

That has examples/templates for deploying JupyterHub, the notebook viewer, as well as S2I based build images for building notebook images from scratch in OpenShift that do not require the supplemental groups to be set.

@GrahamDumpleton
Copy link
Contributor Author

GrahamDumpleton commented Mar 2, 2018

There are various warnings output from start.sh when the configuration isn't right, including the one above of:

Container must be run with group users to update files

which indicates what the issue is in this case. Maybe that and all the other messages should be output with a 'WARNING' prefix to make them more obvious that a configuration change is likely required in way image is being run.

@parente
Copy link
Member

parente commented Mar 2, 2018

Maybe that and all the other messages should be output with a 'WARNING' prefix

A warning prefix sounds reasonable. Or would it be better to have the image startup fail outright?

@GrahamDumpleton
Copy link
Contributor Author

For some of them, it doesn't necessarily mean you can't do anything, would just affect corner cases. Enabling Jupyter Lab interface is first case seen where the notebook wouldn't even start up.

@charliesolomon
Copy link

I wouldn't call enabling Jupyter Lab a corner case... I failed to mention in my post above that I changed the startup CMD to ["start.sh", "jupyter", "lab"] for that OpenShift deployment.

@GrahamDumpleton
Copy link
Contributor Author

I am not saying Jupyter Lab is the corner case, I have other things in mind, like a small number of Python packages that don't handle missing entries in passwd/group file properly. Different situation, although with changes have made to start.sh already, that should be rarer now.

You didn't need to override start command. See:

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.

4 participants