-
Notifications
You must be signed in to change notification settings - Fork 613
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
Utilize SIG Build Docker Images #2515
Conversation
@angerson A TF 2.6 has still a python 3.6 wheel do you plan to push also a 3.6 image? |
OK.. green lights here.. |
We are having some issues with the
|
Was solved in #2623 |
Just for visibility the reason for this auditwheel patch is to specify that tensorflow shared library should be added to the approved manylinux2010 libraries. If we remove this it will fail to export a manylinux2010 package. SIG IO does a similar thing: https://github.com/tensorflow/io/blob/master/tools/build/auditwheel |
Nice to be able to cut a lot of this tech debt from the build file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1,11 +1,9 @@ | |||
#syntax=docker/dockerfile:1.1.5-experimental | |||
FROM gcr.io/tensorflow-testing/nosla-cuda11.2-cudnn8.1-ubuntu18.04-manylinux2010-multipython as dev_container_cpu | |||
ARG PY_VERSION | |||
FROM tensorflow/build:latest-python$PY_VERSION as dev_container_cpu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you can add here a fake but future-proof IMAGE_TYPE
arg I think that we could remove the overhead of maintaining another separated Dockerfile (if you suppose that the our devel and our Vscode/Codespaces images could be almost the same one):
addons/.devcontainer/Dockerfile
Lines 1 to 2 in 4617bc2
ARG IMAGE_TYPE=latest-cpu | |
FROM tfaddons/dev_container:$IMAGE_TYPE |
The main thing we are doing on that additional Dockerfile, other then let the user controlling the IMAGE_TYPE
when they need the GPU image (currently not available) is that we are adding a default
(guid/uid) non root user in the image build.
If we exclude that, with the unification we need to evaluate if without a default non root user, we can still official suggest that we prefer/support only Docker in rootless mode. This is currently the SIG Build policy that doesn't want to add a "default" user in the image.
It is hard to support both as rootless Docker vs root Docker are mutually exclusive for root and non root users permission on the host mounted volumes:
moby/moby#41497
https://discuss.tensorflow.org/t/adopting-open-source-dockerfiles-for-official-tf-nightly-ci/6050/9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the holder for IMAGE_TYPE
. And updated to push it correctly as gpu dev container. I think it should be good to merge and we can revisit once we have a means to get a cpu image.
Made a new PR for the release: |
Description
Move to managed SIG Build images.