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 "latest" tag to BentoML user-facing docker image #1046

Merged
merged 2 commits into from Sep 2, 2020

Conversation

parano
Copy link
Member

@parano parano commented Aug 30, 2020

Description

  • Add the latest tag when releasing a new docker image that is user-facing.
  • We will not do this for BentoML internal docker images, e.g. the azure-function image will not have latest tag
  • latest tag can cause issues when used in a production environment, although it is very convenient when a user is just starting out. We should add a warning about it in our YataiService deployment guide.

Motivation and Context

For context, see PR #1045 from @danield137 and slack discussions with @jackyzha0 here: https://bentoml.slack.com/archives/CK8PQU2JY/p1597872697082800

How Has This Been Tested?

Tested locally, published latest tag for release 0.8.6

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature and improvements (non-breaking change which adds/improves functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Code Refactoring (internal change which is not user facing)
  • Documentation
  • Test, CI, or build

Component(s) if applicable

  • BentoService (service definition, dependency management, API input/output adapters)
  • Model Artifact (model serialization, multi-framework support)
  • Model Server (mico-batching, dockerisation, logging, OpenAPI, instruments)
  • YataiService gRPC server (model registry, cloud deployment automation)
  • YataiService web server (nodejs HTTP server and web UI)
  • Internal (BentoML's own configuration, logging, utility, exception handling)
  • BentoML CLI

Checklist:

  • My code follows the bentoml code style, both ./dev/format.sh and
    ./dev/lint.sh script have passed
    (instructions).
  • My change reduces project test coverage and requires unit tests to be added
  • I have added unit tests covering my code change
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@codecov
Copy link

codecov bot commented Aug 30, 2020

Codecov Report

Merging #1046 into master will decrease coverage by 0.74%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1046      +/-   ##
==========================================
- Coverage   63.54%   62.80%   -0.75%     
==========================================
  Files         125      125              
  Lines        8274     8132     -142     
==========================================
- Hits         5258     5107     -151     
- Misses       3016     3025       +9     
Impacted Files Coverage Δ
bentoml/saved_bundle/local_py_modules.py 74.11% <0.00%> (-10.59%) ⬇️
bentoml/yatai/deployment/operator.py 48.00% <0.00%> (-7.18%) ⬇️
bentoml/cli/config.py 31.50% <0.00%> (-5.21%) ⬇️
bentoml/exceptions.py 75.60% <0.00%> (-3.99%) ⬇️
bentoml/cli/deployment.py 45.88% <0.00%> (-3.57%) ⬇️
bentoml/cli/aws_lambda.py 48.93% <0.00%> (-3.07%) ⬇️
bentoml/cli/aws_sagemaker.py 51.96% <0.00%> (-2.67%) ⬇️
bentoml/adapters/dataframe_output.py 24.56% <0.00%> (-2.56%) ⬇️
bentoml/cli/azure_functions.py 57.14% <0.00%> (-2.48%) ⬇️
bentoml/cli/bento_management.py 28.57% <0.00%> (-2.47%) ⬇️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edbfa22...9959a44. Read the comment docs.

Comment on lines 20 to 22
echo "Tag and push 'latest'.."
docker tag bentoml/model-server:"$BENTOML_VERSION" bentoml/model-server:latest
docker push bentoml/model-server:latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thoughts on tagging with latest in the build step? moby/moby#15780

Copy link
Collaborator

Choose a reason for hiding this comment

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

That way push step can just be docker push bentoml/model-server and it will figure out which tags need to be pushed on its own

Copy link
Member Author

Choose a reason for hiding this comment

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

Great idea!

Do you know how does docker push bentoml/model-server figure out which tags needs to be pushed? I may have other tags that are used only for local development and want to avoid pushing those to docker hub.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok turns out docker push without a specified tag will push all local dev images too so that may not be ideal. I guess we can just leave it at docker push bentoml/model-server:latest && docker push bentoml/model-server:"$BENTOML_VERSION"
image

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes sense, thanks for confirming!

Copy link
Collaborator

@jackyzha0 jackyzha0 left a comment

Choose a reason for hiding this comment

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

LGTM!! clean 🧼

@parano
Copy link
Member Author

parano commented Sep 2, 2020

LGTM!! clean 🧼

Thanks for the suggestion @jackyzha0, it is so much cleaner this way!

@parano
Copy link
Member Author

parano commented Sep 2, 2020

@yubozhao looks like those two tests are flaky on CI, could you help look into it?

@parano parano merged commit d2414b1 into bentoml:master Sep 2, 2020
@yubozhao
Copy link
Contributor

yubozhao commented Sep 2, 2020

@parano yeah. I will try figure out why. From the Github actions, it seems like the unit tests and the API server tests are flaky right?

aarnphm pushed a commit to aarnphm/BentoML that referenced this pull request Jul 29, 2022
* Add latest tag to BentoML user-facing docker image

* updates
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.

None yet

3 participants