----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59950/#review177796 -----------------------------------------------------------
Fix it, then Ship it! src/tests/containerizer/cni_isolator_tests.cpp Lines 1229 (patched) <https://reviews.apache.org/r/59950/#comment251519> I would do the following here instead ``` CniIsolatorTest::CreateSlaveFlags() ``` src/tests/containerizer/cni_isolator_tests.cpp Lines 1241 (patched) <https://reviews.apache.org/r/59950/#comment251518> do you need `mesos::` prefix here? src/tests/containerizer/cni_isolator_tests.cpp Lines 1241-1251 (patched) <https://reviews.apache.org/r/59950/#comment251522> I would introduce a `createNetworkInfo` helper in test/mesos.hpp src/tests/containerizer/cni_isolator_tests.cpp Lines 1260-1271 (patched) <https://reviews.apache.org/r/59950/#comment251521> since we parameterize 'network', we just need one of those? ``` INSTANTIATE_TEST_CASE_P( Network, DefaultExecutorCniTest, Values( Option<v1::NetworkInfo>::none(), Option<v1::NetworkInfo>( v1::createNetworkInfo("__MESOS_TEST__")); ``` src/tests/containerizer/cni_isolator_tests.cpp Lines 1265 (patched) <https://reviews.apache.org/r/59950/#comment251517> 2 lines apart src/tests/containerizer/cni_isolator_tests.cpp Lines 1317-1324 (patched) <https://reviews.apache.org/r/59950/#comment251531> Please follow this example: https://github.com/apache/mesos/blob/master/src/tests/containerizer/runtime_isolator_tests.cpp#L334 the test examples in default executor tests need cleanup (much more verbose) src/tests/containerizer/cni_isolator_tests.cpp Lines 1390-1392 (patched) <https://reviews.apache.org/r/59950/#comment251530> Please use ``` Future<Event::Update> updateRunning; EXPECT_CALL(*scheduler, update(_, _)) .WillOnce(DoAll(FutureArg<1>(&updateRunning), v1::scheduler::SendAcknowledge( frameworkId, offer.agent_id()))); ``` src/tests/containerizer/cni_isolator_tests.cpp Lines 1394-1412 (patched) <https://reviews.apache.org/r/59950/#comment251528> Please use `v1::createCallAccept` - Jie Yu On June 9, 2017, 6:36 p.m., Avinash sridharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59950/ > ----------------------------------------------------------- > > (Updated June 9, 2017, 6:36 p.m.) > > > Review request for mesos, Anand Mazumdar and Jie Yu. > > > Bugs: MESOS-7631 > https://issues.apache.org/jira/browse/MESOS-7631 > > > Repository: mesos > > > Description > ------- > > Added tests for `DefaultExecutor` setting `MESOS_CONTAINER_IP`. > > > Diffs > ----- > > src/tests/containerizer/cni_isolator_tests.cpp > 565e58ae75e918453e4386f5e35a5a844a8b15f8 > > > Diff: https://reviews.apache.org/r/59950/diff/1/ > > > Testing > ------- > > make check > > [==========] Running 2 tests from 2 test cases. > [----------] Global test environment set-up. > [----------] 1 test from HostNetwork/DefaultExecutorCniTest > [ RUN ] HostNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 > I0609 18:16:12.417310 22692 executor.cpp:192] Version: 1.4.0 > I0609 18:16:12.435683 22694 default_executor.cpp:182] Received SUBSCRIBED > event > I0609 18:16:12.441695 22694 default_executor.cpp:186] Subscribed executor on > centos7 > I0609 18:16:12.442006 22694 default_executor.cpp:182] Received LAUNCH_GROUP > event > W0609 18:16:12.444447 22691 default_executor.cpp:443] Setting > `MESOS_CONTAINER_IP` to: 127.0.0.1 > I0609 18:16:12.469161 22691 default_executor.cpp:605] Successfully launched > tasks [ 34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6 ] in child containers [ > 1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e ] > I0609 18:16:12.472266 22693 default_executor.cpp:678] Waiting for child > container > 1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e of > task '34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6' > I0609 18:16:12.481412 22689 default_executor.cpp:182] Received ACKNOWLEDGED > event > I0609 18:16:12.575834 22694 default_executor.cpp:823] Child container > 1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e of > task '34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6' in state TASK_FINISHED exited > with status 0 > I0609 18:16:12.575960 22694 default_executor.cpp:945] Terminating after 1secs > [ OK ] HostNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 > (2366 ms) > [----------] 1 test from HostNetwork/DefaultExecutorCniTest (2368 ms total) > > [----------] 1 test from CniNetwork/DefaultExecutorCniTest > [ RUN ] CniNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 > I0609 18:16:13.042444 22763 executor.cpp:192] Version: 1.4.0 > I0609 18:16:13.059947 22759 default_executor.cpp:182] Received SUBSCRIBED > event > I0609 18:16:13.065265 22759 default_executor.cpp:186] Subscribed executor on > centos7 > I0609 18:16:13.065783 22759 default_executor.cpp:182] Received LAUNCH_GROUP > event > W0609 18:16:13.067163 22763 default_executor.cpp:443] Setting > `MESOS_CONTAINER_IP` to: 10.0.2.15 > I0609 18:16:13.181131 22765 default_executor.cpp:605] Successfully launched > tasks [ 1d265a47-6422-458e-b350-9a1091130c40 ] in child containers [ > 48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 ] > I0609 18:16:13.183614 22764 default_executor.cpp:678] Waiting for child > container > 48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 of > task '1d265a47-6422-458e-b350-9a1091130c40' > I0609 18:16:13.191012 22760 default_executor.cpp:182] Received ACKNOWLEDGED > event > I0609 18:16:13.290139 22763 default_executor.cpp:823] Child container > 48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 of > task '1d265a47-6422-458e-b350-9a1091130c40' in state TASK_FINISHED exited > with status 0 > I0609 18:16:13.290249 22763 default_executor.cpp:945] Terminating after 1secs > [ OK ] CniNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 (728 > ms) > [----------] 1 test from CniNetwork/DefaultExecutorCniTest (729 ms total) > > [----------] Global test environment tear-down > [==========] 2 tests from 2 test cases ran. (3105 ms total) > [ PASSED ] 2 tests. > > > Thanks, > > Avinash sridharan > >