mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abhishek Dasgupta <a10gu...@linux.vnet.ibm.com>
Subject Re: Review Request 43032: Set Docker labels based on TaskInfo labels.
Date Tue, 23 Feb 2016 08:44:23 GMT


> On Feb. 12, 2016, 3:43 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 536
> > <https://reviews.apache.org/r/43032/diff/6/?file=1236033#file1236033line536>
> >
> >     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> 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<std::string> instead
of Try<Nothing>. 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message