subject:"Re\: Review Request 43032\: Set Docker labels based on TaskInfo labels."

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-07-09 Thread Joris Van Remoortere


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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-27 Thread Guangya Liu



> On 二月 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 229-251
> > 
> >
> > What about as this:
> > 
> > Try Docker::validateVersion(const Version& minVersion) const
> > {
> >   if (dockerVersion < minVersion) {
> > string msg = "Insufficient version '" + stringify(version.get()) +
> >  "' of Docker. Please upgrade to >=' " +
> >  stringify(minVersion) + "'";
> > return Error(msg);
> >   }
> > 
> >   return Nothing();
> > }
> 
> Abhishek Dasgupta wrote:
> As we have to use dockerVersion as string, we may expect a parsing error. 
> So that error check is necessary.

Thanks @Abhishek, we can wait and see the comments from @tnachen and @jieyu. 
I'm not sure if we need to update version.cpp to enable it support `=` 
operation.


- Guangya


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


On 二月 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-25 Thread Abhishek Dasgupta



> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 229-251
> > 
> >
> > What about as this:
> > 
> > Try Docker::validateVersion(const Version& minVersion) const
> > {
> >   if (dockerVersion < minVersion) {
> > string msg = "Insufficient version '" + stringify(version.get()) +
> >  "' of Docker. Please upgrade to >=' " +
> >  stringify(minVersion) + "'";
> > return Error(msg);
> >   }
> > 
> >   return Nothing();
> > }

As we have to use dockerVersion as string, we may expect a parsing error. So 
that error check is necessary.


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-25 Thread Abhishek Dasgupta



> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 144-172
> > 
> >
> > why update this function, this can also get the docker version, why do 
> > you want to update the docker version to string?

As I mentioned, it is feasible to update docker version as string. If we use 
Version type instead, we can't use "=" assignment operator without modifying 
version.hpp.


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-25 Thread Abhishek Dasgupta



> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.hpp, line 269
> > 
> >
> > What about
> > 
> > `Version dockerVersion`

That is not working. First of all , we have to initialize dockerVersion here as 
there is no Version() constructor, so we have to do like Version dockerVersion 
= Version(0,0,0). Again, afterwards we can't assign any value to it using "=" 
operator as there is no "=" overloaded in version.hpp. I started as you are 
guiding, but ended up writing current implementation.


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-25 Thread Abhishek Dasgupta



> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 123-127
> > 
> >
> > What about update here as:
> > 
> > 
> >   // Get docker version.
> >   Future version = this->version();
> > 
> >   if (!version.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
> > return Error("Timed out getting docker version");
> >   }
> > 
> >   if (version.isFailed()) {
> > return Error("Failed to get docker version: " + version.failure());
> >   }
> >   
> >   dockerVersion = version.get();

As I mentioned , there are some testcases, which does not create a docker 
instance using Docker::create. So, we can't save dockerVersion inside create()


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-25 Thread Guangya Liu


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




src/docker/docker.hpp (line 269)


What about

`Version dockerVersion`



src/docker/docker.cpp (lines 123 - 127)


What about update here as:

  // Get docker version.
  Future version = this->version();

  if (!version.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
return Error("Timed out getting docker version");
  }

  if (version.isFailed()) {
return Error("Failed to get docker version: " + version.failure());
  }
  
  dockerVersion = version.get();



src/docker/docker.cpp (lines 144 - 172)


why update this function, this can also get the docker version, why do you 
want to update the docker version to string?



src/docker/docker.cpp (lines 229 - 248)


What about as this:

Try Docker::validateVersion(const Version& minVersion) const
{
  if (dockerVersion < minVersion) {
string msg = "Insufficient version '" + stringify(version.get()) +
 "' of Docker. Please upgrade to >=' " +
 stringify(minVersion) + "'";
return Error(msg);
  }

  return Nothing();
}


- Guangya Liu


On 二月 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-24 Thread Abhishek Dasgupta



> On Feb. 23, 2016, 9:46 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 123-128
> > 
> >
> > Can you use following logic to get docker version here? 
> > 
> > Future version_ = this->version();
> > 
> > if (!version_.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
> > return Error("Timed out getting docker version");
> >   }
> > 
> > if (version_.isFailed()) {
> > return Error("Failed to get docker version: " + version.failure());
> >   }
> > 
> > // Get the docker version.
> > version = version_;
> > 
> > I think that the logic of `validateVersion` may also need some udpate 
> > by removing the logic of getting versions in `validateVersion` but only 
> > left the compare part.
> 
> Abhishek Dasgupta wrote:
> Just to make it clear, so you are advicing to get the current docker 
> version inside Docker::create and use validateVersion only to compare 
> versions? So, return type of validateversion should be reverted back to 
> Try again.
> 
> Guangya Liu wrote:
> Yes, what do you say?

splitting logic of docker version in Docker::create has some impacts like some 
testcases don't start a Docker from Docker::create. So to be aligned with your 
suggestion, I moved some of the logic from validateVersion to Docker::version() 
and updated the docker variable inside Docker::version(). Thank you.


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-24 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-24 Thread Abhishek Dasgupta


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

(Updated Feb. 24, 2016, 2:10 p.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
  src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
  src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a299c9e0744b5657984e5bb0afbe4874a266ddb6 
  src/tests/containerizer/docker_tests.cpp 
620819330847a10d9dcd817968df9d2b180a9a29 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-23 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Feb. 23, 2016, 8:44 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 23, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-23 Thread Guangya Liu



> On 二月 23, 2016, 9:46 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 123-128
> > 
> >
> > Can you use following logic to get docker version here? 
> > 
> > Future version_ = this->version();
> > 
> > if (!version_.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
> > return Error("Timed out getting docker version");
> >   }
> > 
> > if (version_.isFailed()) {
> > return Error("Failed to get docker version: " + version.failure());
> >   }
> > 
> > // Get the docker version.
> > version = version_;
> > 
> > I think that the logic of `validateVersion` may also need some udpate 
> > by removing the logic of getting versions in `validateVersion` but only 
> > left the compare part.
> 
> Abhishek Dasgupta wrote:
> Just to make it clear, so you are advicing to get the current docker 
> version inside Docker::create and use validateVersion only to compare 
> versions? So, return type of validateversion should be reverted back to 
> Try again.

Yes, what do you say?


- Guangya


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


On 二月 23, 2016, 8:44 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 23, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-23 Thread Abhishek Dasgupta



> On Feb. 23, 2016, 9:46 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 123-128
> > 
> >
> > Can you use following logic to get docker version here? 
> > 
> > Future version_ = this->version();
> > 
> > if (!version_.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
> > return Error("Timed out getting docker version");
> >   }
> > 
> > if (version_.isFailed()) {
> > return Error("Failed to get docker version: " + version.failure());
> >   }
> > 
> > // Get the docker version.
> > version = version_;
> > 
> > I think that the logic of `validateVersion` may also need some udpate 
> > by removing the logic of getting versions in `validateVersion` but only 
> > left the compare part.

Just to make it clear, so you are advicing to get the current docker version 
inside Docker::create and use validateVersion only to compare versions? So, 
return type of validateversion should be reverted back to Try again.


- Abhishek


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


On Feb. 23, 2016, 8:44 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 23, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-23 Thread Guangya Liu


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




src/docker/docker.cpp (lines 123 - 128)


Can you use following logic to get docker version here? 

Future version_ = this->version();

if (!version_.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
return Error("Timed out getting docker version");
  }

if (version_.isFailed()) {
return Error("Failed to get docker version: " + version.failure());
  }

// Get the docker version.
version = version_;

I think that the logic of `validateVersion` may also need some udpate by 
removing the logic of getting versions in `validateVersion` but only left the 
compare part.


- Guangya Liu


On 二月 23, 2016, 8:44 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 23, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-23 Thread Abhishek Dasgupta


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

(Updated Feb. 23, 2016, 8:44 a.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
  src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
  src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a299c9e0744b5657984e5bb0afbe4874a266ddb6 
  src/tests/containerizer/docker_tests.cpp 
620819330847a10d9dcd817968df9d2b180a9a29 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-23 Thread Abhishek Dasgupta



> On Feb. 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 536
> > 
> >
> > Seems like we're increasing trying to find docker versions in our code 
> > base. How about let's capture the docker version and save it in the Docker 
> > class, so we can re-use it and don't have to compute it every time?
> 
> Abhishek Dasgupta wrote:
> How about we store the docker version in docker class inside 
> validateVersion() method?
> 
> Abhishek Dasgupta wrote:
> Capturing docker version in Docker class needs the member variables of 
> version.hpp to be non-constant variables. I don't know if that can be 
> allowed. Are you suggesting this way or any other way possible?
> 
> Guangya Liu wrote:
> Why need update version.http? Is it possible to get the version from 
> `Docker::Create` by splitting some logic from `validateVersion`?
> 
>   Future version_ = this->version();
> 
>   if (!version_.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
> return Error("Timed out getting docker version");
>   }
> 
>   if (version_.isFailed()) {
> return Error("Failed to get docker version: " + version.failure());
>   }
>   
>   // Get the docker version.
>   version = version_;

Ok, so I update the patch. Here, validateVersion is now returning 
Try instead of Try. the returned string from 
validateVersion is the current docker version in string format. This version is 
saved in a dockerVersion class variable inside Docker::create() method. This 
saved version is used in Docker::Run() to validate if TaskInfo labels will be 
used as Docker Label also or not.


- Abhishek


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


On Feb. 22, 2016, 6:37 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 22, 2016, 6:37 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-22 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Feb. 22, 2016, 6:37 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 22, 2016, 6:37 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-21 Thread Guangya Liu



> On 二月 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 536
> > 
> >
> > Seems like we're increasing trying to find docker versions in our code 
> > base. How about let's capture the docker version and save it in the Docker 
> > class, so we can re-use it and don't have to compute it every time?
> 
> Abhishek Dasgupta wrote:
> How about we store the docker version in docker class inside 
> validateVersion() method?
> 
> Abhishek Dasgupta wrote:
> Capturing docker version in Docker class needs the member variables of 
> version.hpp to be non-constant variables. I don't know if that can be 
> allowed. Are you suggesting this way or any other way possible?

Why need update version.http? Is it possible to get the version from 
`Docker::Create` by splitting some logic from `validateVersion`?

  Future version_ = this->version();

  if (!version_.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
return Error("Timed out getting docker version");
  }

  if (version_.isFailed()) {
return Error("Failed to get docker version: " + version.failure());
  }
  
  // Get the docker version.
  version = version_;


- Guangya


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


On 二月 22, 2016, 6:37 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 22, 2016, 6:37 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-21 Thread Abhishek Dasgupta



> On Feb. 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 543
> > 
> >
> > Does label.value() return a Try? What kind of error do we expect from 
> > it?
> 
> Abhishek Dasgupta wrote:
> No, it does not return Try. But I thought to check if there is value 
> inside label. So, used Try. Should I use Result instead?

Used Option instead.


- Abhishek


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


On Feb. 22, 2016, 6:37 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 22, 2016, 6:37 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-21 Thread Abhishek Dasgupta


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

(Updated Feb. 22, 2016, 6:37 a.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
  src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
  src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a299c9e0744b5657984e5bb0afbe4874a266ddb6 
  src/tests/containerizer/docker_tests.cpp 
620819330847a10d9dcd817968df9d2b180a9a29 
  src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-21 Thread Abhishek Dasgupta


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

(Updated Feb. 22, 2016, 6:29 a.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  CHANGELOG e6cc39b4f45317f94145f0a4a1b64215b9a0cbff 
  Doxyfile 779a8ea9e3dd029c4e5bac876a6bd95d402c17de 
  docs/configuration.md b04e873009dc783cc50eb02a459f0587d020ad23 
  docs/fetcher.md 15f70b51294fa647d653bfe4d641dc032ec91f44 
  docs/high-availability-framework-guide.md 
0d9c483985d61b512339f50f395f9360de034e2d 
  docs/home.md 07214b927c112d7a180507ae03245892455fd757 
  docs/images/log-architecture.png 34c57f19387868486882e6fa8bd5d2362113c952 
  docs/images/log-cluster.png 62042d2811dd21bd7b7301f140a9d474c4e8fd07 
  docs/maintenance.md 4d24ec680f3245b12b99e4bb6440ae5aab473460 
  docs/operational-guide.md a4d6710a8a3a4cde10fdccad332e3d539246ec83 
  docs/release-guide.md f0963e750494fcf1f9d80e87bac8dbd6f76adc9f 
  docs/replicated-log-internals.md 4f379a3ff4e957a486fa13d721f4b66333aea6b3 
  include/mesos/master/allocator.hpp a4743c5a31b18d96722a42d526bfb669d30e6e48 
  include/mesos/mesos.proto 11a71cbe25acbc232cea6b5d72484e2e9eef6167 
  include/mesos/v1/mesos.proto 84e933e0bc30aa8f9b6d6047402f449666a80a23 
  site/README.md eb90a86721d9a2636f98512e2e957e375ccc5110 
  site/Rakefile fbf690fb672edf08793a014a4bca5915476d9754 
  site/source/blog/2016-02-12-mesoscon-2016-cfp-is-now-open.md 
514358ec0675bc74ab9e0e648b7103a8a1c2c7e8 
  site/source/index.html.md 3988fa044a264d988b832494490be3956078ff90 
  src/Makefile.am 73e7ff06ba064c9b04f191009522d7808a7ab58e 
  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
  src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
  src/exec/exec.cpp dec7e8814e7151718d1c89381458753f2e22739e 
  src/launcher/executor.cpp 4149f084b0b234fb995df04e9d1ca704feec082e 
  src/main.dox a6765f6f1535146b76488464e674f8d86f5835ba 
  src/master/allocator/mesos/allocator.hpp 
64bce0fb143b109c26f923cd97d5facb393dee9d 
  src/master/allocator/mesos/hierarchical.hpp 
0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/master/allocator/sorter/drf/sorter.hpp 
46b2a9caf13b028a3aee6c1590679f885be90fd6 
  src/master/allocator/sorter/drf/sorter.cpp 
9e863dd0ca5e2f2f0d517cb833687b757bed2c52 
  src/master/allocator/sorter/sorter.hpp 
ba91a38e47065718af87c9b3b7c5b74d25a258df 
  src/master/http.cpp ae6bc7852202480e58f579a5b48ab5b5e5ff9317 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/master/master.cpp b453bc7fca05c192df616b7d80132985b3248547 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
  src/messages/messages.cpp 41dcdb3996ce173fb0a56704053e4b4e03f6dd63 
  src/slave/containerizer/docker.hpp 4d703813de9757a9a26694c9984902e85ba3380b 
  src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
0fe2f486eb733acf738c1c61fc44f820d7401afc 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
134b6c759b769cf335539e49eff817973c7f96a4 
  src/slave/containerizer/mesos/provisioner/appc/cache.cpp 
e075e488c03d20c7b78e1969ea76e95b89fb7cfa 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f3e7c042f2c9980f6274c8185ee8bb89a8b02002 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
2f1d3e002140f34c646aab445a419c9c3d712f99 
  src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 
  src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a 
  src/slave/metrics.hpp d4432136590a3021f6743c79e98db64cb044118a 
  src/slave/metrics.cpp 42c66d7d7176232ccc71f1e040bcae99900f49f8 
  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/allocator.hpp 4081193abed29b99f60254e98ef9ebc981bde859 
  src/tests/command_executor_tests.cpp 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a299c9e0744b5657984e5bb0afbe4874a266ddb6 
  src/tests/containerizer/docker_tests.cpp 
620819330847a10d9dcd817968df9d2b180a9a29 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
6a60962b4593b3521c182c7320331743ccffd4ba 
  src/tests/containerizer/port_mapping_tests.cpp 
983a6be160aefe5a32acb6111bb3c85230ec 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-19 Thread Abhishek Dasgupta



> On Feb. 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 536
> > 
> >
> > Seems like we're increasing trying to find docker versions in our code 
> > base. How about let's capture the docker version and save it in the Docker 
> > class, so we can re-use it and don't have to compute it every time?
> 
> Abhishek Dasgupta wrote:
> How about we store the docker version in docker class inside 
> validateVersion() method?

Capturing docker version in Docker class needs the member variables of 
version.hpp to be non-constant variables. I don't know if that can be allowed. 
Are you suggesting this way or any other way possible?


- Abhishek


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


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-16 Thread Abhishek Dasgupta



> On Feb. 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/tests/containerizer/docker_tests.cpp, line 139
> > 
> >
> > You need to EXPECT_SOME on the find first, other wise it's basically an 
> > assert.

Do you mean -
EXPECT_SOME(json.get().find("Config.Labels.foo"));
EXPECT_EQ("bar",  json.get().find("Config.Labels.foo").get());


- Abhishek


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


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-16 Thread Abhishek Dasgupta



> On Feb. 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 536
> > 
> >
> > Seems like we're increasing trying to find docker versions in our code 
> > base. How about let's capture the docker version and save it in the Docker 
> > class, so we can re-use it and don't have to compute it every time?

How about we store the docker version in docker class inside validateVersion() 
method?


- Abhishek


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


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-16 Thread Abhishek Dasgupta



> On Feb. 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 543
> > 
> >
> > Does label.value() return a Try? What kind of error do we expect from 
> > it?

No, it does not return Try. But I thought to check if there is value inside 
label. So, used Try. Should I use Result instead?


- Abhishek


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


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-12 Thread Guangya Liu


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




src/docker/docker.cpp (line 539)


s/All/all


- Guangya Liu


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-12 Thread Timothy Chen


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




src/docker/docker.cpp (line 536)


Seems like we're increasing trying to find docker versions in our code 
base. How about let's capture the docker version and save it in the Docker 
class, so we can re-use it and don't have to compute it every time?



src/docker/docker.cpp (line 543)


Does label.value() return a Try? What kind of error do we expect from it?



src/tests/containerizer/docker_tests.cpp (line 132)


How about formatting like:

i.. = strings::remove(
  strings::remove(i.., "[", strings::PREFIX),
  "]",
  strings::SUFFIX);



src/tests/containerizer/docker_tests.cpp (line 139)


You need to EXPECT_SOME on the find first, other wise it's basically an 
assert.


- Timothy Chen


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-05 Thread Guangya Liu


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




src/docker/docker.cpp (lines 538 - 540)


The message needs some update to be more clear, such as:

`Your docker version is smaller than 1.6.0, all of the TaskInfo labels 
 will be ignored`

BTW: The log messag do not need a period to the end.


- Guangya Liu


On 二月 5, 2016, 7:15 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 5, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-05 Thread Abhishek Dasgupta



> On Feb. 5, 2016, 9:54 a.m., haosdent huang wrote:
> > src/tests/containerizer/docker_tests.cpp, line 129
> > 
> >
> > Why adjust await time here? I think 15 seconds should be enough to get 
> > version, start container and inspect.

If Labels are tested , then docker version may need another some time to 
connect to docker. Anyway, while testing, I found 15 seconds are two less time 
for downloading image, inspecting versions and so I increased it to 30.


- Abhishek


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


On Feb. 5, 2016, 7:15 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-05 Thread Abhishek Dasgupta



> On Feb. 5, 2016, 9:54 a.m., haosdent huang wrote:
> > src/tests/containerizer/docker_tests.cpp, line 129
> > 
> >
> > Why adjust await time here? I think 15 seconds should be enough to get 
> > version, start container and inspect.
> 
> Abhishek Dasgupta wrote:
> If Labels are tested , then docker version may need another some time to 
> connect to docker. Anyway, while testing, I found 15 seconds are two less 
> time for downloading image, inspecting versions and so I increased it to 30.

If Labels are tested , then docker version may need some more time to connect 
to docker. Anyway, while testing, I found 15 seconds are too less time for 
downloading image, inspecting versions and so I increased it to 30 to be in 
safe side.


- Abhishek


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


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-05 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-05 Thread haosdent huang


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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (lines 538 - 540)


indent seems missing 1 space here.



src/tests/containerizer/docker_tests.cpp (line 129)


Why adjust await time here? I think 15 seconds should be enough to get 
version, start container and inspect.


- haosdent huang


On Feb. 5, 2016, 7:15 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-05 Thread Abhishek Dasgupta


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

(Updated Feb. 5, 2016, 10:52 a.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Feb. 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Guangya Liu


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




src/docker/docker.cpp (lines 533 - 538)


I prefer that we put version logic inside the block of `if 
(labels.isSome()) {` to improve performance.

if (labels.isSome()) {
   get version;
   if (version.isReady()) {
 if version.get() < Version(1, 6, 0) {
   error
 } else {
   handle labels.
 }
  }
}
}


- Guangya Liu


On 二月 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Abhishek Dasgupta


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

(Updated Feb. 5, 2016, 7:15 a.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Guangya Liu


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




src/docker/docker.cpp (lines 540 - 543)


why not 

Try validateVersion =
docker->validateVersion(Version(1, 6, 0));
  if (validateVersion.isError()) {
LOG(WARNING) << "Current docker version does not support 
labels."
   << "So TaskInfo labels are ignored to set as docker"
   << "labels.";
  } else {
  }


- Guangya Liu


On 二月 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Abhishek Dasgupta



> On Feb. 5, 2016, 3:14 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 540-543
> > 
> >
> > why not 
> > 
> > Try validateVersion =
> > docker->validateVersion(Version(1, 6, 0));
> >   if (validateVersion.isError()) {
> > LOG(WARNING) << "Current docker version does not support 
> > labels."
> ><< "So TaskInfo labels are ignored to set as docker"
> ><< "labels.";
> >   } else {
> >   }

It looked like it was designed for other purpose where docker version 
upgradation is a must, but for this label scenario, I did not think we needed 
any hard bound to upgrade to docker higher version. If you suggest we must use 
validateVersion, I can make the necessary changes.What do you say??


- Abhishek


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


On Feb. 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Abhishek Dasgupta



> On Feb. 5, 2016, 3:14 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 540-543
> > 
> >
> > why not 
> > 
> > Try validateVersion =
> > docker->validateVersion(Version(1, 6, 0));
> >   if (validateVersion.isError()) {
> > LOG(WARNING) << "Current docker version does not support 
> > labels."
> ><< "So TaskInfo labels are ignored to set as docker"
> ><< "labels.";
> >   } else {
> >   }
> 
> Abhishek Dasgupta wrote:
> It looked like it was designed for other purpose where docker version 
> upgradation is a must, but for this label scenario, I did not think we needed 
> any hard bound to upgrade to docker higher version. If you suggest we must 
> use validateVersion, I can make the necessary changes.What do you say??

Ok, as I got suggestion on using validateVersion, I am making the necessary 
changes. I wanted to use it too, but for that dubious thought inside me, I 
refrained from using it. Now I can as I have your suggestions with me. Thank 
you.


- Abhishek


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


On Feb. 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Abhishek Dasgupta


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

(Updated Feb. 4, 2016, 6:02 p.m.)


Review request for mesos, Guangya Liu and haosdent huang.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Abhishek Dasgupta



> On Feb. 3, 2016, 12:04 p.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 535
> > 
> >
> > I think need return Error when `version.isError()`

version is a Future type and it does not have isError(), So I had to deviate a 
little to make the changes as you suggest. Can you review it again, please?


- Abhishek


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


On Feb. 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread haosdent huang



> On Feb. 3, 2016, 12:04 p.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 535
> > 
> >
> > I think need return Error when `version.isError()`
> 
> Abhishek Dasgupta wrote:
> version is a Future type and it does not have isError(), So I had to 
> deviate a little to make the changes as you suggest. Can you review it again, 
> please?

Oh, sorry. We have `validateVersion` in docker.cpp, I think you could use it 
directly. And check whether it return error. There is a related snippet code in 
`Try Docker::create(`.
```
  Try validateVersion = docker->validateVersion(Version(1, 0, 0));
  if (validateVersion.isError()) {
return Error(validateVersion.error());
  }
```


- haosdent


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


On Feb. 4, 2016, 6:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 4, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-04 Thread Guangya Liu


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




src/docker/docker.cpp (line 548)


Shall we add some warning message here telling end user that current docker 
version does not support Label, and those labels are ingnored?


- Guangya Liu


On Feb. 3, 2016, 11:33 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 3, 2016, 11:33 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-03 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Feb. 3, 2016, 11:33 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 3, 2016, 11:33 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-03 Thread haosdent huang


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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (line 535)


I think need return Error when `version.isError()`


- haosdent huang


On Feb. 3, 2016, 11:33 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 3, 2016, 11:33 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-03 Thread Abhishek Dasgupta


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

(Updated Feb. 3, 2016, 11:33 a.m.)


Review request for mesos and Guangya Liu.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-02 Thread Abhishek Dasgupta


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

(Updated Feb. 2, 2016, 8:58 a.m.)


Review request for mesos and Guangya Liu.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-02 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Feb. 2, 2016, 8:58 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 2, 2016, 8:58 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-02-01 Thread Abhishek Dasgupta



> On Jan. 31, 2016, 4 p.m., haosdent huang wrote:
> > src/tests/containerizer/docker_tests.cpp, line 122
> > 
> >
> > Do you need verify this in inspect result?

Will you please tell a little bit more about this? Do you want to verify labels 
in inspect result as I found that only container name is available from the 
current inspect result.


- Abhishek


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


On Jan. 31, 2016, 3:22 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Jan. 31, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-01-31 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [43032]

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 Jan. 31, 2016, 3:22 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Jan. 31, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-01-31 Thread Guangya Liu


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




src/docker/docker.cpp (lines 535 - 536)


one line



src/tests/containerizer/docker_tests.cpp (lines 105 - 106)


What about adding one more label without value but only key?


- Guangya Liu


On 一月 31, 2016, 3:22 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 一月 31, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

2016-01-31 Thread haosdent huang


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




src/docker/docker.cpp (line 533)


Do we need check docker version? Because I remember after 1.6.0, docker 
container starting support labels.



src/tests/containerizer/docker_tests.cpp (line 122)


Do you need verify this in inspect result?


- haosdent huang


On Jan. 31, 2016, 3:22 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Jan. 31, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

52 matches

Mail list logo