Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Till Toenshoff via Review Board]

> On Oct. 22, 2018, 4:03 p.m., James Peach wrote:
> > src/uri/fetchers/docker.cpp
> > Lines 123 (patched)
> > 
> >
> > If curl is so old that it doesn't accept the `--http1.1` flags, then 
> > it's not going to use HTTP/2 by default. In this case, there's no point 
> > forcing HTTP/1.0.

That way we could avoid forcing 1.0 when 1.1 was actually working (just not 
offered as a flag) - very good point - thank you!


- Till


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69075/#review209868
---


On Oct. 22, 2018, 11:22 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 11:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[James Peach]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69075/#review209868
---




src/uri/fetchers/docker.cpp
Lines 123 (patched)


If curl is so old that it doesn't accept the `--http1.1` flags, then it's 
not going to use HTTP/2 by default. In this case, there's no point forcing 
HTTP/1.0.


- James Peach


On Oct. 22, 2018, 11:22 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 11:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69075/#review209854
---



PASS: Mesos patch 69075 was successfully built and tested.

Reviews applied: `['69075']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2505/mesos-review-69075

- Mesos Reviewbot Windows


On Oct. 22, 2018, 7:22 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 7:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Armand Grillet]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69075/#review209845
---


Ship it!




Ship It!

- Armand Grillet


On Oct. 22, 2018, 1:22 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 1:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Till Toenshoff via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69075/
---

(Updated Oct. 22, 2018, 11:22 a.m.)


Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
James Peach.


Changes
---

Make HTTP 1.1 default but allow for fallback on HTTP 1.0 when curl does not 
support otherwise.


Summary (updated)
-

Updated docker image fetcher to enforce HTTP 1.x.


Bugs: MESOS-8907
https://issues.apache.org/jira/browse/MESOS-8907


Repository: mesos


Description (updated)
---

Modifies the 'curl' invocation that is returning an http::Response,
locking it into HTTP 1.x. Our current HTTP parser is unable to process
HTTP 2 responses.

With the advent of curl 7.47, HTTPS connections are being enforced
towards HTTP 2 rather aggressively. As a result, our image fetcher
fails when recent curl versions are being used for pulling images from
a registry that supports HTTP 2.

HTTP 1.1 is chosen as long as the underlying curl supports the
'--http1.1' flag. If curl does not support that flag, fall back to
using HTTP 1.0.

For allowing all the benefits of HTTP 2 where possible, we do not
adapt any 'curl' invocations that do not attempt to parse headers.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 


Diff: https://reviews.apache.org/r/69075/diff/3/

Changes: https://reviews.apache.org/r/69075/diff/2-3/


Testing
---

`make check`
`sudo ./bin/mesos-tests.sh 
--gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
 on a system with curl 7.59.0 installed.


Thanks,

Till Toenshoff