> On Sept. 16, 2019, 1:24 p.m., Qian Zhang wrote: > > In the description of this patch, I see you mentioned we only have this > > issue with a non-default Docker registry, but that seems not what I found. > > I started Mesos agent without setting the flag `--docker_registry` (so it > > just took the default value: `https://registry-1.docker.io`) and then > > launched a container with an image named `zhq527725/whiteout:latest`, and > > then I see: > > > Pulling image 'zhq527725/whiteout:latest' from > > > 'docker-manifest://registry-1.docker.io:443zhq527725/whiteout?latest#https' > > > to '/opt/qzhang/mesos/store/docker/staging/pKccqL' > > > > As you see, the separator between the hostname and the path was missed as > > well, and I found the container can be launched successfully. So what is > > actually the issue? Just the malformed registry URLs in the above message? > > If I read MESOS-9963 correctly, the issue should be launching container > > fails, right? > > James Peach wrote: > In my testing, the malformed URL is used to pull the images, and that > fails. Is there a code path I'm missing? > > Qian Zhang wrote: > Can you please let me know the detailed error message in your testing? > > IIUC, the issue (missing separator) is in the method `ostream& > operator<<(ostream& stream, const URI& uri)`, and that method is used to > print the message `Pulling image ...` but not used when actually fetching > image, right? > > James Peach wrote: > The curl fetcher uses it via > [stringify](https://github.com/apache/mesos/blob/master/src/uri/fetchers/curl.cpp#L129).
I think you are talking about [this code](https://github.com/apache/mesos/blob/1.9.0/src/uri/fetchers/docker.cpp#L235), right? However the URI used in that code is not the malformed URI that you mentioned in this patch, it is actually re-constructed from the malformed URI (see `DockerFetcherPluginProcess::getManifestUri` and `DockerFetcherPluginProcess::getBlobUri`), so even we see the malformed URL in the message like this `"docker-manifest://registry-1.docker.io:443zhq527725/whiteout?latest#https"`, the URI used by curl is still correct, like: `"https://registry-1.docker.io:443/v2/zhq527725/whiteout/manifests/latest"`. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71441/#review217748 ----------------------------------------------------------- On Sept. 6, 2019, 2:20 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71441/ > ----------------------------------------------------------- > > (Updated Sept. 6, 2019, 2:20 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-9963 > https://issues.apache.org/jira/browse/MESOS-9963 > > > Repository: mesos > > > Description > ------- > > If a non-default Docker registry is used for container image, the Docker > puller would omit the separator between the hostname and the path when > constructing the final URI. > > > Diffs > ----- > > src/tests/containerizer/docker_spec_tests.cpp > 4be0716895132e0406299f7d77f8ceb2d95dbe8a > src/tests/uri_tests.cpp 1d33ab110bed7c0b7ecfa5d608965da5a1729562 > src/uri/utils.cpp 3940dc041c1783eec9e7c950402fd7c42e620d8e > > > Diff: https://reviews.apache.org/r/71441/diff/2/ > > > Testing > ------- > > make check (Fedora 30) > > > Thanks, > > James Peach > >