Re: Proposal: moving Mesos website to project codebase
+1 On Fri, Oct 9, 2015 at 8:59 AM, haosdent wrote: > +1! > On Oct 9, 2015 10:37 PM, "Kevin Sweeney" wrote: > > > +1! > > > > On Fri, Oct 9, 2015 at 3:35 PM Marco Massenzio > > wrote: > > > > > +1 > > > > > > Dave - great stuff! > > > > > > *Marco Massenzio* > > > > > > *Distributed Systems Engineerhttp://codetrips.com < > http://codetrips.com > > >* > > > > > > On Fri, Oct 9, 2015 at 3:05 PM, Dave Lester > wrote: > > > > > > > As part of the #MesosCon Europe hackathon, my team has been making > > > > improvements to the website. Among these changes, we'd like to > propose > > > > changing where the website source files live by moving them to the > main > > > > Mesos codebase. Our current progress / working branch of this is > > > > available on GitHub: https://github.com/fayusohenson/mesos/tree/site > > > > > > > > * What does this mean? * > > > > We've added a /site directory to the Mesos codebase, which includes > the > > > > website source files. Today, these live in subversion. The rake file > > and > > > > other parts of building the website all work in this new environment, > > > > plus a number of related fixes (image linking, etc). > > > > > > > > For committers that are familiar with the current model for pushing > the > > > > site live, this immediate change still requires us `svn commit` the > > > > /publish directory for the website (static files that are generated). > > > > > > > > * Why this change? * > > > > 1. Today we do not have an easy process for the community to > contribute > > > > to the project website. By merging this with the Mesos codebase, it > > will > > > > be significantly easier to send a review or pull request. > > > > 2. It'll be easier for committers to manage the website, and check > that > > > > documentation changes render on the website properly before > committing. > > > > Because it's difficult to do today, this is often not checked. :( > > > > 3. It's a solid step toward an automated deployment of the website in > > > > the future: https://issues.apache.org/jira/browse/MESOS-1309 > > > > > > > > * Who approves of this change? * > > > > As the Mesos website maintainer, I feel good about this change and > its > > > > direction for the project. Before committing this change, I'd like > > > > community support that including this in the main Mesos codebase > makes > > > > sense. > > > > > > > > Comments? Questions? > > > > > > > > Dave > > > > > > > > > > -- @paul_b
Lets stop using the CHECK macro in the test harness.
We are currently using the Google log CHECK macros (CHECK_SOME,
CHECK_NOTNULL etc) in the test harness, usually to verify test setup. When
these checks fail, it causes the test harness to abort rather than simply
move onto the next test. The abort prevents any subsequent tests from
running, hiding errors and preventing the generation of the XML test report.
I would like to propose that we eliminate the use of CHECK in the test
harness and replace it with the appropriate Google test macros to fail the
test case.
I
am not proposing that we change the use of CHECK outside the test harness
(although CHECK calls in master and slave can also kill the test harness).
For void functions, CHECK can
easily
be replaced with the corresponding ASSERT equivalent.
For non-void function, ASSERT cannot be used because it does not return the
correct data type and hence we need to use a combination of ADD_FAILURE()
and return.
For example:
CHECK(foo)
would become:
if(!foo) {
ADD_FAILURE();
return anything;
}
If there is general agreement, I will raise tickets to update the Mesos
testing patterns document and each of the test cases.
Thanks
-- Paul Brett
Re: Beware of ASSERT_* Placement
Michael I think Ben's suggestion of using Try<> is just what we want for common functions. In regards to ASSERTs, they can cause tests to be skipped within instantiations of the fixtures or test case as expected. For example, If you look at tests such as SlaveRecoveryTest::ReconnectExecutor, it has 9 ASSERTs in a single test case. The first 5 are in setup code and seem pretty reasonable but the last 4 are: 489 // Executor should inform about the unacknowledged update. 490 ASSERT_EQ(1, reregister.updates_size()); 491 const StatusUpdate& update = reregister.updates(0); 492 ASSERT_EQ(task.task_id(), update.status().task_id()); 493 ASSERT_EQ(TASK_RUNNING, update.status().state()); 494 495 // Scheduler should receive the recovered update. 496 AWAIT_READY(status); 497 ASSERT_EQ(TASK_RUNNING, status.get().state()); So looking at this code, I suspect that lines 492 and 493 might be better as EXPECT? What about 497? What follows afterwards is only cleanup code, so either it is not necessary and we can omit it or 497 should be an expect. Looking through the tests directory, this appears to be a common pattern. Of course, it is all harmless while the code is passing the tests but when a change breaks things, the scope of the breakage can be obscured because of the skipped test conditions. Given the restrictions you point out on the use of ASSERT combined with the ability to hide failing tests, I believe we should have a strong preference for EXPECT over ASSERT unless it is clear that every subsequent test in the test cast is dependent on the result of this test. Just my 5c worth @paul_b On Mon, Jul 27, 2015 at 7:34 PM, Michael Park wrote: > Paul, > > With ASSERT, I completely agree with you about the perils of using ASSERT > > that you list above, but additionally we have cases where ASSERT exits a > > test fixture skipping later tests that might or might not have failed. > > > We should only be using *ASSERT_** in cases where it doesn't make sense to > proceed with the rest of the test if the condition fails, so exiting the > test case seems like it's exactly what we would want. If you're saying that > we currently use it incorrectly, then yeah, we should perhaps write a guide > to help with how to use it correctly. But it sounds like you're saying we > shouldn't use it at all? > > Since the CHECK macro aborts the test harness, a single test failure > > prevents tests from running on all the remaining tests. Particularly > > annoying for anyone running automated regression tests. > > > Perhaps my suggestion of resorting to *CHECK_** was not a good one, but I > still don't think *EXPECT_** is what we want. If we have a condition in > which it doesn't make sense to proceed with the rest of the test, we should > stop. Perhaps the helper function should return a *Try* as Ben suggested, > proceeded by an *ASSERT_** of the result within the test case or something > like that. > > I mainly wanted to inform folks of this limitation and the corresponding > confusing error message that follows. > > On 27 July 2015 at 18:42, Benjamin Mahler > wrote: > > > Michael, note that we've avoided having ASSERT_ or EXPECT_ inside test > > helper methods because they print out the line number of the helper > method, > > rather than the line number where the helper method was called from the > > test. This is why we've been pretty careful when adding helpers and have > > tried to push assertions into the test itself (e.g. helper returns a Try > > instead of internally asserting). > > > > Paul, are you saying that ASSERT within one case in a fixture will stop > > running all other cases for the fixture? Do you have a pointer to this? > > Sounds surprising. > > > > On Mon, Jul 27, 2015 at 3:04 PM, Paul Brett > > wrote: > > > > > Mike > > > > > > I would suggest that we want to avoid both ASSERT and CHECK macros in > > > tests. > > > > > > With ASSERT, I completely agree with you about the perils of using > ASSERT > > > that you list above, but additionally we have cases where ASSERT exits > a > > > test fixture skipping later tests that might or might not have failed. > > > > > > Since the CHECK macro aborts the test harness, a single test failure > > > prevents tests from running on all the remaining tests. Particularly > > > annoying for anyone running automated regression tests. > > > > > > We should add this to either the style guide or mesos-testing-patterns. > > > > > > -- @paul_b > > > > > > On Mon, Jul 27, 2015 at 2:28 PM, Michael Park > w
Re: Beware of ASSERT_* Placement
Mike
I would suggest that we want to avoid both ASSERT and CHECK macros in tests.
With ASSERT, I completely agree with you about the perils of using ASSERT
that you list above, but additionally we have cases where ASSERT exits a
test fixture skipping later tests that might or might not have failed.
Since the CHECK macro aborts the test harness, a single test failure
prevents tests from running on all the remaining tests. Particularly
annoying for anyone running automated regression tests.
We should add this to either the style guide or mesos-testing-patterns.
-- @paul_b
On Mon, Jul 27, 2015 at 2:28 PM, Michael Park wrote:
> I've had at least 3 individuals who ran into the issue of *ASSERT_** macro
> placement and since the generated error message is less than useless, I
> would like to share with you what the issue is.
>
> The source of the issue is that *ASSERT_** macros from *gtest* can only be
> placed in functions that return *void* as described in Assertion Placement
> <
> https://code.google.com/p/googletest/wiki/AdvancedGuide#Assertion_Placement
> >
> .
>
> By placing it in a non-void function, you get useless error messages like
> this:
>
> From *GCC*: "*error: void value not ignored as it ought to be*"
> From *Clang*: "*error: could not convert
> ‘testing::internal::AssertHelper((testing::TestPartResult::Type)2u, ((const
> char*)"../../src/tests/containerizer/port_mapping_tests.cpp"), 320,
>
> gtest_ar.testing::AssertionResult::failure_message()).testing::internal::AssertHelper::operator=(testing::Message())’
> from ‘void’ to ‘int’*"
>
> I think the following are typical situations that result in this mess:
>
>- Using them in *constructors/destructors*. For example, it would be
>really confusing if you're simply translating a *SetUp*/*TearDown* of a
>test fixture to be *constructor/destructor* instead.
>- Using them in *main*, since it returns an *int*.
>- Refactoring a chunk of logic from a test case into a helper function
>that doesn't return *void*. For example, if we were factor out the
>following code inside of a test case:
>
>
>
>
> *AWAIT_READY(offers); ASSERT_EQ(1u, offers.get().size()); offer =
>offers.get()[0]; *
>into a function like this:
>
>
>
>
>
> *Offer getOffer(Future> &offers) {
>AWAIT_READY(offers); ASSERT_EQ(1u, offers.get().size());
> return
>offers.get()[0]; }*
>
>this innocent-looking transformation would trigger the obscure error
>message and you'll be upset once you figure out why.
>
> If you encounter this case, prefer to resort to *CHECK_** from *glog*,
> rather
> than *EXPECT_**, since *CHECK_** has closer semantics.
>
> I hope this will help folks save time and also reduce the amount of head
> banging!
>
> Thanks,
>
> MPark.
>
--
-- Paul Brett
Re: [jira] [Commented] (MESOS-3035) As a Developer I would like a standard way to run a Subprocess in libprocess
Marco
Any progress on getting an update to this? It is a blocker on fixing the
perf issues.
Thanks
-- Paul
On Mon, Jul 13, 2015 at 3:24 PM, Paul Brett (JIRA) wrote:
>
> [
> https://issues.apache.org/jira/browse/MESOS-3035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14625487#comment-14625487
> ]
>
> Paul Brett commented on MESOS-3035:
> ---
>
> +1
>
> > As a Developer I would like a standard way to run a Subprocess in
> libprocess
> >
>
> >
> > Key: MESOS-3035
> > URL: https://issues.apache.org/jira/browse/MESOS-3035
> > Project: Mesos
> > Issue Type: Story
> > Components: libprocess
> >Reporter: Marco Massenzio
> >Assignee: Marco Massenzio
> >
> > As part of MESOS-2830 and MESOS-2902 I have been researching the ability
> to run a {{Subprocess}} and capture the {{stdout / stderr}} along with the
> exit status code.
> > {{process::subprocess()}} offers much of the functionality, but in a way
> that still requires a lot of handiwork on the developer's part; we would
> like to further abstract away the ability to just pass a string, an
> optional set of command-line arguments and then collect the output of the
> command (bonus: without blocking).
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)
>
--
-- Paul Brett
Re: [jira] [Commented] (MESOS-2993) Document per container unique egress flow and network queueing statistics
I should have it for you in a couple of hours. On Tue, Jul 7, 2015 at 11:12 AM, Adam B (JIRA) wrote: > > [ > https://issues.apache.org/jira/browse/MESOS-2993?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14617110#comment-14617110 > ] > > Adam B commented on MESOS-2993: > --- > > [~pbrett] Is there a draft/review for this yet? Wondering if we can get > this in for the next release candidate (rc2). > > > Document per container unique egress flow and network queueing > statistics > > > -- > > > > Key: MESOS-2993 > > URL: https://issues.apache.org/jira/browse/MESOS-2993 > > Project: Mesos > > Issue Type: Bug > > Components: documentation, isolation > > Affects Versions: 0.23.0 > >Reporter: Paul Brett > >Assignee: Paul Brett > > Labels: twitter > > > > Document new network isolation capabilities in 0.23 > > > > -- > This message was sent by Atlassian JIRA > (v6.3.4#6332) > -- -- Paul Brett
Re: Project wide updates to #include statements
Thanks Marco, it would be great to get to a better checker and then turn on more checks for the things we normally comment on in reviews. The style guide currently states the following order: 1. dir2/foo2.h. 2. C system files. 3. C++ system files. 4. Other libraries' .h files. 5. Your project's .h files. Generally speaking,we seem to use the following. 1. dir2/foo2.h - we don't go this 2. C system files. 3. C++ system files. 4. C library files - leveldb 5. C++ library files - boost, glog, gmock, libev, picojson, protobuf, zookeeper 6. Project's .h files - I think these are all wrapped in .hpp files 7. Project .hpp files - mesos, stout, libprocess As Kapil has pointed out on MESOS-2927, all the library files should be identified by '#include <...>' and all the project files should use '#include "..."' - i don't know yet if we have been consistent about this. I have created the stout patch on this basis and plan to incorporate any comments before moving onto libprocess. -- Paul On Thu, Jun 25, 2015 at 3:31 PM, Marco Massenzio wrote: > As I mentioned in the review that Paul submitted, I've been working on > cpplint.py to make it more Mesos-friendly. > I have also submitted a few Pull Requests > <https://github.com/google/styleguide/pulls> to the original github repo, > but got neither love nor attention. > > My fork is here: https://github.com/massenz/styleguide > and the code in the `master` branch ( > https://github.com/massenz/styleguide/tree/master) has all my changes; it > currently works well with the existing code (in that, submitted, valid > Mesos code does not raise errors) apart from the opening brace on a newline > for multi-line method declarations. > > Love to get contributions and pull requests folks, feel free to submit! > > An example CPPLINT.cfg that works with the code in `master` is something > like this: > > $ cat CPPLINT.cfg > # Apache Mesos cpplint custom file > > extensions=cpp,hpp > access_keywords_indent=0 > headers=h,hpp > custom_headers=mesos,process,stout > set braces_newline > > PS - am I the only one to find it hilarious that code that supposedly > checks on style correctness is written in some of the least readable, badly > PEP8-violating Python? :) > > *Marco Massenzio* > *Distributed Systems Engineer* > > On Thu, Jun 25, 2015 at 3:18 PM, Paul Brett > wrote: > > > The style guide prescribes the order of header file inclusions for the > > project and requires that we #include or make explicit forward > declarations > > for any functions we use, however we were only enforcing this at review > > time manually and not commit time. I would like to turn on the checks at > > commit time, so I am in the process of raising changes against stout, > > libprocess and mesos to bring the code base into compliance. Once this > is > > completed, I propose to update cpplint.py and mesos-style.py to enforce > the > > style guide. > > > > Anyone interested can comment on the following tickets: > > > > https://issues.apache.org/jira/browse/MESOS-2926 Extend > > mesos-style.py/cpplint.py to check #include files > > https://issues.apache.org/jira/browse/MESOS-2927 Update mesos #include > > headers > > https://issues.apache.org/jira/browse/MESOS-2928 Update stout #include > > headers > > https://issues.apache.org/jira/browse/MESOS-2929 Update libprocess > > #include > > headers > > > > > > -- Paul Brett > > > -- -- Paul Brett
Project wide updates to #include statements
The style guide prescribes the order of header file inclusions for the project and requires that we #include or make explicit forward declarations for any functions we use, however we were only enforcing this at review time manually and not commit time. I would like to turn on the checks at commit time, so I am in the process of raising changes against stout, libprocess and mesos to bring the code base into compliance. Once this is completed, I propose to update cpplint.py and mesos-style.py to enforce the style guide. Anyone interested can comment on the following tickets: https://issues.apache.org/jira/browse/MESOS-2926 Extend mesos-style.py/cpplint.py to check #include files https://issues.apache.org/jira/browse/MESOS-2927 Update mesos #include headers https://issues.apache.org/jira/browse/MESOS-2928 Update stout #include headers https://issues.apache.org/jira/browse/MESOS-2929 Update libprocess #include headers -- Paul Brett
Re: [jira] [Closed] (MESOS-2780) Non-POD static variables
Disagree that this is a duplicate. MESOS-2777 identifies an issue with the
handling of coverity reports within the project while MESOS-2880 identifies
around 80 locations where non-POD static initializations within the code
base should be corrected. Should MESOS-2777 have a coverity report
attached listing these defects?
-- Paul
On Fri, May 29, 2015 at 8:18 AM, Bernd Mathiske (JIRA)
wrote:
>
> [
> https://issues.apache.org/jira/browse/MESOS-2780?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
> ]
>
> Bernd Mathiske closed MESOS-2780.
> -
> Resolution: Duplicate
>
> Seems to be the same as MESOS-2777.
>
> > Non-POD static variables
> >
> >
> > Key: MESOS-2780
> > URL: https://issues.apache.org/jira/browse/MESOS-2780
> > Project: Mesos
> > Issue Type: Bug
> >Reporter: Paul Brett
> >
> > We declare const non-POD static variables which should be converted to
> C++11 const expr. These include the following:
> > {noformat}
> > tests/isolator_tests.cpp:1080:const string UNPRIVILEGED_USERNAME =
> "mesos.test.unprivileged.user";
> > tests/mesos.hpp:215:const static std::string TEST_CGROUPS_HIERARCHY =
> "/tmp/mesos_test_cgroup";
> > tests/mesos.hpp:218:const static std::string TEST_CGROUPS_ROOT =
> "mesos_test";
> > tests/zookeeper.cpp:53:const Duration ZooKeeperTest::NO_TIMEOUT =
> Seconds(10);
> > master/contender.cpp:45:const Duration
> MASTER_CONTENDER_ZK_SESSION_TIMEOUT = Seconds(10);
> > master/constants.cpp:33:const Bytes MIN_MEM = Megabytes(32);
> > master/constants.cpp:34:const Duration SLAVE_PING_TIMEOUT = Seconds(15);
> > master/constants.cpp:36:const Duration MIN_SLAVE_REREGISTER_TIMEOUT =
> Minutes(10);
> > master/constants.cpp:41:const Duration WHITELIST_WATCH_INTERVAL =
> Seconds(5);
> > master/constants.cpp:43:const std::string MASTER_INFO_LABEL = "info";
> > master/constants.cpp:44:const Duration ZOOKEEPER_SESSION_TIMEOUT =
> Seconds(10);
> > master/constants.cpp:45:const std::string DEFAULT_AUTHENTICATOR =
> "crammd5";
> > master/constants.cpp:46:const std::string DEFAULT_ALLOCATOR =
> "HierarchicalDRF";
> > master/detector.cpp:56:const Duration MASTER_DETECTOR_ZK_SESSION_TIMEOUT
> = Seconds(10);
> > master/http.cpp:274:const string Master::Http::HEALTH_HELP = HELP(
> > master/http.cpp:289:const static string HOSTS_KEY = "hosts";
> > master/http.cpp:290:const static string LEVEL_KEY = "level";
> > master/http.cpp:291:const static string MONITOR_KEY = "monitor";
> > master/http.cpp:293:const string Master::Http::OBSERVE_HELP = HELP(
> > master/http.cpp:385:const string Master::Http::REDIRECT_HELP = HELP(
> > master/http.cpp:424:const string Master::Http::SLAVES_HELP = HELP(
> > master/http.cpp:687:const TaskStateSummary TaskStateSummary::EMPTY;
> > master/http.cpp:864:const string Master::Http::SHUTDOWN_HELP = HELP(
> > master/http.cpp:877:const string Master::Http::TEARDOWN_HELP = HELP(
> > master/http.cpp:974:const string Master::Http::TASKS_HELP = HELP(
> > zookeeper/group.cpp:43:const Duration GroupProcess::RETRY_INTERVAL =
> Seconds(2);
> > zookeeper/authentication.cpp:11:const ACL_vector
> EVERYONE_READ_CREATOR_ALL = {
> > zookeeper/authentication.cpp:23:const ACL_vector
> EVERYONE_CREATE_AND_READ_CREATOR_ALL = {
> > common/build.cpp:32:const std::string DATE = BUILD_DATE;
> > common/build.cpp:34:const std::string USER = BUILD_USER;
> > common/build.cpp:35:const std::string FLAGS = BUILD_FLAGS;
> > common/build.cpp:36:const std::string JAVA_JVM_LIBRARY =
> BUILD_JAVA_JVM_LIBRARY;
> > common/build.cpp:39:const Option GIT_SHA =
> std::string(BUILD_GIT_SHA);
> > common/build.cpp:41:const Option GIT_SHA = None();
> > common/build.cpp:45:const Option GIT_BRANCH =
> std::string(BUILD_GIT_BRANCH);
> > common/build.cpp:47:const Option GIT_BRANCH = None();
> > common/build.cpp:51:const Option GIT_TAG =
> std::string(BUILD_GIT_TAG);
> > common/build.cpp:53:const Option GIT_TAG = None();
> > slave/monitor.cpp:199:const string
> ResourceMonitorProcess::STATISTICS_HELP = HELP(
> > slave/constants.cpp:29:const Duration EXECUTOR_REGISTRATION_TIMEOUT =
> Minutes(1);
> > slave/constants.cpp:30:const Duration EXECUTOR_SHUTDOWN_GRACE_PERIOD =
> Seconds(5);
> > slave/constants.cpp:31:const Duration EXECUTOR_REREGISTER_TIMEOUT =
> Seconds(2);
> > slave/constants.cpp:32:const Duration EXECUTOR_SIGNAL_ESCALATION_TIMEOUT
> = Seconds(3);
> >
constexpr and non-POD static variables
Despite best efforts to avoid non-POD static variables, we appear to still have over 80 instances (see MESOS-2780). We can, of course, use const inline functions to address these but this will change each constant reference at the calling site to a constant function reference. A more natural approach provided by C++11 is to use constexpr. I have submitted for review (https://reviews.apache.org/r/34782/) an example to address the TC handle. What are the views on whitelisting constexpr? -- Paul Brett
Re: Review Request 33040: Expose qdisc statistics from libnl
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33040/ --- (Updated April 10, 2015, 11:17 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Changes --- Remove the dependency on libnl 3.2.26 Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Expose qdisc statistics from libnl Diffs (updated) - configure.ac 868c041 src/linux/routing/queueing/fq_codel.hpp 4f67ab7 src/linux/routing/queueing/fq_codel.cpp 02ad8df src/linux/routing/queueing/ingress.hpp b323a7f src/linux/routing/queueing/ingress.cpp 47c7337 src/linux/routing/queueing/internal.hpp 7c6c4d3 src/tests/routing_tests.cpp 7cc3b57 Diff: https://reviews.apache.org/r/33040/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 33040: Expose qdisc statistics from libnl
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33040/ --- (Updated April 10, 2015, 7:47 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Changes --- Rebased Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Expose qdisc statistics from libnl Diffs (updated) - configure.ac 868c0413a2e2307ae8ab2211f56874595759b139 src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 src/linux/routing/queueing/fq_codel.hpp 4f67ab7d64afea96a07dfcf36769a9c667749a00 src/linux/routing/queueing/fq_codel.cpp 02ad8df7814c0e549a9ca9aef39777684e6abdcb src/linux/routing/queueing/ingress.hpp b323a7f6daed828327d6d9e9740df81582e0ba2b src/linux/routing/queueing/ingress.cpp 47c73376097d70819defdee31a6d1e446df6b8ba src/linux/routing/queueing/internal.hpp 7c6c4d3d960b9a4bf44dcf482212317522353d69 src/linux/routing/queueing/statistics.hpp PRE-CREATION src/linux/routing/queueing/statistics.cpp PRE-CREATION src/tests/routing_tests.cpp 7cc3b57a3b71544874557d2b1cf88a241b7062ba Diff: https://reviews.apache.org/r/33040/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32906: Add safety check for staged but uncommitted changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32906/ --- Review request for mesos and Ian Downes. Bugs: mesos-598 https://issues.apache.org/jira/browse/mesos-598 Repository: mesos Description --- Add safety check for staged but uncommitted changes Diffs - support/post-reviews.py 2b6c479cf2bda1a93a1f9b10eb0de709aa49dcbe Diff: https://reviews.apache.org/r/32906/diff/ Testing --- vim support/post-reviews.py # edited source $ ./support/post-reviews.py Please commit or stash any changes before using post-reviews! $ git add support/post-reviews.py $ ./support/post-reviews.py Please commit staged changes before using post-reviews! $ git commit . [mesos-598 1867d05] Add safety check for staged but uncommitted changes 1 file changed, 8 insertions(+), 1 deletion(-) $ ./support/post-reviews.py Running 'rbt post' across all of ... Thanks, Paul Brett
Review Request 32903: Eliminate the use of 'echo -n' in EC2 scripts
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32903/ --- Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate the use of 'echo -n' in EC2 scripts Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 Diff: https://reviews.apache.org/r/32903/diff/ Testing --- None Thanks, Paul Brett
Re: Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- (Updated April 6, 2015, 10:05 p.m.) Review request for mesos and Ian Downes. Changes --- Eliminate use of 'echo -n' from the rests and support Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs (updated) - src/tests/port_mapping_tests.cpp 55a5e69 support/timed_tests.sh 5e01af9 Diff: https://reviews.apache.org/r/32898/diff/ Testing --- make check (on linux host only) Thanks, Paul Brett
Re: Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- (Updated April 6, 2015, 7:27 p.m.) Review request for mesos and Ian Downes. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 support/timed_tests.sh 5e01af9411d1d736b12a6996c3ecb0f18468faca Diff: https://reviews.apache.org/r/32898/diff/ Testing (updated) --- make check (on linux host only) Thanks, Paul Brett
Review Request 32898: Eliminate use of 'echo -n'
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32898/ --- Review request for mesos. Bugs: mesos-1424 https://issues.apache.org/jira/browse/mesos-1424 Repository: mesos Description --- Eliminate use of 'echo -n' Diffs - ec2/deploy.amazon64-old/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64-old/root/mesos-ec2/setup 8d4f4b07c4b6bdf92c66db109930b1c285e7c783 ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile 8b50912745977cb71232ba1dfa77f8bb0d60191e ec2/deploy.amazon64/root/mesos-ec2/setup b6b736091d4d5be431c8da29cdb98360a1df2d29 ec2/deploy.centos64/root/mesos-ec2/hypertable/Capfile f85584af5580ecfe5ee5ce3f03fb78408b466ccd ec2/deploy.centos64/root/mesos-ec2/setup f380f7a1c29034b795d4ed5197e52effabb5a175 ec2/deploy.lucid64/root/mesos-ec2/setup 0a74757433521cfe4b37b0e13221375558dce118 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 support/timed_tests.sh 5e01af9411d1d736b12a6996c3ecb0f18468faca Diff: https://reviews.apache.org/r/32898/diff/ Testing --- Thanks, Paul Brett
Review Request 32895: Environment variables are case sensitive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32895/ --- Review request for mesos and Ian Downes. Bugs: mesos-1801 https://issues.apache.org/jira/browse/mesos-1801 Repository: mesos Description --- Environment variables are case sensitive Diffs - src/deploy/mesos-master-env.sh.template 7fe7e536de351c9c8673d9bb8b16b59926e00c2c src/deploy/mesos-slave-env.sh.template 31567d6a47e19385aed56edfc7e67457c8cdde3e Diff: https://reviews.apache.org/r/32895/diff/ Testing --- Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 4, 2015, 12:39 a.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Updated to reflect review comments Bugs: mesos-2591 https://issues.apache.org/jira/browse/mesos-2591 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp e4136095fca55637864f495098189ab3ad8d8fe7 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 3, 2015, 10:36 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: mesos-2591 https://issues.apache.org/jira/browse/mesos-2591 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs - src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp e4136095fca55637864f495098189ab3ad8d8fe7 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/ --- (Updated April 3, 2015, 10:36 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs: mesos-2591 https://issues.apache.org/jira/browse/mesos-2591 Repository: mesos Description --- Refactor statistics helper out of PortMappingIsolatorTest for easier reuse. Diffs - src/slave/containerizer/isolators/network/port_mapping.hpp 466cd82e69665af217d61392b739b9bba16e1e13 src/slave/containerizer/isolators/network/port_mapping.cpp ccdc44f465f204f674b859c429ba1a6ada51cd88 src/tests/port_mapping_tests.cpp 58709b5a0ac58f13985dcc4b71250ec41487ff18 Diff: https://reviews.apache.org/r/32656/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 3, 2015, 10:17 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Correct indentation Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp e4136095fca55637864f495098189ab3ad8d8fe7 src/tests/port_mapping_tests.cpp 55a5e69ed818fd0856179026e3deb889236fea77 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 3, 2015, 9:58 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Rebase Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/slave/containerizer/mesos/containerizer.hpp ae61a0fcd19f2ba808624312401f020121baf5d4 src/slave/containerizer/mesos/containerizer.cpp e4136095fca55637864f495098189ab3ad8d8fe7 src/tests/port_mapping_tests.cpp 58709b5a0ac58f13985dcc4b71250ec41487ff18 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/ --- (Updated April 3, 2015, 9:55 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Changes --- Rebased. Caught waitForFileCreation change dropped from 32653 Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor statistics helper out of PortMappingIsolatorTest for easier reuse. Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.hpp 466cd82e69665af217d61392b739b9bba16e1e13 src/slave/containerizer/isolators/network/port_mapping.cpp ccdc44f465f204f674b859c429ba1a6ada51cd88 src/tests/port_mapping_tests.cpp 58709b5a0ac58f13985dcc4b71250ec41487ff18 Diff: https://reviews.apache.org/r/32656/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 3, 2015, 7:19 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp 58709b5a0ac58f13985dcc4b71250ec41487ff18 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 3, 2015, 6:29 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 2, 2015, 7:06 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 1, 2015, 11:41 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/slave/containerizer/mesos/containerizer.hpp ae61a0f src/slave/containerizer/mesos/containerizer.cpp e413609 src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 1, 2015, 11:07 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Changes --- Wrangling with post-review & reviewboard to get good patches Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32653: Replace busy loop on ready file with a more relaxed loop
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/ --- (Updated April 1, 2015, 11 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Summary (updated) - Replace busy loop on ready file with a more relaxed loop Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Replace busy loop on ready file with a more relaxed loop Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32653/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more relaxed loop
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/ --- (Updated April 1, 2015, 11 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description (updated) --- Replace busy loop on ready file with a more relaxed loop Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32653/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more relaxed loop
> On April 1, 2015, 10:22 p.m., Jie Yu wrote: > > src/tests/port_mapping_tests.cpp, line 403 > > <https://reviews.apache.org/r/32653/diff/3/?file=911841#file911841line403> > > > > 60seconds might be too long. Probably change it to 15 seconds so that > > it's consistent with AWAIT_READY default. 60 seconds is appropriate for some of these networking tests since when we have high packet losses, the tests can take some time to complete (I have often seen times in the 20-30 second range). - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/#review78605 ------- On April 1, 2015, 8:54 p.m., Paul Brett wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32653/ > --- > > (Updated April 1, 2015, 8:54 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > --- > > Replace busy look on ready file with a more relaxed loop > > > Diffs > - > > src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 > > Diff: https://reviews.apache.org/r/32653/diff/ > > > Testing > --- > > make check > > > Thanks, > > Paul Brett > >
Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/ --- (Updated April 1, 2015, 10:33 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor statistics helper out of PortMappingIsolatorTest for easier reuse. Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.hpp 33837b4662959a003c8f38d1e786c6615287a4ff src/slave/containerizer/isolators/network/port_mapping.cpp e691d463515084518c94cdec3fbdf37be4a72977 src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32656/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
> On April 1, 2015, 8:48 p.m., Jie Yu wrote: > > The diff does seem to be correct. Needs one more rev. - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/#review78585 --- On April 1, 2015, 9:30 p.m., Paul Brett wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32654/ > --- > > (Updated April 1, 2015, 9:30 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > --- > > Clean up HostIPNetwork since every use performs the same extract & stringify > operation > > > Diffs > - > > src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 > > Diff: https://reviews.apache.org/r/32654/diff/ > > > Testing > --- > > make check > > > Thanks, > > Paul Brett > >
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 1, 2015, 9:30 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more relaxed loop
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/ --- (Updated April 1, 2015, 8:54 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Replace busy look on ready file with a more relaxed loop Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32653/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 1, 2015, 8:42 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more relaxed loop
> On March 31, 2015, 10:42 p.m., Chi Zhang wrote: > > src/tests/port_mapping_tests.cpp, line 396 > > <https://reviews.apache.org/r/32653/diff/2/?file=910398#file910398line396> > > > > how about at least have this function return a future and change call > > sites to use AWAIT_READY? > > > > When this goes into the library, the tests don't have to be changed > > anymore. > > > > More useful, AWAIT_READY supports timeout and has a default timeout > > time of 10s so you can save the timeout logic here. > > > > It'd be great see this in the library code! Good idea but not required for this change. I will make this a TODO so that we can quickly fix the current issue where a failure can cause the test to hang forever. - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/#review78421 --- On April 1, 2015, 3:43 p.m., Paul Brett wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32653/ > --- > > (Updated April 1, 2015, 3:43 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > --- > > Replace busy look on ready file with a more relaxed loop > > > Diffs > - > > src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 > > Diff: https://reviews.apache.org/r/32653/diff/ > > > Testing > --- > > make check > > > Thanks, > > Paul Brett > >
Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.
> On March 31, 2015, 11:38 p.m., Chi Zhang wrote: > > src/tests/port_mapping_tests.cpp, lines 188-192 > > <https://reviews.apache.org/r/32656/diff/1/?file=910499#file910499line188> > > > > having no output here isn't expected; why delay reporting it? Having no output here is currently an expected behaviour. If mesos-network-helper is called with an invalid pid (for example, before a namespace has been created) then it generates no output and returns with an error status. I believe it would be better to always generate valid JSON output, with a clear error indication if an error occurs. - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/#review78432 --- On April 1, 2015, 3:44 p.m., Paul Brett wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32656/ > --- > > (Updated April 1, 2015, 3:44 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > --- > > Refactor statistics helper out of PortMappingIsolatorTest for easier reuse. > > > Diffs > - > > src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 > > Diff: https://reviews.apache.org/r/32656/diff/ > > > Testing > --- > > make check > > > Thanks, > > Paul Brett > >
Re: Review Request 32664: Add port mapping isolator statistics tests
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32664/ --- (Updated April 1, 2015, 3:45 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Add port mapping isolator statistics tests Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32664/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32660: Report network isolator statistics on a per container basis (MESOS-2332)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32660/ --- (Updated April 1, 2015, 3:45 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Report network isolator statistics on a per container basis (MESOS-2332) Diffs - include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 src/slave/containerizer/isolators/network/port_mapping.hpp 33837b4662959a003c8f38d1e786c6615287a4ff src/slave/containerizer/isolators/network/port_mapping.cpp e691d463515084518c94cdec3fbdf37be4a72977 src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef Diff: https://reviews.apache.org/r/32660/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 1, 2015, 3:44 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32659: Pull the common container definitions out of PortIsolatorMappingTest for reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32659/ --- (Updated April 1, 2015, 3:44 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Pull the common container definitions out of PortIsolatorMappingTest for reuse. Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32659/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/ --- (Updated April 1, 2015, 3:44 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor statistics helper out of PortMappingIsolatorTest for easier reuse. Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32656/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more relaxed loop
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/ --- (Updated April 1, 2015, 3:43 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Replace busy look on ready file with a more relaxed loop Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32653/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32657: Refactor wrappers for JSON encoded stats in preparation for greater reuse
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32657/ --- (Updated April 1, 2015, 3:39 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor wrappers for JSON encoded stats in preparation for greater reuse Diffs - src/slave/containerizer/isolators/network/port_mapping.hpp 33837b4662959a003c8f38d1e786c6615287a4ff src/slave/containerizer/isolators/network/port_mapping.cpp e691d463515084518c94cdec3fbdf37be4a72977 src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32657/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 1, 2015, 1:11 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32664: Add port mapping isolator statistics tests
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32664/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Add port mapping isolator statistics tests Diffs - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32664/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32660: Report network isolator statistics on a per container basis (MESOS-2332)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32660/ --- (Updated April 1, 2015, 12:53 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Report network isolator statistics on a per container basis (MESOS-2332) Diffs (updated) - include/mesos/mesos.proto 3c592d5ab3092ecbeddfaff95e0c1addc3ac58f8 src/slave/containerizer/isolators/network/port_mapping.hpp 33837b4662959a003c8f38d1e786c6615287a4ff src/slave/containerizer/isolators/network/port_mapping.cpp e691d463515084518c94cdec3fbdf37be4a72977 src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef Diff: https://reviews.apache.org/r/32660/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32659: Pull the common container definitions out of PortIsolatorMappingTest for reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32659/ --- (Updated April 1, 2015, 12:52 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Pull the common container definitions out of PortIsolatorMappingTest for reuse. Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32659/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32658: Report errors from mesos-network-helper in JSON output stream so that we can catch and process them in the caller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32658/ --- (Updated April 1, 2015, 12:45 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Report pid & errors from mesos-network-helper in JSON output fields. Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.cpp e691d463515084518c94cdec3fbdf37be4a72977 Diff: https://reviews.apache.org/r/32658/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32657: Refactor wrappers for JSON encoded stats in preparation for greater reuse
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32657/ --- (Updated April 1, 2015, 12:44 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor wrappers for JSON encoded stats in preparation for greater reuse Diffs (updated) - src/slave/containerizer/isolators/network/port_mapping.hpp 33837b4662959a003c8f38d1e786c6615287a4ff src/slave/containerizer/isolators/network/port_mapping.cpp e691d463515084518c94cdec3fbdf37be4a72977 src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32657/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/ --- (Updated April 1, 2015, 12:41 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor statistics helper out of PortMappingIsolatorTest for easier reuse. Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32656/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 1, 2015, 12:41 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more relaxed loop
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/ --- (Updated April 1, 2015, 12:40 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Replace busy look on ready file with a more relaxed loop Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32653/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 1, 2015, 12:39 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated March 31, 2015, 11:52 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Changes --- Fix patch header Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32660: Report network isolator statistics on a per container basis (MESOS-2332)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32660/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Report network isolator statistics on a per container basis (MESOS-2332) Diffs - include/mesos/mesos.proto ec8efaec13f54a56d82411f6cdbdb8ad8b103748 src/slave/containerizer/isolators/network/port_mapping.hpp 4dd066a47d43cb1d52f93294d86309151738743e src/slave/containerizer/isolators/network/port_mapping.cpp 4bf0adeeac1cb6fe59f9c2ca8d5980b1500f5ddd src/slave/flags.hpp dbaf5f532d0bc65a6d16856b8ffcc2c06a98f1fa Diff: https://reviews.apache.org/r/32660/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32659: Pull the common container definitions out of PortIsolatorMappingTest for reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32659/ --- (Updated March 31, 2015, 11:11 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Pull the common container definitions out of PortIsolatorMappingTest for reuse. Diffs - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32659/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32659: Pull the common container definitions out of PortIsolatorMappingTest for reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32659/ --- (Updated March 31, 2015, 11:08 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Pull the common container definitions out of PortIsolatorMappingTest for reuse. Diffs - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32659/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32658: Report errors from mesos-network-helper in JSON output stream so that we can catch and process them in the caller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32658/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Report pid & errors from mesos-network-helper in JSON output fields. Diffs - src/slave/containerizer/isolators/network/port_mapping.cpp e691d46 Diff: https://reviews.apache.org/r/32658/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32659: Pull the common container definitions out of PortIsolatorMappingTest for reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32659/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Pull the common container definitions out of PortIsolatorMappingTest for reuse. Diffs - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32659/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32657: Refactor wrappers for JSON encoded stats in preparation for greater reuse
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32657/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor wrappers for JSON encoded stats in preparation for greater reuse Diffs - src/slave/containerizer/isolators/network/port_mapping.hpp 33837b4 src/slave/containerizer/isolators/network/port_mapping.cpp e691d46 src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32657/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor statistics helper out of PortMappingIsolatorTest for easier reuse. Diffs - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32656/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated March 31, 2015, 5:35 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32655/diff/ Testing (updated) --- make check Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated March 31, 2015, 3:55 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Changes --- Remove dependency on having applied waitForFileCreation first Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- Thanks, Paul Brett
Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated March 31, 2015, 3:52 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Changes --- Remove dependency on having applied waitForFileCreation first Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- Thanks, Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more relaxed loop
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/ --- (Updated March 31, 2015, 3:33 a.m.) Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Changes --- Fix misapplied patch Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Replace busy look on ready file with a more relaxed loop Diffs (updated) - src/tests/port_mapping_tests.cpp f4124c3 Diff: https://reviews.apache.org/r/32653/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class Diffs - src/tests/port_mapping_tests.cpp 8192deac8d9b7ea1896bb62a8b5961ef90326fa4 Diff: https://reviews.apache.org/r/32655/diff/ Testing --- Thanks, Paul Brett
Review Request 32654: Clean up HostIPNetwork since every use performs the same extract & stringify operation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Clean up HostIPNetwork since every use performs the same extract & stringify operation Diffs - src/tests/port_mapping_tests.cpp 8192deac8d9b7ea1896bb62a8b5961ef90326fa4 Diff: https://reviews.apache.org/r/32654/diff/ Testing --- make check Thanks, Paul Brett
Review Request 32653: Replace busy look on ready file with a more relaxed loop
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32653/ --- Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Replace busy look on ready file with a more relaxed loop Diffs - src/tests/port_mapping_tests.cpp 8192deac8d9b7ea1896bb62a8b5961ef90326fa4 Diff: https://reviews.apache.org/r/32653/diff/ Testing --- make check Thanks, Paul Brett
Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics
> On March 18, 2015, 8:58 p.m., Jie Yu wrote: > > Looking at all the port mapping isolator tests here, they are pretty much > > duplicating logics in MesosContainerier (e.g., launchHelper, > > statisticsHelper). It's not good for readability. > > > > It would be cool if we can simply write integration tests and use > > MesosContainerizer and command line executor directly, that can simplify > > the code a lot and make it more readable. Agreed but beyond the scope of this change. I have refactored within port_mapping_tests.cpp to eliminate duplication where I can. - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76943 ------- On March 17, 2015, 6:56 p.m., Paul Brett wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32133/ > --- > > (Updated March 17, 2015, 6:56 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > --- > > Refactor port isolator tests to break out helper functions for testing of > bandwidth limit statistics > > > Diffs > - > > src/tests/port_mapping_tests.cpp 82f98a47fa374fda13b0be76b07ccc03174a7b96 > > Diff: https://reviews.apache.org/r/32133/diff/ > > > Testing > --- > > make check > > > Thanks, > > Paul Brett > >
Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics
> On March 17, 2015, 5:51 p.m., Ian Downes wrote: > > src/tests/port_mapping_tests.cpp, line 196 > > <https://reviews.apache.org/r/32133/diff/1/?file=896458#file896458line196> > > > > Why an empty JSON object rather than a Try? Current implementation of the mesos-network-helper has the helper exiting with return code 1 and no output if the network namespace identified by pid does not exist (possible due to race conditions). I will modify the helper to return a JSON object with an error message to allow easier identification of this situation. With this change, we would return an JSON object under all circumstances although only the error field might be set. > On March 17, 2015, 5:51 p.m., Ian Downes wrote: > > src/tests/port_mapping_tests.cpp, line 676 > > <https://reviews.apache.org/r/32133/diff/1/?file=896458#file896458line676> > > > > Better would be if waitForFileCreation() returned a Future that > > was ready when the path existed. Then you could do AWAIT_READY() and the > > (configurable) timeout would be in the caller waiting on the future, not in > > the helper. > > > > I could imagine this would be generally useful; consider adding this to > > stout? Will abstract out waitForFileCreation as separate ticket - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76744 --- On March 17, 2015, 6:56 p.m., Paul Brett wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32133/ > --- > > (Updated March 17, 2015, 6:56 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > --- > > Refactor port isolator tests to break out helper functions for testing of > bandwidth limit statistics > > > Diffs > - > > src/tests/port_mapping_tests.cpp 82f98a47fa374fda13b0be76b07ccc03174a7b96 > > Diff: https://reviews.apache.org/r/32133/diff/ > > > Testing > --- > > make check > > > Thanks, > > Paul Brett > >
Re: Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics
> On March 17, 2015, 5:51 p.m., Ian Downes wrote: > > src/tests/port_mapping_tests.cpp, lines 266-286 > > <https://reviews.apache.org/r/32133/diff/1/?file=896458#file896458line266> > > > > I don't see the value in pulling these out as separate functions > > compared to the original code? _Perhaps_ a single function that took the > > pXX to find? Functions are broken out here in preparation for use in subsequent patches - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/#review76744 ------- On March 17, 2015, 6:56 p.m., Paul Brett wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32133/ > --- > > (Updated March 17, 2015, 6:56 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang. > > > Bugs: mesos-2332 > https://issues.apache.org/jira/browse/mesos-2332 > > > Repository: mesos > > > Description > --- > > Refactor port isolator tests to break out helper functions for testing of > bandwidth limit statistics > > > Diffs > - > > src/tests/port_mapping_tests.cpp 82f98a47fa374fda13b0be76b07ccc03174a7b96 > > Diff: https://reviews.apache.org/r/32133/diff/ > > > Testing > --- > > make check > > > Thanks, > > Paul Brett > >
Review Request 32133: Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32133/ --- Review request for mesos and Ian Downes. Bugs: mesos-2332 https://issues.apache.org/jira/browse/mesos-2332 Repository: mesos Description --- Refactor port isolator tests to break out helper functions for testing of bandwidth limit statistics Diffs - src/tests/port_mapping_tests.cpp 82f98a47fa374fda13b0be76b07ccc03174a7b96 Diff: https://reviews.apache.org/r/32133/diff/ Testing --- make check Thanks, Paul Brett
