You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by George Sirois <ge...@gmail.com> on 2016/03/28 20:21:47 UTC
Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/
-----------------------------------------------------------
Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
Repository: aurora
Description
-------
In instances where the root filesystem is read-only, it is desirable to
have the executor/runner extract themselves into the sandbox.
Diffs
-----
RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
Diff: https://reviews.apache.org/r/45396/diff/
Testing
-------
Thanks,
George Sirois
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125728
-----------------------------------------------------------
Master (0950095) is green with this patch.
./build-support/jenkins/build.sh
However, it appears that it might lack test coverage.
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by George Sirois <ge...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125707
-----------------------------------------------------------
@ReviewBot retry
- George Sirois
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Steve Niemitz <sn...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125722
-----------------------------------------------------------
RELEASE-NOTES.md (line 14)
<https://reviews.apache.org/r/45396/#comment188580>
In the future I think we should revisit this and make this the default behavior. The only reason I think it shouldn't be enabled by default now is because the mesos.native lib inside the pex is massive, and duplicating it into every sandbox is 1) slow, and 2) a waste of space.
However, once my patch lands, the lib goes down to ~8 MB, which I think is worth duplicating, especially to get rid of any weird race conditions with multiple executors launching.
- Steve Niemitz
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Steve Niemitz <sn...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125726
-----------------------------------------------------------
Ship it!
Ship It!
- Steve Niemitz
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125887
-----------------------------------------------------------
Ship it!
LGTM, thanks for adding the tests!
If you don't mind fixing the two style nits below I can commit this.
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java (line 112)
<https://reviews.apache.org/r/45396/#comment188786>
nit: +blank line before
src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java (line 71)
<https://reviews.apache.org/r/45396/#comment188788>
same style nit here, blank line after method signature that's broken up over multiple lines.
- Joshua Cohen
On March 29, 2016, 5:49 a.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 29, 2016, 5:49 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db
> docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125827
-----------------------------------------------------------
Ship it!
Master (f28f41a) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 29, 2016, 5:49 a.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 29, 2016, 5:49 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db
> docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125903
-----------------------------------------------------------
Master (f28f41a) is red with this patch.
./build-support/jenkins/build.sh
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1)[0m
[1m [0m
[1m assert hct._total_latency == 0[0m
[1m assert hct.metrics.sample()['total_latency_secs'] == 0[0m
[1m [0m
[1m # start the health check (during health check it is still 0)[0m
[1m epsilon = 0.001[0m
[1m self._clock.tick(1.0 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=0.5)[0m
[1m assert hct._total_latency == 0[0m
[1m assert hct.metrics.sample()['total_latency_secs'] == 0[0m
[1m assert hct.metrics.sample()['checks'] == 0[0m
[1m [0m
[1m # finish the health check[0m
[1m self._clock.tick(0.5 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1) # interval_secs[0m
[1m> assert hct._total_latency == 0.5[0m
[1m[31mE AssertionError: assert 0.5009999999999999 == 0.5[0m
[1m[31mE + where 0.5009999999999999 = <apache.aurora.executor.common.health_checker.HealthChecker object at 0x7f7ee76057d0>._total_latency[0m
src/test/python/apache/aurora/executor/common/test_health_checker.py:174: AssertionError
-------------- Captured stderr call --------------
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7ee7605390>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7ee7605390>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7ee7605390>] Time now: 1.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7ee7605390>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7ee7605390>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7ee7605390>] Time now: 1.501
[<twitter.common.testing.clock.ThreadedClock object at 0x7f7ee7605390>] Time now: 1.502
generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
[1m[31m 1 failed, 663 passed, 5 skipped, 1 warnings in 201.06 seconds [0m
FAILURE
16:37:05 04:20 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 29, 2016, 4:18 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 29, 2016, 4:18 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db
> docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by George Sirois <ge...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/
-----------------------------------------------------------
(Updated March 29, 2016, 4:18 p.m.)
Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
Changes
-------
Fixes for style nits.
Repository: aurora
Description
-------
In instances where the root filesystem is read-only, it is desirable to
have the executor/runner extract themselves into the sandbox.
Diffs (updated)
-----
RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db
docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45396/diff/
Testing
-------
Thanks,
George Sirois
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by George Sirois <ge...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/
-----------------------------------------------------------
(Updated March 29, 2016, 5:49 a.m.)
Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
Repository: aurora
Description
-------
In instances where the root filesystem is read-only, it is desirable to
have the executor/runner extract themselves into the sandbox.
Diffs (updated)
-----
RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db
docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45396/diff/
Testing
-------
Thanks,
George Sirois
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Joshua Cohen <jc...@apache.org>.
> On March 29, 2016, 12:03 a.m., Joshua Cohen wrote:
> > Are there any concerns about any other unintended side effect of setting $HOME to the sandbox? I suppose it's opt-in for now, so that allays most fears.
>
> George Sirois wrote:
> I couldn't think of any (nor did I discover any in testing, both with and without Docker), but making it opt-in should allow us to gain experience with running it in production environments without impacting existing deployments. The reason I used HOME (as opposed to PEX_ROOT) is that the executor strips PEX_ROOT before forking the runner, which has the same issue. We could add some plumbing to get around that, but it seems like if you're running with a read-only root FS, you'd want HOME to be a writable location anyway.
Yeah, I'm not coming up with anything either other than potentially causing tasks to exceed their disk space allotment due to ~/.pex/install now being counted against them. That's arguably the right thing to do though, operators should just be cognizant of that when enabling this (or if/when we make it the default).
- Joshua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125779
-----------------------------------------------------------
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by George Sirois <ge...@gmail.com>.
> On March 29, 2016, 12:03 a.m., Joshua Cohen wrote:
> > Are there any concerns about any other unintended side effect of setting $HOME to the sandbox? I suppose it's opt-in for now, so that allays most fears.
I couldn't think of any (nor did I discover any in testing, both with and without Docker), but making it opt-in should allow us to gain experience with running it in production environments without impacting existing deployments. The reason I used HOME (as opposed to PEX_ROOT) is that the executor strips PEX_ROOT before forking the runner, which has the same issue. We could add some plumbing to get around that, but it seems like if you're running with a read-only root FS, you'd want HOME to be a writable location anyway.
- George
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125779
-----------------------------------------------------------
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by George Sirois <ge...@gmail.com>.
> On March 29, 2016, 12:03 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java, line 112
> > <https://reviews.apache.org/r/45396/diff/1/?file=1316952#file1316952line112>
> >
> > Given that this is a static method, it should be pretty easy to test, would you mind adding a test case for this behavior (probably requires refactoring the method to take the arg values as parameters, rather than referencing them directly).
Sure, will do.
- George
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125779
-----------------------------------------------------------
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125779
-----------------------------------------------------------
Are there any concerns about any other unintended side effect of setting $HOME to the sandbox? I suppose it's opt-in for now, so that allays most fears.
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java (line 112)
<https://reviews.apache.org/r/45396/#comment188641>
Given that this is a static method, it should be pretty easy to test, would you mind adding a test case for this behavior (probably requires refactoring the method to take the arg values as parameters, rather than referencing them directly).
- Joshua Cohen
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>
Re: Review Request 45396: Adds the ability to set HOME to the sandbox
before running the executor.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45396/#review125706
-----------------------------------------------------------
Master (0950095) is red with this patch.
./build-support/jenkins/build.sh
[1m self._clock.tick(0.5 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1) # interval_secs[0m
[1m assert hct._total_latency == 0.5[0m
[1m assert hct.metrics.sample()['total_latency_secs'] == 0.5[0m
[1m assert hct.metrics.sample()['checks'] == 1[0m
[1m [0m
[1m # tick again[0m
[1m self._clock.tick(1.0 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.tick(0.5 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1) # interval_secs[0m
[1m> assert hct._total_latency == 1.0[0m
[1m[31mE AssertionError: assert 1.001 == 1.0[0m
[1m[31mE + where 1.001 = <apache.aurora.executor.common.health_checker.HealthChecker object at 0x7f20f710edd0>._total_latency[0m
src/test/python/apache/aurora/executor/common/test_health_checker.py:184: AssertionError
-------------- Captured stderr call --------------
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 1.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 1.501
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 1.502
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 1.502
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 2.502
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 2.503
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 2.503
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 3.003
[<twitter.common.testing.clock.ThreadedClock object at 0x7f20f710ed10>] Time now: 3.004
generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
[1m[31m 1 failed, 663 passed, 5 skipped, 1 warnings in 285.62 seconds [0m
FAILURE
18:47:48 06:36 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 28, 2016, 6:21 p.m., George Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> -----------------------------------------------------------
>
> (Updated March 28, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
> docs/deploying-aurora-scheduler.md 03bfdbab927c924486b04c42df2ad236c0f414a0
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
>
> Diff: https://reviews.apache.org/r/45396/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> George Sirois
>
>