Re: Review Request 44674: Supported image name specified with private registry prefix.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 2, 2016, 8:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated April 2, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
>   src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> d3eccd9902a7896d52e6c79b13bd389bb6380b8d 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

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




src/docker/spec.cpp (lines 103 - 105)


Always good to check it explicitly, but we should not get any segfault 
without this check.


- Gilbert Song


On April 2, 2016, 1:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated April 2, 2016, 1:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
>   src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> d3eccd9902a7896d52e6c79b13bd389bb6380b8d 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

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

(Updated April 2, 2016, 1:20 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Supported image name specified with private registry prefix.


Diffs (updated)
-

  include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
  src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
d3eccd9902a7896d52e6c79b13bd389bb6380b8d 

Diff: https://reviews.apache.org/r/44674/diff/


Testing
---

make check

sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"

Tested with private registry localhost:80/ubuntu


Thanks,

Gilbert Song

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44672, 44673, 44674]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 1, 2016, 10:41 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated April 1, 2016, 10:41 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
>   src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> d3eccd9902a7896d52e6c79b13bd389bb6380b8d 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

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

(Updated April 1, 2016, 3:41 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Supported image name specified with private registry prefix.


Diffs (updated)
-

  include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
  src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
d3eccd9902a7896d52e6c79b13bd389bb6380b8d 

Diff: https://reviews.apache.org/r/44674/diff/


Testing
---

make check

sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"

Tested with private registry localhost:80/ubuntu


Thanks,

Gilbert Song

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Jie Yu]

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




src/docker/spec.cpp (lines 103 - 105)


Let's remove this. I think checking if 'registry' is empty or not should be 
done at the upper level.

If 'registry' is empty, just return None().



src/docker/spec.cpp (line 123)


Hum, I am thinking about returning Try here. Return 'https' if port 
!= 80 or not specified.



src/docker/spec.cpp (lines 125 - 127)


This is not needed.



src/docker/spec.cpp (line 142)


I think we should just return string here (instead of a Try). If registry 
is empty, return an empty string.


- Jie Yu


On April 1, 2016, 8:25 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated April 1, 2016, 8:25 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
>   src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> d3eccd9902a7896d52e6c79b13bd389bb6380b8d 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44672, 44673, 44674]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 1, 2016, 8:25 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated April 1, 2016, 8:25 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
>   src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> d3eccd9902a7896d52e6c79b13bd389bb6380b8d 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

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

(Updated April 1, 2016, 1:25 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Supported image name specified with private registry prefix.


Diffs (updated)
-

  include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
  src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
d3eccd9902a7896d52e6c79b13bd389bb6380b8d 

Diff: https://reviews.apache.org/r/44674/diff/


Testing
---

make check

sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"

Tested with private registry localhost:80/ubuntu


Thanks,

Gilbert Song

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Jie Yu]

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




src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (lines 192 
- 209)


Can we move this logic into docker spec file:
```
Result spec::getRegistryPort(const string& registry);
Result spec::getRegistryScheme(const string& registry);
Try spec::getRegistryHost(const string& registry);
```


- Jie Yu


On March 17, 2016, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated March 17, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Guangya Liu]

> On 三月 22, 2016, 2:23 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 395
> > 
> >
> > Just a question: Where did set the schema was set to `https` for this 
> > case?
> 
> Gilbert Song wrote:
> Hey Guangya, we set `https` as default in /uri/fetchers/docker.cpp:
> 
> DockerFetcherPluginProcess::getManifestUri()
> DockerFetcherPluginProcess::getBlobUri()

Thanks Gilbert, got it.


- Guangya


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


On 三月 17, 2016, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated 三月 17, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44672, 44673, 44674]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 17, 2016, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated March 17, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

> On March 22, 2016, 7:23 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 395
> > 
> >
> > Just a question: Where did set the schema was set to `https` for this 
> > case?

Hey Guangya, we set `https` as default in /uri/fetchers/docker.cpp:

DockerFetcherPluginProcess::getManifestUri()
DockerFetcherPluginProcess::getBlobUri()


- Gilbert


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


On March 17, 2016, 9:57 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated March 17, 2016, 9:57 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Guangya Liu]

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




src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 395)


Just a question: Where did set the schema was set to `https` for this case?


- Guangya Liu


On 三月 17, 2016, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated 三月 17, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

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

(Updated March 17, 2016, 9:57 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Supported image name specified with private registry prefix.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6d637ed14f35feb554c8fcc63a7a7e046aaca574 

Diff: https://reviews.apache.org/r/44674/diff/


Testing
---

make check

sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"

Tested with private registry localhost:80/ubuntu


Thanks,

Gilbert Song

Review Request 44674: Supported image name specified with private registry prefix.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Supported image name specified with private registry prefix.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6d637ed14f35feb554c8fcc63a7a7e046aaca574 

Diff: https://reviews.apache.org/r/44674/diff/


Testing
---

make check

sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"

Tested with private registry localhost:80/ubuntu


Thanks,

Gilbert Song