Make start.sh the entrypoint
#2087
Conversation
manics
force-pushed
the
entrypoint-startsh
branch
from
f458cbe to
53aeaa8
Compare
manics
changed the title
start.sh the entrypoint
manics
force-pushed
the
entrypoint-startsh
branch
from
53aeaa8 to
ef98dd4
Compare
| ) | ||
| assert "Wrong python" not in logs | ||
| assert "Python" in logs | ||
|
|
||
|
|
||
| def test_startsh_multiple_exec(container: TrackedContainer) -> None: |
There was a problem hiding this comment.
Thank you for adding a test 👍
for more information, see https://pre-commit.ci
|
Just to be sure - you searched for all the occurrences of |
|
I missed one mention, fixed in the last commit. |
|
I've worked out what's causing the test to fail. Previously start-notebook.py ran start.sh ...., now it's the other way around, so now the output is and start-notebook.py runs |
| @@ -17,8 +17,8 @@ def test_python_version(container: TrackedContainer) -> None: | |||
| tty=True, | |||
| command=["python", "--version"], | |||
| ) | |||
| assert logs.startswith("Python ") | |||
| full_version = logs.split()[1] | |||
| python = next(line for line in logs.splitlines() if line.startswith("Python ")) | |||
There was a problem hiding this comment.
I think I simplified this line so it stays quite readable and is still robust to logging changes in start.sh
| @@ -31,4 +31,4 @@ def test_python_pinned_version(container: TrackedContainer) -> None: | |||
| tty=True, | |||
| command=["cat", "/opt/conda/conda-meta/pinned"], | |||
| ) | |||
| assert logs.startswith(f"python {EXPECTED_PYTHON_VERSION}.*") | |||
| assert f"python {EXPECTED_PYTHON_VERSION}.*" in logs | |||
There was a problem hiding this comment.
Let's have the simplest check possible here, should be just fine for our purposes.
There was a problem hiding this comment.
Overall, great job, @manics, thank you ❤️
I will merge this when tests pass.
|
NICE! |
|
@manics @minrk @yuvipanda I merged this 2 days ago, and it seems to work well 👍 @manics, thank you for this PR, it's great. |
|
@mathbunnyru awesome! I think we've mostly removed the workarounds in jupyterhub for more consistent behavior with images not from docker-stacks, so I think this will more directly affect users with a better experience and less configuration required. There are likely docs that should be updated. |
|
Wieee this is turning out very helpful right now, thanks to this we can still get the benefits from |
Move
start.shfrom CMD to ENTRYPOINTThis ensures the setup steps performed in start.sh will always be run unless the user overrides the entrypoint. For example, someone can pass
jupyterhub-singleuser(the default with KubeSpawner) and the image should "just work" as intuitively expected: jupyterhub/zero-to-jupyterhub-k8s#2138 (comment)There's a check to prevent re-execution of the script for anyone who continues to pass it as the first CMD.
Closes #1528