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

Copy remote branch entrypoint to compile and production image stages #3213

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

lanxih
Copy link
Contributor

@lanxih lanxih commented Jun 27, 2024

Description

Torchserve docker build with remote branch needs to copy the remote version of entrypoint (dockerd-entrypoint.sh) to the image to keep it consistent with the --branch_name specified for the build.

Fixes #(issue)
We encountered this issue where the torchserve image is expected from --branch_name v0.11.0 in order to work with pinned torchserve pypi package version, but the current entrypoint is out-of-sync and uses additional flag --disable-token from master.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Feature/Issue validation/testing

Re-run bash ./build_image.sh --branch_name v0.11.0 --pythonversion 3.10 --tag test-image to verify that the newly built image now has /usr/local/bin/dockerd-entrypoint.sh from branch v0.11.0 without --disable-token set.

% docker run --rm -it --entrypoint bash test-image
model-server@465e40c1ead4:~$ cat /usr/local/bin/dockerd-entrypoint.sh
#!/bin/bash
set -e

if [[ "$1" = "serve" ]]; then
    shift 1
    torchserve --start --ts-config /home/model-server/config.properties
else
    eval "$@"
fi

# prevent docker exit
tail -f /dev/null

Also ran without branch name to verify the entrypoint has the new flag.

@@ -78,6 +78,8 @@ RUN \

WORKDIR "serve"

RUN cp docker/dockerd-entrypoint.sh /usr/local/bin/dockerd-entrypoint.sh
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please change this to COPY instead of RUN cp

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great question, I also tried COPY but seems that would copy dockerd-entrypoint.sh from the volume (which is the older version prior to cloning from specific branch) to the image, so I suppose that does not work...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh..Interesting

@agunapal agunapal added this to the v0.12.0 milestone Jun 27, 2024
@lanxih lanxih requested a review from agunapal June 28, 2024 00:03
Copy link
Collaborator

@agunapal agunapal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@agunapal agunapal enabled auto-merge June 28, 2024 00:17
@agunapal agunapal added this pull request to the merge queue Jun 28, 2024
Merged via the queue into pytorch:master with commit 040c0ea Jun 28, 2024
12 checks passed
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.

2 participants