mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Avinash sridharan <avin...@mesosphere.io>
Subject Re: Review Request 43258: Modified agent to get container status from containerizer.
Date Tue, 09 Feb 2016 20:36:01 GMT


> On Feb. 9, 2016, 1:33 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3080-3091
> > <https://reviews.apache.org/r/43258/diff/3/?file=1236975#file1236975line3080>
> >
> >     This looks problematic to me now as we introduce 'status()' interface for isolators.
What if there's a network isolator that wants to set the NetworkInfo in ContainerStatus using
an IP that's different than the agent IP (e.g., think about the calico network isolator).
In that case, we'll still set the ip in the NetworkInfo to be the agent IP?
> >     
> >     My hunch is that we should combine containerizer->status() with this in the
same function because logicially speaking, both of them are mutating TaskStatus with additional
runtime information (ContainerStatus).
> >     
> >     Maybe:
> >     ```
> >     Future<ContainerStatus> getContainerStatus(const ContainerID& containerId)
> >     {
> >       return containerizer->status()
> >         .then([]() {
> >           // Set IP in NetworkInfo if not yet set.
> >         });
> >     }
> >     ```

Modified so that we set the container IP as the IP address of the agent only if the NetworkInfo
is not set by the isolators after `containerizer->status` has been invoked.


> On Feb. 9, 2016, 1:33 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3113-3114
> > <https://reviews.apache.org/r/43258/diff/3/?file=1236975#file1236975line3113>
> >
> >     I am not sure if we want to set NetworkInfo in this case. Setting NetworkInfo.ip
to be the agent IP is definitely problematic to me because it's unclear to me if a network
isolator (e.g., calico) is used for the containr. Blindly returning the agent IP is definitely
not correct.

Since, `ContainerStatus` is being updated after `containerizer->status` is called we shouldn't
face this problem. Though if `NetworkInfo` is set by the `HookModules` we might end up hitting
this particular case.


- Avinash


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


On Feb. 9, 2016, 8:32 p.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43258/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2016, 8:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Bugs: MESOS-4490
>     https://issues.apache.org/jira/browse/MESOS-4490
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Modified agent to get container status from containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
>   src/slave/slave.cpp 9dda3a2c4dc4c355488d34dc8d0606330a756f2a 
>   src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
>   src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
>   src/tests/status_update_manager_tests.cpp 7bedd499a241a61938069381e0d4fafa4b8f96db

> 
> Diff: https://reviews.apache.org/r/43258/diff/
> 
> 
> Testing
> -------
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


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