Update Dockerfiles for python artifacts to use latest git version #14588

ghostwriternr · 2018-03-04T02:23:43Z

Due to a recent change (Feb 2018) in Github's policy to discontinue support for weak cryptographic standards, older git versions stopped working with Github repositories. The Dockerfiles for building python artifacts currently uses git 2.0.5, that fails with an error tlsv1 alert protocol version.

The CentOS image by the latest source Dockerfile, manylinux comes with the latest version of git as well, removing the need to manually build git from source.

Due to a recent change (Feb 2018) in Github's policy to discontinue support for weak cryptographic standards, older git versions stopped working with Github repositories. The Dockerfiles for building python artifacts currently uses git 2.0.5, that fails with an error `tlsv1 alert protocol version`. The CentOS image by the latest source Dockerfile, `manylinux` comes with the latest version of git as well, removing the need to manually build git from source.

jtattermusch

LGTM (pending tests).

grpc-testing · 2018-03-05T11:22:39Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-03-05T11:32:07Z

[trickle] No significant performance differences

grpc-testing · 2018-03-05T12:14:07Z

[microbenchmarks] No significant performance differences

matt-kwong · 2018-03-05T19:13:50Z

I'm not sure if manylinux comes with the latest git version. When I run git --version when trying to build the Dockerfile, I see 1.8.2.3.

Step 1/13 : FROM quay.io/pypa/manylinux1_x86_64
 ---> 2478ba64a995
Step 2/13 : RUN yum update -y
 ---> Using cache
 ---> 4472acbf0aa9
Step 3/13 : RUN git --version
 ---> Running in 06861e91360c
git version 1.8.2.3
 ---> 92a151b1704f
Removing intermediate container 06861e91360c

manylinux is based off of CentOS 5, which is very old. I think if we want to update the version of git, we have to change the version downloaded. Also, my understanding is that the tlsv1 alert protocol version error is a result of an outdated OpenSSL version on CentOS 5, not an outdated git version, but I might be wrong on this.

Our manylinux Dockerfiles are broken and can't be built, but we have prebuilt images available to download and have been using those. It isn't a high priority to fix these, because we believe manylinux2 should fix these issues, although we'd be happy to accept fixes before then.

ghostwriternr · 2018-03-05T19:35:42Z

@jtattermusch @matt-kwong Thanks a lot for the review. But weirdly enough, when I tried running the manylinux1_x86_64 image, this is what I see:

(grpc) root@grpc:~/grpc# docker run -it quay.io/pypa/manylinux1_x86_64 bash
[root@29c6954d2a8b /]# ls
bin  curl_7.52.1.orig.tar.gz  dev  etc  home  lib  lib64  lost+found  media  mnt  opt  proc  root  sbin  selinux  srv  sys  tmp  usr  var
[root@29c6954d2a8b /]# yum update -y
base                                                                                                                                                                  | 1.1 kB     00:00     
base/primary                                                                                                                                                          | 1.3 MB     00:01     
base                                                                                                                                                                               3667/3667
epel                                                                                                                                                                  | 3.6 kB     00:00     
epel/primary_db                                                                                                                                                       | 2.8 MB     00:02     
extras                                                                                                                                                                | 2.1 kB     00:00     
extras/primary_db                                                                                                                                                     | 173 kB     00:00     
libselinux                                                                                                                                                            | 1.9 kB     00:00     
libselinux/primary_db                                                                                                                                                 | 128 kB     00:00     
testing-devtools-2-centos-5                                                                                                                                           |  951 B     00:00     
testing-devtools-2-centos-5/primary                                                                                                                                   |  26 kB     00:00     
testing-devtools-2-centos-5                                                                                                                                                            98/98
updates                                                                                                                                                               | 1.9 kB     00:00     
updates/primary_db                                                                                                                                                    | 1.0 MB     00:01     
Reducing CentOS-5 - libselinux to included packages only
Finished
Setting up Update Process
No Packages marked for Update
[root@29c6954d2a8b /]# git --version
git version 2.16.2

I've tried doing it after a Docker purge as well and seeing the same result.

(grpc) root@grpc:~/grpc# docker run -it quay.io/pypa/manylinux1_x86_64 bash
Unable to find image 'quay.io/pypa/manylinux1_x86_64:latest' locally
latest: Pulling from pypa/manylinux1_x86_64
2068b24f564b: Pull complete 
ccdf631d5f9e: Pull complete 
c370bd21ff6b: Pull complete 
Digest: sha256:54f069627bc23d2e7d7885d92d033b6293646cacb08395ad370758c2c0ccf2de
Status: Downloaded newer image for quay.io/pypa/manylinux1_x86_64:latest
[root@1dd0dc189228 /]# yum update -y
base                                                                                                                                                                  | 1.1 kB     00:00     
base/primary                                                                                                                                                          | 1.3 MB     00:01     
base                                                                                                                                                                               3667/3667
epel                                                                                                                                                                  | 3.6 kB     00:00     
epel/primary_db                                                                                                                                                       | 2.8 MB     00:02     
extras                                                                                                                                                                | 2.1 kB     00:00     
extras/primary_db                                                                                                                                                     | 173 kB     00:00     
libselinux                                                                                                                                                            | 1.9 kB     00:00     
libselinux/primary_db                                                                                                                                                 | 128 kB     00:00     
testing-devtools-2-centos-5                                                                                                                                           |  951 B     00:00     
testing-devtools-2-centos-5/primary                                                                                                                                   |  26 kB     00:00     
testing-devtools-2-centos-5                                                                                                                                                            98/98
updates                                                                                                                                                               | 1.9 kB     00:00     
updates/primary_db                                                                                                                                                    | 1.0 MB     00:01     
Reducing CentOS-5 - libselinux to included packages only
Finished
Setting up Update Process
No Packages marked for Update
[root@1dd0dc189228 /]# git --version
git version 2.16.2
[root@1dd0dc189228 /]#

Could it be because you're seeing a cached interim container on your side?

You're right about the tlsv1 alert protocol version being the result of an outdated OpenSSL. But the latest git version is built using newer OpenSSL versions too and thus works fine. I read about this in the deprecation notice on Github's blog.

If you insist, I can push a patch which builds the latest OpenSSL + curl first (this was actually what I tried first before noticing the latest git version on yum. So I still have the diff with me). But I don't think it's necessary.

I personally don't think I can access your pre-built images either (which I noticed in the CI failures. Correct me if I'm wrong). Is there something else I can try? This is a roadblock for me, so I could use your guidance.

ghostwriternr · 2018-03-05T19:43:41Z

Also, if you don't mind, may I ask what the current procedure is for building python artifacts, if not this? I see python packages updated on PyPI last week, so there should be another way (or I'm on a tangent and looking at the wrong place). I was attempting #14294 when I noticed this.

matt-kwong · 2018-03-05T20:40:48Z

That's strange. I just ran docker system purge and am still getting a different version even with a --no-cache flag

$ docker build --no-cache ./tools/dockerfile/grpc_artifact_python_manylinux_x64
Sending build context to Docker daemon  3.072kB
Step 1/13 : FROM quay.io/pypa/manylinux1_x86_64
 ---> 2478ba64a995
Step 2/13 : RUN yum update -y
 ---> Running in 3452a8513419
Reducing CentOS-5 - libselinux to included packages only
Finished
Setting up Update Process
No Packages marked for Update
 ---> d487d0782d92
Removing intermediate container 3452a8513419
Step 3/13 : RUN git --version
 ---> Running in 931192e3432e
git version 1.8.2.3
 ---> cd4b5bbef735
Removing intermediate container 931192e3432e

Maybe these steps don't stop docker from using a cached image of quay.io/pypa/manylinux1_x86_64.

I didn't realize Git shipped with OpenSSL; I thought it would rely on the system's provided OpenSSL. If this works, I'm glad to go with this and don't think downloading git is necessary. I'll fix the issues on my side to upload the image to our DockerHub repo, then we can merge this.

We do use this Dockerfile for building Python artifacts. We just happen to use prebuilt image from before it was broken by GitHub's changes.

matt-kwong · 2018-03-05T20:41:17Z

tools/dockerfile/grpc_artifact_python_manylinux_x64/Dockerfile

-#############################################################
-# Update Git to allow cloning submodules with --reference arg
-RUN yum remove -y git
-RUN yum install -y curl-devel expat-devel gettext-devel linux-headers openssl-devel zlib-devel gcc


This line shouldn't be deleted.

matt-kwong · 2018-03-05T20:41:30Z

tools/dockerfile/grpc_artifact_python_manylinux_x86/Dockerfile

-#############################################################
-# Update Git to allow cloning submodules with --reference arg
-RUN yum remove -y git
-RUN yum install -y curl-devel expat-devel gettext-devel linux-headers openssl-devel zlib-devel gcc


matt-kwong · 2018-03-05T20:47:47Z

I got it to work. I had to run docker image rm quay.io/pypa/manylinux1_* to get rid of the base image I had. The --no-cache flag doesn't affect using the base from cache =/.

It looks like it's building correctly, and I'm uploading the Docker images now. Could you revert the line deletions I mentioned above, and we can retest/merge? Thanks!

89ce16b removed a few package installations from the dockerfiles for building python artifacts. Revert them. Revert certain package install removal from python artifact dockerfiles

ghostwriternr · 2018-03-05T20:58:47Z

I've made the changes. Thanks for clarifying the build process!

grpc-testing · 2018-03-05T21:24:54Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-03-05T21:34:40Z

[trickle] No significant performance differences

grpc-testing · 2018-03-05T22:17:03Z

[microbenchmarks] No significant performance differences

matt-kwong · 2018-03-05T23:49:54Z

Tests are all passing except for Bazel builds which are still experimental. Thanks for the pull request!

ghostwriternr requested review from jtattermusch, matt-kwong and nicolasnoble as code owners March 4, 2018 02:23

jtattermusch assigned matt-kwong Mar 5, 2018

jtattermusch added the kokoro:run label Mar 5, 2018

kokoro-team removed kokoro:run labels Mar 5, 2018

jtattermusch approved these changes Mar 5, 2018

View reviewed changes

matt-kwong reviewed Mar 5, 2018

View reviewed changes

Revert removals from python artifact dockerfiles

32f7c48

89ce16b removed a few package installations from the dockerfiles for building python artifacts. Revert them. Revert certain package install removal from python artifact dockerfiles

matt-kwong added the kokoro:run label Mar 5, 2018

kokoro-team removed the kokoro:run label Mar 5, 2018

matt-kwong merged commit 673439d into grpc:master Mar 5, 2018

ghostwriternr deleted the task-runner-git-fix branch March 6, 2018 04:40

lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Dockerfiles for python artifacts to use latest git version #14588

Update Dockerfiles for python artifacts to use latest git version #14588

ghostwriternr commented Mar 4, 2018

jtattermusch left a comment

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

matt-kwong commented Mar 5, 2018

ghostwriternr commented Mar 5, 2018 •

edited

Loading

ghostwriternr commented Mar 5, 2018

matt-kwong commented Mar 5, 2018

matt-kwong Mar 5, 2018

matt-kwong Mar 5, 2018

matt-kwong commented Mar 5, 2018

ghostwriternr commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

matt-kwong commented Mar 5, 2018

Update Dockerfiles for python artifacts to use latest git version #14588

Update Dockerfiles for python artifacts to use latest git version #14588

Conversation

ghostwriternr commented Mar 4, 2018

jtattermusch left a comment

Choose a reason for hiding this comment

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

matt-kwong commented Mar 5, 2018

ghostwriternr commented Mar 5, 2018 • edited Loading

ghostwriternr commented Mar 5, 2018

matt-kwong commented Mar 5, 2018

matt-kwong Mar 5, 2018

Choose a reason for hiding this comment

matt-kwong Mar 5, 2018

Choose a reason for hiding this comment

matt-kwong commented Mar 5, 2018

ghostwriternr commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

grpc-testing commented Mar 5, 2018

matt-kwong commented Mar 5, 2018

ghostwriternr commented Mar 5, 2018 •

edited

Loading