Review Request 60101: Prevent the fetcher from setting overly-permissive fs permissions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60101/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-7298 https://issues.apache.org/jira/browse/MESOS-7298 Repository: mesos Description --- Prevent the fetcher from setting overly-permissive fs permissions. Diffs - src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 Diff: https://reviews.apache.org/r/60101/diff/1/ Testing --- Thanks, Silas Snider
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/ --- (Updated June 14, 2017, 6:31 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Test that bind-mounted host network configuration is mounted readonly. Diffs (updated) - src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 Diff: https://reviews.apache.org/r/58250/diff/7/ Changes: https://reviews.apache.org/r/58250/diff/6-7/ Testing --- Thanks, Silas Snider
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/ --- (Updated June 14, 2017, 2:12 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Test that bind-mounted host network configuration is mounted readonly. Diffs (updated) - src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 Diff: https://reviews.apache.org/r/58250/diff/6/ Changes: https://reviews.apache.org/r/58250/diff/5-6/ Testing --- Thanks, Silas Snider
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/ --- (Updated June 14, 2017, 1:06 a.m.) Review request for mesos and Jie Yu. Bugs: MESOS-7268 https://issues.apache.org/jira/browse/MESOS-7268 Repository: mesos Description --- Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. Diffs (updated) - src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 36217d2e5 src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 3aecd8c5a Diff: https://reviews.apache.org/r/57884/diff/4/ Changes: https://reviews.apache.org/r/57884/diff/3-4/ Testing --- Thanks, Silas Snider
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
> On June 13, 2017, 11:57 p.m., Jie Yu wrote: > > > > Jie Yu wrote: > Also, for nested container, we don't need to do another read only bind > mount. fixed - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/#review177833 --- On June 14, 2017, 1:06 a.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57884/ > --- > > (Updated June 14, 2017, 1:06 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7268 > https://issues.apache.org/jira/browse/MESOS-7268 > > > Repository: mesos > > > Description > --- > > Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 36217d2e5 > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 3aecd8c5a > > > Diff: https://reviews.apache.org/r/57884/diff/4/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/ --- (Updated June 14, 2017, 12:38 a.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Test that bind-mounted host network configuration is mounted readonly. Diffs (updated) - src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 Diff: https://reviews.apache.org/r/58250/diff/5/ Changes: https://reviews.apache.org/r/58250/diff/4-5/ Testing --- Thanks, Silas Snider
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/ --- (Updated June 14, 2017, 12:33 a.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Test that bind-mounted host network configuration is mounted readonly. Diffs (updated) - src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 Diff: https://reviews.apache.org/r/58250/diff/4/ Changes: https://reviews.apache.org/r/58250/diff/3-4/ Testing --- Thanks, Silas Snider
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/ --- (Updated June 14, 2017, 12:32 a.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Test that bind-mounted host network configuration is mounted readonly. Diffs (updated) - src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 Diff: https://reviews.apache.org/r/58250/diff/3/ Changes: https://reviews.apache.org/r/58250/diff/2-3/ Testing --- Thanks, Silas Snider
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
-cb37-4125-9b4c-d1eb95fea3d9-S0 > I0613 17:09:37.213358 36323 executor.cpp:169] Received SUBSCRIBED event > I0613 17:09:37.214424 36323 executor.cpp:173] Subscribed executor on > core-dev > I0613 17:09:37.214689 36323 executor.cpp:169] Received LAUNCH event > I0613 17:09:37.214915 36323 executor.cpp:624] Starting task > 55d299b4-2663-4bd5-980a-2b5df95181a4 > I0613 17:09:37.216902 36323 executor.cpp:468] Running > '/home/jie/workspace/dist/mesos/build/src/mesos-containerizer launch > ' > I0613 17:09:37.219539 36323 executor.cpp:636] Forked command at 36346 > Changing root to > /tmp/CniIsolatorTest_ROOT_INTERNET_CURL_ReadOnlyBindMounts_OTI05M/provisioner/containers/2798da96-2f37-4e27-b737-aa01fc6b4a5d/backends/overlay/rootfses/c00bda57-a3eb-435b-9499-2e1c2bfb7a56 > /dev/mapper/centos-root on /etc/hosts type xfs > (ro,seclabel,relatime,attr2,inode64,logbsize=128k,sunit=256,swidth=512,noquota) > I0613 17:09:37.477999 36300 executor.cpp:915] Command exited with status > 1 (pid: 36346) > > /home/jie/workspace/mesos/src/tests/containerizer/cni_isolator_tests.cpp:1526: > Failure > Expected: TASK_FINISHED > To be equal to: statusFinished->state() > Which is: TASK_FAILED > I0613 17:09:37.492488 36311 exec.cpp:435] Executor asked to shutdown > I0613 17:09:37.492908 36306 executor.cpp:169] Received SHUTDOWN event > I0613 17:09:37.492959 36306 executor.cpp:733] Shutting down > [ FAILED ] CniIsolatorTest.ROOT_INTERNET_CURL_ReadOnlyBindMounts (6247 > ms) > [--] 1 test from CniIsolatorTest (6249 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (6347 ms total) > [ PASSED ] 0 tests. > [ FAILED ] 1 test, listed below: > [ FAILED ] CniIsolatorTest.ROOT_INTERNET_CURL_ReadOnlyBindMounts > ``` > > Silas Snider wrote: > Are you sure that you're running exactly this patch? I'm suspicious about > the line 'c: applet not found', since I'm not trying to run a program called > 'applet' at all? > > Jie Yu wrote: > this is a busybox issue. you didn't set argv[0] to 'sh', i think busybox > will complain like that > > https://stackoverflow.com/questions/19043700/busybox-in-embedded-linux-shows-applet-not-found Yeah, I'll fix that (totally forgot that I had fixed it in one working copy but not the other). It looks like the rest of the test is failing because one (or more) of the mounts is rw -- you're running with my other change, right? - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/#review177834 --- On June 13, 2017, 10:14 p.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58250/ > --- > > (Updated June 13, 2017, 10:14 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > --- > > Test that bind-mounted host network configuration is mounted readonly. > > > Diffs > - > > src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 > > > Diff: https://reviews.apache.org/r/58250/diff/2/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
-cb37-4125-9b4c-d1eb95fea3d9-S0 > I0613 17:09:37.213358 36323 executor.cpp:169] Received SUBSCRIBED event > I0613 17:09:37.214424 36323 executor.cpp:173] Subscribed executor on > core-dev > I0613 17:09:37.214689 36323 executor.cpp:169] Received LAUNCH event > I0613 17:09:37.214915 36323 executor.cpp:624] Starting task > 55d299b4-2663-4bd5-980a-2b5df95181a4 > I0613 17:09:37.216902 36323 executor.cpp:468] Running > '/home/jie/workspace/dist/mesos/build/src/mesos-containerizer launch > ' > I0613 17:09:37.219539 36323 executor.cpp:636] Forked command at 36346 > Changing root to > /tmp/CniIsolatorTest_ROOT_INTERNET_CURL_ReadOnlyBindMounts_OTI05M/provisioner/containers/2798da96-2f37-4e27-b737-aa01fc6b4a5d/backends/overlay/rootfses/c00bda57-a3eb-435b-9499-2e1c2bfb7a56 > /dev/mapper/centos-root on /etc/hosts type xfs > (ro,seclabel,relatime,attr2,inode64,logbsize=128k,sunit=256,swidth=512,noquota) > I0613 17:09:37.477999 36300 executor.cpp:915] Command exited with status > 1 (pid: 36346) > > /home/jie/workspace/mesos/src/tests/containerizer/cni_isolator_tests.cpp:1526: > Failure > Expected: TASK_FINISHED > To be equal to: statusFinished->state() > Which is: TASK_FAILED > I0613 17:09:37.492488 36311 exec.cpp:435] Executor asked to shutdown > I0613 17:09:37.492908 36306 executor.cpp:169] Received SHUTDOWN event > I0613 17:09:37.492959 36306 executor.cpp:733] Shutting down > [ FAILED ] CniIsolatorTest.ROOT_INTERNET_CURL_ReadOnlyBindMounts (6247 > ms) > [--] 1 test from CniIsolatorTest (6249 ms total) > > [--] Global test environment tear-down > [==] 1 test from 1 test case ran. (6347 ms total) > [ PASSED ] 0 tests. > [ FAILED ] 1 test, listed below: > [ FAILED ] CniIsolatorTest.ROOT_INTERNET_CURL_ReadOnlyBindMounts > ``` Are you sure that you're running exactly this patch? I'm suspicious about the line 'c: applet not found', since I'm not trying to run a program called 'applet' at all? - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/#review177834 --- On June 13, 2017, 10:14 p.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58250/ > --- > > (Updated June 13, 2017, 10:14 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > --- > > Test that bind-mounted host network configuration is mounted readonly. > > > Diffs > - > > src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 > > > Diff: https://reviews.apache.org/r/58250/diff/2/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/ --- (Updated June 13, 2017, 10:14 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Test that bind-mounted host network configuration is mounted readonly. Diffs (updated) - src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 Diff: https://reviews.apache.org/r/58250/diff/2/ Changes: https://reviews.apache.org/r/58250/diff/1-2/ Testing --- Thanks, Silas Snider
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/ --- (Updated June 13, 2017, 10:02 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-7268 https://issues.apache.org/jira/browse/MESOS-7268 Repository: mesos Description --- Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. Diffs (updated) - src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 6e95315b70a5d9d3b4b21c4cf235b0a483760190 Diff: https://reviews.apache.org/r/57884/diff/3/ Changes: https://reviews.apache.org/r/57884/diff/2-3/ Testing --- Thanks, Silas Snider
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
> On April 6, 2017, 12:22 a.m., Jie Yu wrote: > > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > > Lines 1523 (patched) > > <https://reviews.apache.org/r/57884/diff/2/?file=1685497#file1685497line1523> > > > > Instead of putting this test under LinuxFilesystemIsolatorTest, I would > > put it under CniIsolatorTest. The code change is in CNI isolator, so > > putting it in CniIsolatorTest is more appropriate? > > > > I would write a similar test like this test: > > TEST_F(CniIsolatorTest, ROOT_OverrideHostname) > > Silas Snider wrote: > I disagree (though not super strongly) -- I think that the functionality > being guarded/tested is that the linux fs isolator correctly mounts the /etc/ > networking files readonly. That it does so *today* by stealth-including the > CNI isolator is an implementation detail, and a surprising one at that. To > protect against the general case, I put it here. > > If you still think it should move, I'm totally willing to move it though. > > Jie Yu wrote: > the mount is done by the CNI isolator, not linux filesystem isolator. > Even in the future, we allow isolator dependency, and let linux filesystem > isolator handles all the bind mount, this is still a logic (read-only bind > mount for /etc) introduced by CNI isolator. Put that in other words, if we > disable cni isolator and enable linux filesystem isolator, your test will > fail. I finally got the CNI tests working locally per our slack discussion yesterday, so I now have a working test that fails before this change and succeeds afterwards in the CNI isolator tests in review request https://reviews.apache.org/r/58250/ - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/#review171183 --- On April 5, 2017, 7:53 p.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57884/ > --- > > (Updated April 5, 2017, 7:53 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7268 > https://issues.apache.org/jira/browse/MESOS-7268 > > > Repository: mesos > > > Description > --- > > Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 6e95315b70a5d9d3b4b21c4cf235b0a483760190 > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > 5e489ef6a522000c55b0fb9a27bce2567f82bb73 > > > Diff: https://reviews.apache.org/r/57884/diff/2/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58250/ --- Review request for mesos. Repository: mesos Description --- Test that bind-mounted host network configuration is mounted readonly. Diffs - src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 Diff: https://reviews.apache.org/r/58250/diff/1/ Testing --- Thanks, Silas Snider
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
> On April 6, 2017, 12:22 a.m., Jie Yu wrote: > > I would actually split this patch into two patches (one for code one for > > test) so that we can easily backport the code if we want to. > > Silas Snider wrote: > Done. (see https://reviews.apache.org/r/58250/) - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/#review171183 --- On April 5, 2017, 7:53 p.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57884/ > --- > > (Updated April 5, 2017, 7:53 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7268 > https://issues.apache.org/jira/browse/MESOS-7268 > > > Repository: mesos > > > Description > --- > > Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 6e95315b70a5d9d3b4b21c4cf235b0a483760190 > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > 5e489ef6a522000c55b0fb9a27bce2567f82bb73 > > > Diff: https://reviews.apache.org/r/57884/diff/2/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
> On April 6, 2017, 12:22 a.m., Jie Yu wrote: > > I would actually split this patch into two patches (one for code one for > > test) so that we can easily backport the code if we want to. Done. > On April 6, 2017, 12:22 a.m., Jie Yu wrote: > > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > > Lines 1523 (patched) > > <https://reviews.apache.org/r/57884/diff/2/?file=1685497#file1685497line1523> > > > > Instead of putting this test under LinuxFilesystemIsolatorTest, I would > > put it under CniIsolatorTest. The code change is in CNI isolator, so > > putting it in CniIsolatorTest is more appropriate? > > > > I would write a similar test like this test: > > TEST_F(CniIsolatorTest, ROOT_OverrideHostname) I disagree (though not super strongly) -- I think that the functionality being guarded/tested is that the linux fs isolator correctly mounts the /etc/ networking files readonly. That it does so *today* by stealth-including the CNI isolator is an implementation detail, and a surprising one at that. To protect against the general case, I put it here. If you still think it should move, I'm totally willing to move it though. - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/#review171183 ------- On April 5, 2017, 7:53 p.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57884/ > --- > > (Updated April 5, 2017, 7:53 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7268 > https://issues.apache.org/jira/browse/MESOS-7268 > > > Repository: mesos > > > Description > --- > > Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 6e95315b70a5d9d3b4b21c4cf235b0a483760190 > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > 5e489ef6a522000c55b0fb9a27bce2567f82bb73 > > > Diff: https://reviews.apache.org/r/57884/diff/2/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/ --- (Updated April 5, 2017, 7:53 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-7268 https://issues.apache.org/jira/browse/MESOS-7268 Repository: mesos Description --- Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. Diffs (updated) - src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 6e95315b70a5d9d3b4b21c4cf235b0a483760190 src/tests/containerizer/linux_filesystem_isolator_tests.cpp 5e489ef6a522000c55b0fb9a27bce2567f82bb73 Diff: https://reviews.apache.org/r/57884/diff/2/ Changes: https://reviews.apache.org/r/57884/diff/1-2/ Testing --- Thanks, Silas Snider
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
- Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/#review170196 --- On April 5, 2017, 7:53 p.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57884/ > --- > > (Updated April 5, 2017, 7:53 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7268 > https://issues.apache.org/jira/browse/MESOS-7268 > > > Repository: mesos > > > Description > --- > > Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 6e95315b70a5d9d3b4b21c4cf235b0a483760190 > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > 5e489ef6a522000c55b0fb9a27bce2567f82bb73 > > > Diff: https://reviews.apache.org/r/57884/diff/2/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
> On March 27, 2017, 5:19 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > > Line 1910 (original), 1910 (patched) > > <https://reviews.apache.org/r/57884/diff/1/?file=1672886#file1672886line1910> > > > > Are you sure this works? I remembered that for read only bind mount, > > you need to do a bind mount and a remount with read only flag. > > https://lwn.net/Articles/281157/ > > > > That probably means we should add a unit test for this. Take a look at > > CniIsolatorTest.ROOT_OverrideHostname which will give you some idea how to > > adding a unit test for this. Sorry it took so long -- I was pulled onto another project briefly. I've updated to match the recommendation, confirmed that it works by adding a test which I confirmed passes now, but does not without my change. - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57884/#review170196 ------- On April 5, 2017, 7:53 p.m., Silas Snider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57884/ > --- > > (Updated April 5, 2017, 7:53 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7268 > https://issues.apache.org/jira/browse/MESOS-7268 > > > Repository: mesos > > > Description > --- > > Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 6e95315b70a5d9d3b4b21c4cf235b0a483760190 > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > 5e489ef6a522000c55b0fb9a27bce2567f82bb73 > > > Diff: https://reviews.apache.org/r/57884/diff/2/ > > > Testing > --- > > > Thanks, > > Silas Snider > >
Review Request 57652: Allow authenticators to return any http Response.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57652/ --- Review request for mesos. Bugs: MESOS-7247 https://issues.apache.org/jira/browse/MESOS-7247 Repository: mesos Description --- Previously only allowed Unauthorized/Forbidden. Diffs - 3rdparty/libprocess/include/process/authenticator.hpp 272d92117547512328c09dfc04c6ca4bf7b6b937 Diff: https://reviews.apache.org/r/57652/diff/1/ Testing --- Thanks, Silas Snider
Re: Review Request 57651: Update http authenticator tests to work with any http response.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57651/ --- (Updated March 15, 2017, 4:47 p.m.) Review request for mesos. Bugs: MESOS-7247 https://issues.apache.org/jira/browse/MESOS-7247 Repository: mesos Description --- Update http authenticator tests to work with any http response. Diffs - src/tests/http_authentication_tests.cpp 0eeed9d19881cf3fa5fec7fb7fedc1e92784f58b Diff: https://reviews.apache.org/r/57651/diff/1/ Testing --- Thanks, Silas Snider
Review Request 57651: Update http authenticator tests to work with any http response.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57651/ --- Review request for mesos. Repository: mesos Description --- Update http authenticator tests to work with any http response. Diffs - src/tests/http_authentication_tests.cpp 0eeed9d19881cf3fa5fec7fb7fedc1e92784f58b Diff: https://reviews.apache.org/r/57651/diff/1/ Testing --- Thanks, Silas Snider
Re: Review Request 51803: Ensured `HealthCheck::HTTPCheckInfo` compatible with the old one.
> On Sept. 12, 2016, 5:23 p.m., Silas Snider wrote: > > include/mesos/mesos.proto, line 374 > > <https://reviews.apache.org/r/51803/diff/3/?file=1496755#file1496755line374> > > > > Why is this being deprecated when the comment above mentions needing to > > support it? > > haosdent huang wrote: > @swsnider, we may not support this in this form. And it is not supported > in HTTP health check now indeed. So mark it deprecated here. Cool, that makes sense. I'll see if we can have a conversation about this on the mailing list, but that shouldn't block this change. - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51803/#review148521 --- On Sept. 12, 2016, 2 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51803/ > --- > > (Updated Sept. 12, 2016, 2 p.m.) > > > Review request for mesos, Alexander Rukletsov, Joseph Wu, Silas Snider, and > Jiang Yan Xu. > > > Bugs: MESOS-6110 > https://issues.apache.org/jira/browse/MESOS-6110 > > > Repository: mesos > > > Description > --- > > Ensured `HealthCheck::HTTPCheckInfo` compatible with the old one. > > > Diffs > - > > include/mesos/mesos.proto eb61b6243202464da2307d06d80700f19f9c25d4 > include/mesos/v1/mesos.proto 62231522f4bfddfc6c440a299dd01080cbe25f6a > > Diff: https://reviews.apache.org/r/51803/diff/ > > > Testing > --- > > > Thanks, > > haosdent huang > >
Re: Review Request 51803: Ensured `HealthCheck::HTTPCheckInfo` compatible with the old one.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51803/#review148521 --- include/mesos/mesos.proto (line 374) <https://reviews.apache.org/r/51803/#comment215963> Why is this being deprecated when the comment above mentions needing to support it? - Silas Snider On Sept. 12, 2016, 2 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51803/ > --- > > (Updated Sept. 12, 2016, 2 p.m.) > > > Review request for mesos, Alexander Rukletsov, Joseph Wu, Silas Snider, and > Jiang Yan Xu. > > > Bugs: MESOS-6110 > https://issues.apache.org/jira/browse/MESOS-6110 > > > Repository: mesos > > > Description > --- > > Ensured `HealthCheck::HTTPCheckInfo` compatible with the old one. > > > Diffs > - > > include/mesos/mesos.proto eb61b6243202464da2307d06d80700f19f9c25d4 > include/mesos/v1/mesos.proto 62231522f4bfddfc6c440a299dd01080cbe25f6a > > Diff: https://reviews.apache.org/r/51803/diff/ > > > Testing > --- > > > Thanks, > > haosdent huang > >
Re: Review Request 51560: Deprecated using health checks without setting the type.
> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote: > > src/health-check/health_checker.cpp, line 183 > > <https://reviews.apache.org/r/51560/diff/2/?file=1489281#file1489281line183> > > > > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* > > Command healthcheck without specifying the type field, but this change will > > make the absence of type only support command health checks. > > haosdent huang wrote: > Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK? > > Silas Snider wrote: > Indeed, but it was in the proto for use by schedulers/executors. > > haosdent huang wrote: > Sorry, I still could not get the idea here. > > If there are not used -> Should be OK although schedulers/executors use > old protocols without using HTTP/TCP health check. -> It should be safe to > set the type to COMMAND if it is empty, right? > > Silas Snider wrote: > The point is: just because Mesos itself didn't use them doesn't mean that > they weren't being set/read by custom executors and frameworks. It's still > part of the API that you support either command or http healthchecks *in the > api* without type being set. > > Silas Snider wrote: > Also, it *was* supported by 1.0.0, so it **must** be supported until 2.0. > > haosdent huang wrote: > >The point is: just because Mesos itself didn't use them doesn't mean > that they weren't being set/read by custom executors and frameworks. It's > still part of the API that you support either command or http healthchecks in > the api without type being set. > > Thank you very much, got it. > > >Also, it was supported by 1.0.0, so it must be supported until 2.0. > > I think we need to discuss about it more. Can you clarify? What needs to be discussed in your opinion? - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51560/#review147468 --- On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51560/ > --- > > (Updated Aug. 31, 2016, 6:29 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joseph Wu. > > > Bugs: MESOS-6110 > https://issues.apache.org/jira/browse/MESOS-6110 > > > Repository: mesos > > > Description > --- > > Deprecated using health checks without setting the type. > > > Diffs > - > > CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b > docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 > src/health-check/health_checker.cpp > f373df19fc8af8e9650be61e3b101e89362a67cd > > Diff: https://reviews.apache.org/r/51560/diff/ > > > Testing > --- > > Could verfied from > https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md > > > Thanks, > > haosdent huang > >
Re: Review Request 51560: Deprecated using health checks without setting the type.
> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote: > > src/health-check/health_checker.cpp, line 183 > > <https://reviews.apache.org/r/51560/diff/2/?file=1489281#file1489281line183> > > > > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* > > Command healthcheck without specifying the type field, but this change will > > make the absence of type only support command health checks. > > haosdent huang wrote: > Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK? > > Silas Snider wrote: > Indeed, but it was in the proto for use by schedulers/executors. > > haosdent huang wrote: > Sorry, I still could not get the idea here. > > If there are not used -> Should be OK although schedulers/executors use > old protocols without using HTTP/TCP health check. -> It should be safe to > set the type to COMMAND if it is empty, right? > > Silas Snider wrote: > The point is: just because Mesos itself didn't use them doesn't mean that > they weren't being set/read by custom executors and frameworks. It's still > part of the API that you support either command or http healthchecks *in the > api* without type being set. Also, it *was* supported by 1.0.0, so it **must** be supported until 2.0. - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51560/#review147468 --- On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51560/ > --- > > (Updated Aug. 31, 2016, 6:29 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joseph Wu. > > > Bugs: MESOS-6110 > https://issues.apache.org/jira/browse/MESOS-6110 > > > Repository: mesos > > > Description > --- > > Deprecated using health checks without setting the type. > > > Diffs > - > > CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b > docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 > src/health-check/health_checker.cpp > f373df19fc8af8e9650be61e3b101e89362a67cd > > Diff: https://reviews.apache.org/r/51560/diff/ > > > Testing > --- > > Could verfied from > https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md > > > Thanks, > > haosdent huang > >
Re: Review Request 51560: Deprecated using health checks without setting the type.
> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote: > > src/health-check/health_checker.cpp, line 183 > > <https://reviews.apache.org/r/51560/diff/2/?file=1489281#file1489281line183> > > > > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* > > Command healthcheck without specifying the type field, but this change will > > make the absence of type only support command health checks. > > haosdent huang wrote: > Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK? > > Silas Snider wrote: > Indeed, but it was in the proto for use by schedulers/executors. > > haosdent huang wrote: > Sorry, I still could not get the idea here. > > If there are not used -> Should be OK although schedulers/executors use > old protocols without using HTTP/TCP health check. -> It should be safe to > set the type to COMMAND if it is empty, right? The point is: just because Mesos itself didn't use them doesn't mean that they weren't being set/read by custom executors and frameworks. It's still part of the API that you support either command or http healthchecks *in the api* without type being set. - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51560/#review147468 --- On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51560/ > --- > > (Updated Aug. 31, 2016, 6:29 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joseph Wu. > > > Bugs: MESOS-6110 > https://issues.apache.org/jira/browse/MESOS-6110 > > > Repository: mesos > > > Description > --- > > Deprecated using health checks without setting the type. > > > Diffs > - > > CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b > docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 > src/health-check/health_checker.cpp > f373df19fc8af8e9650be61e3b101e89362a67cd > > Diff: https://reviews.apache.org/r/51560/diff/ > > > Testing > --- > > Could verfied from > https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md > > > Thanks, > > haosdent huang > >
Re: Review Request 51560: Deprecated using health checks without setting the type.
> On Aug. 31, 2016, 7:07 p.m., Silas Snider wrote: > > src/health-check/health_checker.cpp, line 183 > > <https://reviews.apache.org/r/51560/diff/2/?file=1489281#file1489281line183> > > > > This is incorrect. In 1.0.0, you could specify *either* HTTP *or* > > Command healthcheck without specifying the type field, but this change will > > make the absence of type only support command health checks. > > haosdent huang wrote: > Actually we didn't support HTTP/TCP before 1.0.0. So I think should be OK? Indeed, but it was in the proto for use by schedulers/executors. - Silas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51560/#review147468 --- On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51560/ > --- > > (Updated Aug. 31, 2016, 6:29 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joseph Wu. > > > Bugs: MESOS-6110 > https://issues.apache.org/jira/browse/MESOS-6110 > > > Repository: mesos > > > Description > --- > > Deprecated using health checks without setting the type. > > > Diffs > - > > CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b > docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 > src/health-check/health_checker.cpp > f373df19fc8af8e9650be61e3b101e89362a67cd > > Diff: https://reviews.apache.org/r/51560/diff/ > > > Testing > --- > > Could verfied from > https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md > > > Thanks, > > haosdent huang > >
Re: Review Request 51560: Deprecated using health checks without setting the type.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51560/#review147468 --- src/health-check/health_checker.cpp (line 183) <https://reviews.apache.org/r/51560/#comment214652> This is incorrect. In 1.0.0, you could specify *either* HTTP *or* Command healthcheck without specifying the type field, but this change will make the absence of type only support command health checks. - Silas Snider On Aug. 31, 2016, 6:29 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51560/ > --- > > (Updated Aug. 31, 2016, 6:29 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joseph Wu. > > > Bugs: MESOS-6110 > https://issues.apache.org/jira/browse/MESOS-6110 > > > Repository: mesos > > > Description > --- > > Deprecated using health checks without setting the type. > > > Diffs > - > > CHANGELOG eadf546f96e4f8f83107de3e1421197875a46e2b > docs/upgrades.md dc43cad4d9e0bec0f4017ac53d740687c060b332 > src/health-check/health_checker.cpp > f373df19fc8af8e9650be61e3b101e89362a67cd > > Diff: https://reviews.apache.org/r/51560/diff/ > > > Testing > --- > > Could verfied from > https://github.com/haosdent/mesos/blob/85abb6da058bf8daf4e794df7da8eb5f240b671c/docs/upgrades.md > > > Thanks, > > haosdent huang > >
Re: Review Request 51101: Fixed XFS isolator's handling of old containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51101/#review146066 --- Ship it! Ship It! - Silas Snider On Aug. 16, 2016, 4:43 p.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51101/ > --- > > (Updated Aug. 16, 2016, 4:43 p.m.) > > > Review request for mesos, James Peach and Silas Snider. > > > Bugs: MESOS-6049 > https://issues.apache.org/jira/browse/MESOS-6049 > > > Repository: mesos > > > Description > --- > > Old containers, after recovery of the agent, do not have any entries > stored in `infos` but could still get updated when executors > reregister, tasks terminate and queries for usage are made by the > containerizer. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/xfs/disk.cpp > 60e849322ff755e00eced1b80adadb47bf964cbf > src/tests/containerizer/xfs_quota_tests.cpp > 243ef33686059f3ef46f0c29cc59fa2a79d4ba5b > > Diff: https://reviews.apache.org/r/51101/diff/ > > > Testing > --- > > make check. > > Added a test for this scenario. > > > Thanks, > > Jiang Yan Xu > >
Review Request 48240: Added /dev/fd to the list of symlinks created by filesystem/linux.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48240/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-5543 https://issues.apache.org/jira/browse/MESOS-5543 Repository: mesos Description --- This is necessary to enable bash subshell redirection within the container. Diffs - src/linux/fs.cpp 3190fcec572eddef3e3d5e81f5e508798deee1bd Diff: https://reviews.apache.org/r/48240/diff/ Testing --- I ran a task using the filesystem/linux isolator, that used bash subshell redirection (i.e., `grep -q -E 'something' <(tail -q -c +0 -f somefile)`. Without this line, the task fails. With it, it runs just fine. Thanks, Silas Snider
