Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp (line 93)


WE don't use 'const' for primitive types. Please use `bool cached` here.



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 
174 - 175)


```
VLOG(1) << "Ignored cached image '" << imageReference << "'";
```


- Jie Yu


On May 14, 2016, 12:58 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated May 14, 2016, 12:58 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

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

(Updated 五月 14, 2016, 12:58 a.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


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


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for docker.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
137af502a66e6a65773c00eaacbe392576376284 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
cf630cc0b67a325529fa04ad2b1708e013b9596a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 

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


Testing
---

make
make check

It is difficult to test this with unit test, but just test with mesos execute 
by setting `force_pull_image` as true and found that the iamge was always 
pulled when `force_pull_image` is true.


Thanks,

Guangya Liu

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

> On 五月 12, 2016, 10:34 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp, line 
> > 174
> > 
> >
> > since you already have the check above, why dont you return None() 
> > there to avoid checking `forcepull` again?
> 
> Guangya Liu wrote:
> if (forcePullImage) {
>   VLOG(1) << "The mesos containerizer is trying to force pull image '"
>   << imageReference << "' from registry";
> }
> //The above check is mainly for logging.
> 
> if (forcePullImage || !storedImages.contains(imageReference)) {
>   return None();
> }
> 
> Gilbert Song wrote:
> How about:
> 
> ```
> if (!storedImages.contains(imageReference)) {
>   return None();
> }
> 
> if (forcePullImage) {
>   VLOG(1) << "The mesos containerizer is trying to force pull image '"
>   << imageReference << "' from registry";
>   
>   return None();
> }
> ```

Good! ;-)


- Guangya


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


On 五月 13, 2016, 3:26 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated 五月 13, 2016, 3:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

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

(Updated 五月 13, 2016, 3:26 a.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


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


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for docker.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
137af502a66e6a65773c00eaacbe392576376284 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
cf630cc0b67a325529fa04ad2b1708e013b9596a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 

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


Testing
---

make
make check

It is difficult to test this with unit test, but just test with mesos execute 
by setting `force_pull_image` as true and found that the iamge was always 
pulled when `force_pull_image` is true.


Thanks,

Guangya Liu

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

> On 五月 12, 2016, 10:34 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp, 
> > lines 169-176
> > 
> >
> > btw, I would prefer check forcepulling after storedImages.contains, 
> > which may be more logical.

I discussed this with @haosdent, if we put `forcePullImage` first, then we 
don't need search storedImages if forcePullImage is true.


> On 五月 12, 2016, 10:34 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp, line 
> > 174
> > 
> >
> > since you already have the check above, why dont you return None() 
> > there to avoid checking `forcepull` again?

if (forcePullImage) {
  VLOG(1) << "The mesos containerizer is trying to force pull image '"
  << imageReference << "' from registry";
}
//The above check is mainly for logging.

if (forcePullImage || !storedImages.contains(imageReference)) {
  return None();
}


- Guangya


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


On 四月 18, 2016, 8:31 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated 四月 18, 2016, 8:31 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Gilbert Song]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp (line 93)


ditto.



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (line 67)


since a protobuf optional field should have a default value, so this one 
will always be overrided.



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 
169 - 176)


btw, I would prefer check forcepulling after storedImages.contains, which 
may be more logical.



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (line 174)


since you already have the check above, why dont you return None() there to 
avoid checking `forcepull` again?


- Gilbert Song


On April 18, 2016, 1:31 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated April 18, 2016, 1:31 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

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

(Updated 四月 18, 2016, 8:31 a.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


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


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for docker.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
137af502a66e6a65773c00eaacbe392576376284 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
cf630cc0b67a325529fa04ad2b1708e013b9596a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 

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


Testing
---

make
make check

It is difficult to test this with unit test, but just test with mesos execute 
by setting `force_pull_image` as true and found that the iamge was always 
pulled when `force_pull_image` is true.


Thanks,

Guangya Liu

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On March 15, 2016, 7:55 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated March 15, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[haosdent huang]

> On March 15, 2016, 7:42 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp, line 188
> > 
> >
> > Because we don't set default value of `force_pull_image` in protobuf 
> > file. Seems here would be `false`. It is a excepted behaviour, right?
> 
> Guangya Liu wrote:
> Yes, there is already a similar parameter here 
> https://reviews.apache.org/r/28190 for docker containerizer, I was using 
> similar logic to handle it in mesos containerizer.

Got it. Thank you for explanation.


- haosdent


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


On March 15, 2016, 7:55 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated March 15, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

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

(Updated 三月 15, 2016, 7:55 a.m.)


Review request for mesos, haosdent huang and Jie Yu.


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


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for docker.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
137af502a66e6a65773c00eaacbe392576376284 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
cf630cc0b67a325529fa04ad2b1708e013b9596a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 

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


Testing
---

make
make check

It is difficult to test this with unit test, but just test with mesos execute 
by setting `force_pull_image` as true and found that the iamge was always 
pulled when `force_pull_image` is true.


Thanks,

Guangya Liu

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

> On 三月 15, 2016, 7:34 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp, line 
> > 93
> > 
> >
> > Should we have default value for `forcePullImage`, for example: 
> > `forcePullImage=false`.

Actually, the default value does not make much sense here as we always put this 
parameter in.


- Guangya


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


On 三月 15, 2016, 7:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated 三月 15, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

> On 三月 15, 2016, 7:42 a.m., haosdent huang wrote:
> > And seems don't contains `appc` part in this patch?

AppC will be handled in the following patch.


> On 三月 15, 2016, 7:42 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp, line 188
> > 
> >
> > Because we don't set default value of `force_pull_image` in protobuf 
> > file. Seems here would be `false`. It is a excepted behaviour, right?

Yes, there is already a similar parameter here 
https://reviews.apache.org/r/28190 for docker containerizer, I was using 
similar logic to handle it in mesos containerizer.


- Guangya


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


On 三月 15, 2016, 7:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated 三月 15, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[haosdent huang]

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



And seems don't contains `appc` part in this patch?


src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 188)


Because we don't set default value of `force_pull_image` in protobuf file. 
Seems here would be `false`. It is a excepted behaviour, right?


- haosdent huang


On March 15, 2016, 7:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated March 15, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[haosdent huang]

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




src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp (line 93)


Should we have default value for `forcePullImage`, for example: 
`forcePullImage=false`.



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (line 167)


Maybe we need log `forcePullImage` option in this log item.



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (line 169)


I suggest change
```
!storedImages.contains(imageReference) || forcePullImage
```
to 
```
forcePullImage || !storedImages.contains(imageReference)
```
So that we don't need search `storedImages` if `forcePullImage` is true.


- haosdent huang


On March 15, 2016, 7:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated March 15, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for docker.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
137af502a66e6a65773c00eaacbe392576376284 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
cf630cc0b67a325529fa04ad2b1708e013b9596a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 

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


Testing
---

make
make check

It is difficult to test this with unit test, but just test with mesos execute 
by setting `force_pull_image` as true and found that the iamge was always 
pulled when `force_pull_image` is true.


Thanks,

Guangya Liu