Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-20 Thread Gilbert Song 

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 199)


Not sure how the indentation should be here, since we do it differently in 
our code base. So I will give a ship it now.


- Gilbert Song


On July 19, 2016, 2:12 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated July 19, 2016, 2:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1c9e17b23d7f2ee49e00b80053e4de1797d73c73 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-19 Thread Srinivas Brahmaroutu 

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

(Updated July 19, 2016, 9:12 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
1c9e17b23d7f2ee49e00b80053e4de1797d73c73 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
6825742546a87c8148097e6e13a94592e3b6754e 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-19 Thread Srinivas Brahmaroutu 

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

(Updated July 19, 2016, 6:30 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
1c9e17b23d7f2ee49e00b80053e4de1797d73c73 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
6825742546a87c8148097e6e13a94592e3b6754e 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-15 Thread Guangya Liu 


> On 七月 15, 2016, 1:42 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/appc/store.cpp, line 199
> > 
> >
> > I recalled Gilbert asked why returning `Future` here but you dropped 
> > the comments without any comment, can you please help explain?
> 
> Gilbert Song wrote:
> It is because we have to do `spec::getManifest()` after we have the 
> ImageId ready.

I see, thanks Gilbert.


- Guangya


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


On 七月 14, 2016, 5:28 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 七月 14, 2016, 5:28 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e900d752b70165236a198f70c8cb24876a678e4b 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-15 Thread Gilbert Song 

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




src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 198 - 199)


Just a style nit. Could we move this `->` down? Please find examples in the 
code base.


- Gilbert Song


On July 14, 2016, 10:28 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated July 14, 2016, 10:28 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e900d752b70165236a198f70c8cb24876a678e4b 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-15 Thread Gilbert Song 


> On July 15, 2016, 6:42 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/appc/store.cpp, line 199
> > 
> >
> > I recalled Gilbert asked why returning `Future` here but you dropped 
> > the comments without any comment, can you please help explain?

It is because we have to do `spec::getManifest()` after we have the ImageId 
ready.


> On July 15, 2016, 6:42 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/appc/store.cpp, line 200
> > 
> >
> > move this before L213.

+1.


> On July 15, 2016, 6:42 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, line 403
> > 
> >
> > 4 spaces for the indent

Thanks Guangya.

@Srini, seems like you missed this one.


- Gilbert


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


On July 14, 2016, 10:28 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated July 14, 2016, 10:28 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e900d752b70165236a198f70c8cb24876a678e4b 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-15 Thread Guangya Liu 

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




src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 199)


I recalled Gilbert asked why returning `Future` here but you dropped the 
comments without any comment, can you please help explain?



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 200)


move this before L213.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 403)


4 spaces for the indent


- Guangya Liu


On 七月 14, 2016, 5:28 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 七月 14, 2016, 5:28 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e900d752b70165236a198f70c8cb24876a678e4b 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-14 Thread Srinivas Brahmaroutu 

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

(Updated July 14, 2016, 5:28 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e900d752b70165236a198f70c8cb24876a678e4b 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
6825742546a87c8148097e6e13a94592e3b6754e 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-13 Thread Srinivas Brahmaroutu 

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

(Updated July 13, 2016, 11:05 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e900d752b70165236a198f70c8cb24876a678e4b 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
6825742546a87c8148097e6e13a94592e3b6754e 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-13 Thread Gilbert Song 

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 202)


Could you add a comment why we always grab the manfest from the first 
ImageId?

People may be confused when they are reading the codes here. Thanks.


- Gilbert Song


On July 13, 2016, 3:26 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated July 13, 2016, 3:26 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e900d752b70165236a198f70c8cb24876a678e4b 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-13 Thread Srinivas Brahmaroutu 

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

(Updated July 13, 2016, 10:26 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Summary (updated)
-

Added appcManifest to ImageInfo and ProvisionInfo.


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


Repository: mesos


Description (updated)
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e900d752b70165236a198f70c8cb24876a678e4b 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
6825742546a87c8148097e6e13a94592e3b6754e 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-08 Thread Gilbert Song 

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




src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 205)


no need this var.



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 211 - 219)


Please check `.isError()`

```
if (manifest.isError()) {
  return Error("");
}

return ImageInfo{rootfses, None(), manifest.get()};
```


- Gilbert Song


On July 5, 2016, 11:32 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated July 5, 2016, 11:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-07 Thread Gilbert Song 

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




src/slave/containerizer/mesos/containerizer.cpp (line 306)


This relates to the isolator dependency. Please move it right above 
`docker/runtime`.



src/slave/containerizer/mesos/provisioner/provisioner.hpp (line 24)


One newline below.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 335)


Why do you add this logic? It seems incorrect to me. Please remove it.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 337)


add two more space at the beginning.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 404)


Please use None() for appc case here.

Fix the indentation please.



src/slave/containerizer/mesos/provisioner/store.hpp (line 25)


One newline below.


- Gilbert Song


On July 5, 2016, 11:32 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated July 5, 2016, 11:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-06 Thread Srinivas Brahmaroutu 

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

(Updated July 6, 2016, 6:32 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
6825742546a87c8148097e6e13a94592e3b6754e 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-01 Thread Guangya Liu 


> On 七月 1, 2016, 7:06 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/appc/store.cpp, lines 212-213
> > 
> >
> > 1) You should add a "'" to the end of the log message.
> > 2) s/image/Appc image
> > 3) Here should check `_manifest.isError()`
> > 4) I prefer `return Error` here to fail fast.
> > 
> > if (_manifest.isError()) {
> >   return Error(
> >   "Failed to get manifest for AppC image '" + 
> > appc.SerializeAsString() + 
> >   "': " + _manifest.error());
> > }
> > 
> > VLOG(1) << "Failed to get manifest for Appc image  '"
> > << appc.SerializeAsString() << "'";

Please ignore the following in above comment.

VLOG(1) << "Failed to get manifest for Appc image  '"
<< appc.SerializeAsString() << "'";


- Guangya


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


On 六月 30, 2016, 6 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 六月 30, 2016, 6 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-01 Thread Guangya Liu 

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




src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 207 - 208)


new line here



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 212 - 213)


1) You should add a "'" to the end of the log message.
2) s/image/Appc image
3) Here should check `_manifest.isError()`
4) I prefer `return Error` here to fail fast.

if (_manifest.isError()) {
  return Error(
  "Failed to get manifest for AppC image '" + appc.SerializeAsString() 
+ 
  "': " + _manifest.error());
}

VLOG(1) << "Failed to get manifest for Appc image  '"
<< appc.SerializeAsString() << "'";



src/slave/containerizer/mesos/provisioner/provisioner.hpp (lines 24 - 25)


new line here



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 334 - 335)


I think you do not need to add the `APPC` condition here as the current 
logic is OK.

The logic here is handling the case where the docker image is single layer 
and appc image.

if (imageInfo.layers.size() == 1 || image.type() != Image::DOCKER) {
  return ProvisionInfo{
rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};
}



src/slave/containerizer/mesos/provisioner/store.hpp (lines 25 - 26)


new line here


- Guangya Liu


On 六月 30, 2016, 6 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 六月 30, 2016, 6 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-30 Thread Srinivas Brahmaroutu 

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

(Updated June 30, 2016, 6 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
d984efd4742ec084d66538c48a36ea768832324d 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
249acad49122d988e44744384bcf840b941c0997 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-29 Thread Srinivas Brahmaroutu 

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

(Updated June 30, 2016, 5:39 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
d984efd4742ec084d66538c48a36ea768832324d 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
249acad49122d988e44744384bcf840b941c0997 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-28 Thread Srinivas Brahmaroutu 

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

(Updated June 28, 2016, 10:53 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
  src/slave/containerizer/mesos/containerizer.cpp 
d984efd4742ec084d66538c48a36ea768832324d 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
249acad49122d988e44744384bcf840b941c0997 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-28 Thread Srinivas Brahmaroutu 


> On June 28, 2016, 3:38 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, lines 334-335
> > 
> >
> > Can we simplify the logic as:
> > 
> > if (imageInfo.layers.size() == 1) {
> >   return ProvisionInfo{
> > rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};
> > }

I try not to generalize. I like to keep the checks in case other containerizers 
supported.


- Srinivas


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


On June 28, 2016, 6:39 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated June 28, 2016, 6:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-28 Thread Guangya Liu 

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




include/mesos/appc/spec.proto 


This comment should be addressed in https://reviews.apache.org/r/49207/

I saw that you fixed some commentes for other patches, it is better address 
the comments in the original patch.

Ditto for the update of include/mesos/slave/isolator.proto.



src/slave/containerizer/mesos/containerizer.cpp (line 317)


Put `appc/runtime` above `docker/runtime`?



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 212 - 213)


VLOG(1) << "Failed to get manifest for AppC image  '"
<< appc.SerializeAsString() << "'";

Please make sure the `<<` keep aligned.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 334 - 335)


Can we simplify the logic as:

if (imageInfo.layers.size() == 1) {
  return ProvisionInfo{
rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};
}



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 403 - 404)


What about 

return ProvisionInfo{
  rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};


- Guangya Liu


On 六月 28, 2016, 6:39 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 六月 28, 2016, 6:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-28 Thread Srinivas Brahmaroutu 


> On June 27, 2016, 6:14 p.m., Gilbert Song wrote:
> > Srini, I guess you may want to add another patch before this one to 
> > implement the runtime isolator then plug it in using this patch. And you 
> > may need to rebase and figure out why your patches failed on review bot.
> 
> Guangya Liu wrote:
> The build failure is caused by `{"appc/runtime", 
> ::create},`, here should be 
> "App`C`RuntimeIsolatorProcess".
> 
> Gilbert Song wrote:
> Thanks Guangya. @Srini, please make sure your patches are compilable and 
> tested before pushing to the review board.

@gilbert I think we can test runtime without having implementation of the 
isolator. 
patch to add tests, to test appc tar ball
patch to add implementation for exec, workingDirectory and environment
patch to add tests to see if exec, workingDirectory and environment are 
properly updated at runtime.


- Srinivas


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


On June 28, 2016, 6:39 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated June 28, 2016, 6:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-28 Thread Srinivas Brahmaroutu 

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

(Updated June 28, 2016, 6:39 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
  include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
  src/slave/containerizer/mesos/containerizer.cpp 
d984efd4742ec084d66538c48a36ea768832324d 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
249acad49122d988e44744384bcf840b941c0997 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-27 Thread Gilbert Song 


> On June 27, 2016, 11:14 a.m., Gilbert Song wrote:
> > Srini, I guess you may want to add another patch before this one to 
> > implement the runtime isolator then plug it in using this patch. And you 
> > may need to rebase and figure out why your patches failed on review bot.
> 
> Guangya Liu wrote:
> The build failure is caused by `{"appc/runtime", 
> ::create},`, here should be 
> "App`C`RuntimeIsolatorProcess".

Thanks Guangya. @Srini, please make sure your patches are compilable and tested 
before pushing to the review board.


> On June 27, 2016, 11:14 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 311
> > 
> >
> > Move above docker runtime.
> 
> Guangya Liu wrote:
> Gilbert, just a question, do you think we need keep alphina order here?

For here, we shouldn't. The order in the vector may impact on isolator 
dependency.


- Gilbert


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


On June 25, 2016, 3:53 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated June 25, 2016, 3:53 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-27 Thread Gilbert Song 

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



Srini, I guess you may want to add another patch before this one to implement 
the runtime isolator then plug it in using this patch. And you may need to 
rebase and figure out why your patches failed on review bot.


src/slave/containerizer/mesos/containerizer.cpp (lines 83 - 86)


Move above docker runtime



src/slave/containerizer/mesos/containerizer.cpp (line 311)


Move above docker runtime.



src/slave/containerizer/mesos/containerizer.cpp (line 1013)


You need to CHECK at most one manifest exist.


- Gilbert Song


On June 25, 2016, 3:53 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated June 25, 2016, 3:53 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-25 Thread Guangya Liu 


> On 六月 26, 2016, 2:52 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/appc/store.cpp, lines 211-212
> > 
> >
> > VLOG(1) << "Failed to get manifest for image  '"
> > << appc.SerializeAsString() << "'";

VLOG(1) << "Failed to get manifest for AppC image  '"
<< appc.SerializeAsString() << "'";


- Guangya


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


On 六月 25, 2016, 10:53 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 六月 25, 2016, 10:53 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-06-25 Thread Guangya Liu 

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




src/slave/containerizer/mesos/containerizer.cpp (lines 78 - 83)


adjust the order



src/slave/containerizer/mesos/containerizer.cpp (line 311)


s/AppcRuntimeIsolatorProcess/AppCRuntimeIsolatorProcess



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 209 - 210)


add a blank line here



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 211 - 212)


VLOG(1) << "Failed to get manifest for image  '"
<< appc.SerializeAsString() << "'";



src/slave/containerizer/mesos/provisioner/provisioner.hpp (line 63)


s/Appc/AppC



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 334 - 336)


You will lost the appcManifest if the AppC image has only one layer.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 404)


Not yours, but for windows, I think that we also do not need 
`imageInfo.dockerManifest` to be here.



src/slave/containerizer/mesos/provisioner/store.hpp (line 48)


s/Appc/AppC


- Guangya Liu


On 六月 25, 2016, 10:53 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 六月 25, 2016, 10:53 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>