> 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 > >
