date:20170505

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

2017-05-05 Thread Mesos Reviewbot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58720/#review174111
---



Patch looks great!

Reviews applied: [58719, 58720]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On May 5, 2017, 5:02 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 5, 2017, 5:02 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
>   src/cli_new/lib/cli/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/4/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58923: Added new ContainerLaunchInfo task_environment.

2017-05-05 Thread Till Toenshoff



> On May 4, 2017, 2:04 a.m., Adam B wrote:
> > The terminology in this area of the code confuses me. Do we have a glossary 
> > for the different executors, command tasks, etc. somewhere?

That is confusing indeed - will add this particular point to a JIRA touching 
this area. https://issues.apache.org/jira/browse/MESOS-7435


> On May 4, 2017, 2:04 a.m., Adam B wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 166 (patched)
> > 
> >
> > +1, or just drop "already".
> > And tell me which non-custom executors do decide to pass their 
> > environment on to the tasks.
> > 
> > Also, the "Additional environment" confuses me. What is it in addition 
> > to? The new `task_environment`? Comment should explain the relationship.

I did refrain from listing the non-custom executors as this passing-on should 
be true for all of them.


> On May 4, 2017, 2:04 a.m., Adam B wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 180-184 (original), 184-191 (patched)
> > 
> >
> > What's the difference between a `task command` and a `command task`?

It should have been "command task".


> On May 4, 2017, 2:04 a.m., Adam B wrote:
> > include/mesos/slave/containerizer.proto
> > Lines 192 (patched)
> > 
> >
> > What order did you insert this in? I'd say either go numeric and put it 
> > at the end, or put it next to `environment` so you can explain the 
> > difference.

I deducted the current ordering was mostly based on the ordering of things when 
launching a task -- pre_exec_commands come first, pre/executor things come 
next, task specifics last. I put it below `command` as it strongly belongs to 
the task and the environment that may come with `command` (`CommandInfo`) 
itself.


- Till


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58923/#review173859
---


On May 6, 2017, 2:46 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58923/
> ---
> 
> (Updated May 6, 2017, 2:46 a.m.)
> 
> 
> Review request for mesos, Adam B, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-7429
> https://issues.apache.org/jira/browse/MESOS-7429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> c30b1fc659ee9b3cd343899638ced6408d8b51a2 
> 
> 
> Diff: https://reviews.apache.org/r/58923/diff/2/
> 
> 
> Testing
> ---
> 
> make check and functional testing on chain end.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 58923: Added new ContainerLaunchInfo task_environment.

2017-05-05 Thread Till Toenshoff


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58923/
---

(Updated May 6, 2017, 2:46 a.m.)


Review request for mesos, Adam B, Gilbert Song, and Jie Yu.


Changes
---

Addressed comments - thanks for the reviews.


Bugs: MESOS-7429
https://issues.apache.org/jira/browse/MESOS-7429


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
c30b1fc659ee9b3cd343899638ced6408d8b51a2 


Diff: https://reviews.apache.org/r/58923/diff/2/

Changes: https://reviews.apache.org/r/58923/diff/1-2/


Testing
---

make check and functional testing on chain end.


Thanks,

Till Toenshoff

Re: Review Request 58999: Added --secret_resolver flag to agent.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58999/
---

(Updated May 5, 2017, 9:26 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Remove stale pointer copy.


Repository: mesos


Description
---

Updated Containerizer to accept SecretResolver.


Diffs (updated)
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/containerizer/containerizer.hpp 
4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
  src/slave/containerizer/containerizer.cpp 
9024371b6c4228f0903cfeef3bbec736e1a425f8 
  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
  src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
  src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
b47a6b5081a63ac474ac4634701b1a572eb58137 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 


Diff: https://reviews.apache.org/r/58999/diff/3/

Changes: https://reviews.apache.org/r/58999/diff/2-3/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 59001: Added volume secret isolator.

2017-05-05 Thread Kapil Arya



> On May 5, 2017, 2:34 p.m., Chun-Hung Hsiao wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 228 (patched)
> > 
> >
> > This might violate the assumption that there is only one filesystem 
> > isolator. Maybe check that 'filesystem/linux' is enabled in the creator of 
> > 'volume/image' below?
> 
> Chun-Hung Hsiao wrote:
> Please ignore the "Maybe..." question.

Thanks for the catch. I have fixed it now.


> On May 5, 2017, 2:34 p.m., Chun-Hung Hsiao wrote:
> > src/slave/containerizer/mesos/isolators/volume/secret.cpp
> > Lines 286 (patched)
> > 
> >
> > Can we just write the secret to `sandboxSecretPath`?

The idea was to have a tmpfs mount on `sandboxSecretRootDir` and copy secret 
file there so that we won't persist anything on the filesystem. If my 
understanding is correct, we need to copy the file _after_ the container has 
been created (with tmpfs mount), while the secret is downloaded _before_ 
container creation. That's why the additional step.


- Kapil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59001/#review174051
---


On May 5, 2017, 8:53 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59001/
> ---
> 
> (Updated May 5, 2017, 8:53 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added volume secret isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 89cbd3f5a93f4891e8272d3b1136059ab1069d01 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
>   src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
>   src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
>   src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59001/diff/3/
> 
> 
> Testing
> ---
> 
> Added new tests an ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 59001: Added volume secret isolator.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59001/
---

(Updated May 5, 2017, 8:53 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed Chun-Hung's comments.


Bugs: MESOS-7418
https://issues.apache.org/jira/browse/MESOS-7418


Repository: mesos


Description (updated)
---

Added volume secret isolator.


Diffs (updated)
-

  src/CMakeLists.txt 89cbd3f5a93f4891e8272d3b1136059ab1069d01 
  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
  src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/59001/diff/3/

Changes: https://reviews.apache.org/r/59001/diff/2-3/


Testing
---

Added new tests an ran `make check`.


Thanks,

Kapil Arya

Review Request 59038: Fixed flakiness in HierarchicalAllocatorTest.NestedRoleDRF.

2017-05-05 Thread Neil Conway


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59038/
---

Review request for mesos, Anindya Sinha and Michael Park.


Bugs: MESOS-7462
https://issues.apache.org/jira/browse/MESOS-7462


Repository: mesos


Description
---

Fixed flakiness in HierarchicalAllocatorTest.NestedRoleDRF.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
33d5c0ea0182e09b3f3f30d20a33d46c23d81697 


Diff: https://reviews.apache.org/r/59038/diff/1/


Testing
---

`./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
--gtest_repeat=1000 --gtest_break_on_failure .`


Thanks,

Neil Conway

Re: Review Request 59000: Added environment secret isolator.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59000/
---

(Updated May 5, 2017, 8:33 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.


Changes
---

Added cmake changes


Repository: mesos


Description
---

Added environment secret isolator.


Diffs (updated)
-

  src/CMakeLists.txt 89cbd3f5a93f4891e8272d3b1136059ab1069d01 
  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/59000/diff/3/

Changes: https://reviews.apache.org/r/59000/diff/2-3/


Testing
---

Added a new test and ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 58999: Added --secret_resolver flag to agent.

2017-05-05 Thread Kapil Arya



> On May 4, 2017, 6:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?
> 
> Jie Yu wrote:
> Even default parameter is hacky. We should at least have a TODO 
> somewhere. I saw the isolator will simply call secretFetcher.get() assuming 
> it's Some(), while you're passing None() to containerizer in the test.
> 
> I don't like the way we inject `Fetcher` in tests also. That's the reason 
> why RAW pointer is evil. If you have a managed pointer, you probably don't 
> have this issue. Maybe we should use a managed pointer in the interface? The 
> fact that the module create returns a raw pointer is a bad decision in 
> retrospect. It should have been unique_ptr or Owned pointer.
> 
> Jie Yu wrote:
> Any reason this cannot be a Shared pointer? `Shared`?
> 
> just make the method in SecretFetcher const.
> 
> Kapil Arya wrote:
> Are you suggesting `SecretFecther::fetch(...) const;` ?
> 
> Kapil Arya wrote:
> I am not sure how we can use managed pointers with modules because a 
> module might just return a pointer to a static object without ever calling 
> `new`.
> 
> I think we'll need to use raw pointer with a default parameter instead.
> 
> Jie Yu wrote:
> As I said, the fact that module returns a raw pointer is a tech debt. We 
> should force it to return an Owned pointer or unique_ptr in the future.
> 
> Can you follow up with patches to fix that if possible? I think we can 
> change the module interface at anytime?
> 
> Jie Yu wrote:
> For the time being, can you just add a documentation to the SecretFetcher 
> saying that the create must returns a dynamically allocated object whose 
> lifecycle should be delegate to the caller?
> 
> Kapil Arya wrote:
> Yes, I'll add a comment/TODO for now. I already spoke with Benjamin about 
> making appropriate changes to the module interface in 1.4 timeframe. The 
> module API is marked experimental and can change at any time.

Added comments to `mesos/module.hpp` (see 
https://reviews.apache.org/r/58759/diff/5#0.2).


- Kapil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58999/#review173969
---


On May 5, 2017, 6:41 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 5, 2017, 6:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretResolver.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58760: Added default secret resolver module.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58760/
---

(Updated May 5, 2017, 8:32 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Added cmake changes.


Bugs: MESOS-7418
https://issues.apache.org/jira/browse/MESOS-7418


Repository: mesos


Description
---

Added default secret resolver module.


Diffs (updated)
-

  src/CMakeLists.txt 89cbd3f5a93f4891e8272d3b1136059ab1069d01 
  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/secret/resolver.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/58760/diff/5/

Changes: https://reviews.apache.org/r/58760/diff/4-5/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58759: Introduced SecretResolver module interface.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58759/
---

(Updated May 5, 2017, 8:31 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Added comments/TODO about unique_ptr for `ModuleManager::create`.


Bugs: MESOS-7418
https://issues.apache.org/jira/browse/MESOS-7418


Repository: mesos


Description
---

Introduced SecretResolver module interface.


Diffs (updated)
-

  include/mesos/module.hpp c28d01df54906b2b8ebfe7ac9e719a0d1be45614 
  include/mesos/module/secret_resolver.hpp PRE-CREATION 
  include/mesos/secret/resolver.hpp PRE-CREATION 
  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 


Diff: https://reviews.apache.org/r/58759/diff/5/

Changes: https://reviews.apache.org/r/58759/diff/4-5/


Testing
---


Thanks,

Kapil Arya

Review Request 59037: Fixed a typo that detects if the platform is linux.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59037/
---

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Fixed a typo that detects if the platform is linux.


Diffs
-

  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
be45fc59027f176b43b767e9441fd8089ceec7b4 


Diff: https://reviews.apache.org/r/59037/diff/1/


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 58939: Filesystem isolation check for Mesos image provisioner.

2017-05-05 Thread Jie Yu


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58939/#review174106
---




src/slave/containerizer/mesos/containerizer.cpp
Lines 1162-1172 (patched)


Why doing this check after the rootfs has been provisioned? I'd prefer if 
we can check before provisioning the fs.


- Jie Yu


On May 5, 2017, 6:07 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58939/
> ---
> 
> (Updated May 5, 2017, 6:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: mesos-7374
> https://issues.apache.org/jira/browse/mesos-7374
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checked if the 'filesystem/linux' isolator is enabled and the 'linux'
> launcher is used when launching a mesos containerizer with an image
> under Linux. This prevents the executor from messing up with the host
> filesystem. The check is in `MesosContainerizerProcess::prepare()`
> after provisioning and before launching, since provisioning itself
> does not depend on the filesystem isolator.
> 
> Also checked that the 'filesystem/linux' is enabled and the 'linux'
> launcher is used when enabling the 'docker/runtime' isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> 08350e638a0f20746e369cdc78c96126f2e1df3f 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> be45fc59027f176b43b767e9441fd8089ceec7b4 
> 
> 
> Diff: https://reviews.apache.org/r/58939/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a simplified case of mesos-7374.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 59012: Implemented passing the secret fetcher to registry puller.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59012/#review174103
---



Should we use a shared pointer for `SecretResolver` instead of passing a 
`SecretResolver*` around?

- Chun-Hung Hsiao


On May 5, 2017, 11:16 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59012/
> ---
> 
> (Updated May 5, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing the secret fetcher to registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 
> 15c79e9401a821e445fbd32b34503e4fb0014d42 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 09a40a5835454bb7519d11bae4a851337a89b935 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> 62ddb7a332030f3116477408d8b16c19e434c159 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> e1abff19c8877ad07900bb2f44deca1cdda41f58 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7d6c1b93a2c0e265b9344a0fc27f1cf4ed5325f2 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> be45fc59027f176b43b767e9441fd8089ceec7b4 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 82a9be64264ae829773c1e2e8a4360f78641cbf6 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
> 
> 
> Diff: https://reviews.apache.org/r/59012/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Neil Conway


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/#review174102
---


Ship it!




Ship It!

- Neil Conway


On May 5, 2017, 11:27 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58942/
> ---
> 
> (Updated May 5, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CHANGELOG for 1.3.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f41a1e0c8105ab61d9b26e3e6d5e92de41ac5f0d 
> 
> 
> Diff: https://reviews.apache.org/r/58942/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58892: Added C++11 scoped enumeration to style guide.

2017-05-05 Thread Michael Park


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58892/#review174101
---


Ship it!




Ship It!

- Michael Park


On May 2, 2017, 3:11 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58892/
> ---
> 
> (Updated May 2, 2017, 3:11 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We've been using it for new code now.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md ccb81f48e2edc9c1e7c328cc26e44d658b4c43b4 
> 
> 
> Diff: https://reviews.apache.org/r/58892/diff/2/
> 
> 
> Testing
> ---
> 
> Markdown viewer.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Michael Park



> On May 3, 2017, 8:35 p.m., Adam B wrote:
> > CHANGELOG
> > Lines 95 (patched)
> > 
> >
> > I bet we can graduate a few more of these. Maybe not the ones that 
> > still have "Unresolved Critical Issues", but some of the others.

We're going to cut now. We've graduated a few features that people replied to.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/#review173867
---


On May 5, 2017, 4:27 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58942/
> ---
> 
> (Updated May 5, 2017, 4:27 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CHANGELOG for 1.3.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f41a1e0c8105ab61d9b26e3e6d5e92de41ac5f0d 
> 
> 
> Diff: https://reviews.apache.org/r/58942/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Michael Park


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/
---

(Updated May 5, 2017, 4:27 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Graduated task groups and CNI.


Repository: mesos


Description
---

CHANGELOG for 1.3.0 release.


Diffs (updated)
-

  CHANGELOG f41a1e0c8105ab61d9b26e3e6d5e92de41ac5f0d 


Diff: https://reviews.apache.org/r/58942/diff/7/

Changes: https://reviews.apache.org/r/58942/diff/6-7/


Testing
---


Thanks,

Michael Park

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Michael Park


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/
---

(Updated May 5, 2017, 4:20 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Added a few missing JIRAs.


Repository: mesos


Description
---

CHANGELOG for 1.3.0 release.


Diffs (updated)
-

  CHANGELOG f41a1e0c8105ab61d9b26e3e6d5e92de41ac5f0d 


Diff: https://reviews.apache.org/r/58942/diff/6/

Changes: https://reviews.apache.org/r/58942/diff/5-6/


Testing
---


Thanks,

Michael Park

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Michael Park


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/
---

(Updated May 5, 2017, 4:15 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Added MESOS-7453.


Repository: mesos


Description
---

CHANGELOG for 1.3.0 release.


Diffs (updated)
-

  CHANGELOG f41a1e0c8105ab61d9b26e3e6d5e92de41ac5f0d 


Diff: https://reviews.apache.org/r/58942/diff/5/

Changes: https://reviews.apache.org/r/58942/diff/4-5/


Testing
---


Thanks,

Michael Park

Re: Review Request 59012: Implemented passing the secret fetcher to registry puller.

2017-05-05 Thread Gilbert Song


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59012/
---

(Updated May 5, 2017, 4:16 p.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Bugs: MESOS-7088
https://issues.apache.org/jira/browse/MESOS-7088


Repository: mesos


Description
---

Implemented passing the secret fetcher to registry puller.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 
15c79e9401a821e445fbd32b34503e4fb0014d42 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
09a40a5835454bb7519d11bae4a851337a89b935 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
62ddb7a332030f3116477408d8b16c19e434c159 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
e1abff19c8877ad07900bb2f44deca1cdda41f58 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7d6c1b93a2c0e265b9344a0fc27f1cf4ed5325f2 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
be45fc59027f176b43b767e9441fd8089ceec7b4 
  src/slave/containerizer/mesos/provisioner/store.hpp 
82a9be64264ae829773c1e2e8a4360f78641cbf6 
  src/slave/containerizer/mesos/provisioner/store.cpp 
7141d63fcf2dbc3fbf00508c7f92945aab014fb2 


Diff: https://reviews.apache.org/r/59012/diff/3/

Changes: https://reviews.apache.org/r/59012/diff/2-3/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Michael Park


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/
---

(Updated May 5, 2017, 3:42 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Rebased.


Repository: mesos


Description
---

CHANGELOG for 1.3.0 release.


Diffs (updated)
-

  CHANGELOG f41a1e0c8105ab61d9b26e3e6d5e92de41ac5f0d 


Diff: https://reviews.apache.org/r/58942/diff/4/

Changes: https://reviews.apache.org/r/58942/diff/3-4/


Testing
---


Thanks,

Michael Park

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

2017-05-05 Thread Jie Yu


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58725/#review174087
---


Ship it!




Ship It!

- Jie Yu


On May 1, 2017, 5:24 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58725/
> ---
> 
> (Updated May 1, 2017, 5:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7427
> https://issues.apache.org/jira/browse/MESOS-7427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
> to the headers of HTTP requests for fetching manifests from any Docker
> registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
> field strictly and reject the requests if it is not specified.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58725/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on the following registries:
> 1. Local docker private registries with the following version:
>2.0.1, 2.1.1, 2.2.1, 2.3.1, 2.4.1, 2.5.1, 2.6.1
> 2. Amazon ECR repository
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 59017: Added support for docker spec helper 'parseAuthConfig()'.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59017/#review174083
---


Fix it, then Ship it!





include/mesos/docker/spec.hpp
Line 76 (original), 76 (patched)


How about "parsing the docker config from a JSON object" and "from a 
string"?


- Chun-Hung Hsiao


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59017/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for docker spec helper 'parseAuthConfig()'.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
>   src/docker/spec.cpp 6b5588e534215451593cc20011847c3abbab9b17 
> 
> 
> Diff: https://reviews.apache.org/r/59017/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59018/#review174075
---




src/uri/fetchers/docker.cpp
Lines 693 (patched)


Is it a good idea to overload the "password" field in a URI as a docker 
config carrier?


- Chun-Hung Hsiao


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59018/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported Image::Secret in docker URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
> 
> 
> Diff: https://reviews.apache.org/r/59018/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> Manually tested using mesos-execute with task group. Verified that pulling 
> separate private image from different registries using different image 
> secrets works correctly.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52064: Support for multiple versions of docs.

2017-05-05 Thread Tim Anderegg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52064/
---

(Updated May 5, 2017, 9:23 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


Changes
---

Removed file that only had whitespace changes.


Bugs: MESOS-3011
https://issues.apache.org/jira/browse/MESOS-3011


Repository: mesos


Description
---

Support for multiple versions of docs.


Diffs (updated)
-

  site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
  site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
  site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
  site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac 
  site/source/assets/js/versions.js PRE-CREATION 
  site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 


Diff: https://reviews.apache.org/r/52064/diff/5/

Changes: https://reviews.apache.org/r/52064/diff/4-5/


Testing
---

Testing was done manually to verify that the documentation was built for each 
version of Mesos that is supported (some older versions do not have compatible 
documentation).


Thanks,

Tim Anderegg

Re: Review Request 52064: Support for multiple versions of docs.

2017-05-05 Thread Tim Anderegg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52064/
---

(Updated May 5, 2017, 9:20 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


Changes
---

Got rid of build.sh phantom change for real this time.


Bugs: MESOS-3011
https://issues.apache.org/jira/browse/MESOS-3011


Repository: mesos


Description
---

Support for multiple versions of docs.


Diffs (updated)
-

  site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
  site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
  site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
  site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac 
  site/source/assets/js/versions.js PRE-CREATION 
  site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
  site/source/layouts/documentation.erb 
a91f916a5fb7348b2702c272e7a2059bdf628c66 


Diff: https://reviews.apache.org/r/52064/diff/4/

Changes: https://reviews.apache.org/r/52064/diff/3-4/


Testing
---

Testing was done manually to verify that the documentation was built for each 
version of Mesos that is supported (some older versions do not have compatible 
documentation).


Thanks,

Tim Anderegg

Re: Review Request 52064: Support for multiple versions of docs.

2017-05-05 Thread Tim Anderegg



> On Dec. 23, 2016, 5:38 p.m., haosdent huang wrote:
> > site/Rakefile
> > Lines 122 (patched)
> > 
> >
> > Replace all `\/latest\/` looks a bit dangerous. Do you have an example 
> > for this?
> 
> Tim Anderegg wrote:
> I ran the code across the documentation from a number of versions, and 
> determined that the only place the \/latest\/ string occurred was in URL's 
> that should be replaced, e.g. here: 
> https://raw.githubusercontent.com/apache/mesos/0.22.1/docs/home.md

Well, you were right :)  There were a few edge cases where this was too gready, 
so I updated the regex to be more restrictive.


> On Dec. 23, 2016, 5:38 p.m., haosdent huang wrote:
> > site/data/releases.yml
> > Lines 108 (patched)
> > 
> >
> > I think we only need to keep the version `>=0.26.0`. This reduce the 
> > website generation time as well.
> 
> Tim Anderegg wrote:
> I'm happy to remove the older versions if desired.

I went with >=0.22.1, since that is the oldest tag on the apache/mesos github 
repo.  If we should just go with 0.26.0, I can do that instead.


> On Dec. 23, 2016, 5:38 p.m., haosdent huang wrote:
> > site/source/assets/css/main.css
> > Lines 39-41 (patched)
> > 
> >
> > Could not find the differece for this css change. Is it necessary?
> 
> Tim Anderegg wrote:
> This was to better space the  tag used to select the proper 
> version that is part of the breadcrumb.  Without it, the alignement is off.

Actually, you were right this was not needed, sorry!


> On Dec. 23, 2016, 5:38 p.m., haosdent huang wrote:
> > site/source/layouts/gettingstarted.erb
> > Lines 1 (patched)
> > 
> >
> > This layout file is unused, right?
> 
> Tim Anderegg wrote:
> It is unused in recent versions, but was used in older versions.  Since 
> the most recent "layouts" folder is used for rendering all versions of the 
> documentation after the generation process, it must contain all past layouts 
> or else the renderer will produce an error.

Now that we are only doing versions >= 0.22.1, this is no longer needed.


- Tim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52064/#review160091
---


On May 5, 2017, 9:16 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated May 5, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
>   site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/3/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>

Re: Review Request 52064: Support for multiple versions of docs.

2017-05-05 Thread Tim Anderegg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52064/
---

(Updated May 5, 2017, 9:16 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


Changes
---

OK, I believe I have addressed all issues here, I also simplified the code a 
bit to use releases.yml as the source of versions to generate docs for, rather 
than the git tags themselves, and made the "docs:" key optional in that file.  
It only needs to be present if the given version should not have documentation 
generated for it, in which case it is set as "docs: false".  This also removes 
the need for the version check within the Rakefile itself.  Please let me know 
if any additional changes are needed.


Bugs: MESOS-3011
https://issues.apache.org/jira/browse/MESOS-3011


Repository: mesos


Description
---

Support for multiple versions of docs.


Diffs (updated)
-

  site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
  site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
  site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
  site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
  site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac 
  site/source/assets/js/versions.js PRE-CREATION 
  site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
  site/source/layouts/documentation.erb 
a91f916a5fb7348b2702c272e7a2059bdf628c66 


Diff: https://reviews.apache.org/r/52064/diff/3/

Changes: https://reviews.apache.org/r/52064/diff/2-3/


Testing
---

Testing was done manually to verify that the documentation was built for each 
version of Mesos that is supported (some older versions do not have compatible 
documentation).


Thanks,

Tim Anderegg

Re: Review Request 59016: Fixed the comment style issue in docker/spec.hpp.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59016/#review174073
---


Ship it!




Ship It!

- Chun-Hung Hsiao


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59016/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the comment style issue in docker/spec.hpp.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
> 
> 
> Diff: https://reviews.apache.org/r/59016/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59014: Implemented resolving an image secret in registry puller.

2017-05-05 Thread Gilbert Song


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59014/
---

(Updated May 5, 2017, 1:30 p.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Bugs: MESOS-7088
https://issues.apache.org/jira/browse/MESOS-7088


Repository: mesos


Description
---

Implemented resolving an image secret in registry puller.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 


Diff: https://reviews.apache.org/r/59014/diff/2/

Changes: https://reviews.apache.org/r/59014/diff/1-2/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 59012: Implemented passing the secret fetcher to registry puller.

2017-05-05 Thread Gilbert Song


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59012/
---

(Updated May 5, 2017, 1:30 p.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Bugs: MESOS-7088
https://issues.apache.org/jira/browse/MESOS-7088


Repository: mesos


Description
---

Implemented passing the secret fetcher to registry puller.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 
15c79e9401a821e445fbd32b34503e4fb0014d42 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
09a40a5835454bb7519d11bae4a851337a89b935 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
62ddb7a332030f3116477408d8b16c19e434c159 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
e1abff19c8877ad07900bb2f44deca1cdda41f58 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7d6c1b93a2c0e265b9344a0fc27f1cf4ed5325f2 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
be45fc59027f176b43b767e9441fd8089ceec7b4 
  src/slave/containerizer/mesos/provisioner/store.hpp 
82a9be64264ae829773c1e2e8a4360f78641cbf6 
  src/slave/containerizer/mesos/provisioner/store.cpp 
7141d63fcf2dbc3fbf00508c7f92945aab014fb2 


Diff: https://reviews.apache.org/r/59012/diff/2/

Changes: https://reviews.apache.org/r/59012/diff/1-2/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 59010: Updated protobuf comments for Image::Secret.

2017-05-05 Thread Gilbert Song


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59010/
---

(Updated May 5, 2017, 1:30 p.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Repository: mesos


Description
---

Updated protobuf comments for Image::Secret.


Diffs (updated)
-

  include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
  include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 


Diff: https://reviews.apache.org/r/59010/diff/2/

Changes: https://reviews.apache.org/r/59010/diff/1-2/


Testing
---

N/A.


Thanks,

Gilbert Song

Review Request 59029: Added a script to publish JARs to maven snapshot repository.

2017-05-05 Thread Vinod Kone


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59029/
---

Review request for mesos, Anand Mazumdar and Kapil Arya.


Repository: mesos


Description
---

This could be used to publish JARs from nightly builds. Most of the
code is copied from support/tag.sh.


Diffs
-

  support/snapshot.sh PRE-CREATION 


Diff: https://reviews.apache.org/r/59029/diff/1/


Testing
---

Manually uploaded a snapshot build to maven


Thanks,

Vinod Kone

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Michael Park



> On May 3, 2017, 4:48 a.m., Stephan Erb wrote:
> > CHANGELOG
> > Lines 117 (patched)
> > 
> >
> > I was once confused by this "All issues" title in presence of the 
> > "Unresolved Critical Issues". 
> > 
> > Maybe change it to "Newly resolved Issues" or something similar to make 
> > it clearer?
> 
> Adam B wrote:
> Great feedback. We only recently added the "Unresolved Critical Issues" 
> and other sections above.
> I like the sound of "All newly resolved issues:" or "All issues resolved 
> in this release:".
> We have "All" in there because we don't bother to go through the 
> JIRA-generated list to remove the issues already mentioned above; otherwise 
> it could be "Other newly resolved issues:"

I've updated it to `All Resolved Issues`. Let me know if you want it to be 
updated to something else.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/#review173725
---


On May 5, 2017, 12:51 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58942/
> ---
> 
> (Updated May 5, 2017, 12:51 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CHANGELOG for 1.3.0 release.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/executor.hpp 
> cd2f2f03cba8a435f142b31def9f89a6cd61af73 
>   3rdparty/libprocess/include/process/process.hpp 
> 944fcc6449edfd022db4048f70a13aff4a1ff345 
>   3rdparty/libprocess/src/libev.hpp d451931871db650894e4a6e5b0d19ba876f65391 
>   3rdparty/libprocess/src/libev.cpp 07717c4e078d7293c0363f8cbeda8e1c54f4a5a0 
>   3rdparty/libprocess/src/libevent.hpp 
> 2eb9790d82a06d57ed4267ceff066bda00e7ffb4 
>   3rdparty/libprocess/src/libevent.cpp 
> eb3998c2043c7c8be2a6ea1458bb3e1824f6df1e 
>   3rdparty/libprocess/src/process.cpp 
> 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
>   3rdparty/stout/include/Makefile.am ea4341b2dccc3d73b9d347d29486ab4c8ff42217 
>   3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
> 9181429383a991fe2b87701d2bfd0e858ac2537b 
>   3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/uuid.hpp 
> 16830840c3528c9c26f57393cfdb73a09558a369 
>   3rdparty/stout/include/stout/version.hpp 
> 9120e42eedcfb4fc5ee41f08354441bc10914baf 
>   3rdparty/stout/tests/version_tests.cpp 
> bce185ec493054ec81d0764088d04f3e4147303d 
>   CHANGELOG efaeb8a25e8cacf643a14c60e621ad606f92ee8d 
>   CMakeLists.txt 4ba6dd04ad594b0087784d12b785894d9ae257ac 
>   configure.ac fe5b20b53ea007dd8f2e3349e9904f6c4f43a14b 
>   docs/getting-started.md 85a959c7e04866e31ac7e29c31808b363e1830d7 
>   docs/powered-by-mesos.md d2eceb88f72c42b8191a46559bb3a66cd149a536 
>   include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 
>   src/master/constants.hpp 725680b1e5f1fb9ca87ec5130d0eff72309432ae 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
>   src/master/main.cpp 06c735e17801846876177221018a55735e4cf4df 
>   src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> e995ff450d710d847eab827b7021f454f475fcef 
>   src/slave/http.cpp 5beaf91de1ba948658fa7fa299a364432afcf50b 
>   src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
>   src/slave/slave.cpp a37c6c888e7573209aadb07576cfb727fa1ec4ff 
>   src/tests/hierarchical_allocator_tests.cpp 
> 33d5c0ea0182e09b3f3f30d20a33d46c23d81697 
>   src/tests/master_tests.cpp ceee2f4a5d38e0f4200f444769e058d2173de821 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 8c54372b7f6d94f0335561086f2a8cb90373e285 
>   src/tests/persistent_volume_tests.cpp 
> ef0085316d76bd05a86b7930446f6439b2101645 
>   src/tests/reservation_endpoints_tests.cpp 
> 8c195eb7d788fbca8722d66d81c77d399702160a 
> 
> 
> Diff: https://reviews.apache.org/r/58942/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

2017-05-05 Thread Michael Park


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58942/
---

(Updated May 5, 2017, 12:51 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Addressed a couple of comments.


Repository: mesos


Description
---

CHANGELOG for 1.3.0 release.


Diffs (updated)
-

  3rdparty/libprocess/include/process/executor.hpp 
cd2f2f03cba8a435f142b31def9f89a6cd61af73 
  3rdparty/libprocess/include/process/process.hpp 
944fcc6449edfd022db4048f70a13aff4a1ff345 
  3rdparty/libprocess/src/libev.hpp d451931871db650894e4a6e5b0d19ba876f65391 
  3rdparty/libprocess/src/libev.cpp 07717c4e078d7293c0363f8cbeda8e1c54f4a5a0 
  3rdparty/libprocess/src/libevent.hpp 2eb9790d82a06d57ed4267ceff066bda00e7ffb4 
  3rdparty/libprocess/src/libevent.cpp eb3998c2043c7c8be2a6ea1458bb3e1824f6df1e 
  3rdparty/libprocess/src/process.cpp 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
  3rdparty/stout/include/Makefile.am ea4341b2dccc3d73b9d347d29486ab4c8ff42217 
  3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
9181429383a991fe2b87701d2bfd0e858ac2537b 
  3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION 
  3rdparty/stout/include/stout/uuid.hpp 
16830840c3528c9c26f57393cfdb73a09558a369 
  3rdparty/stout/include/stout/version.hpp 
9120e42eedcfb4fc5ee41f08354441bc10914baf 
  3rdparty/stout/tests/version_tests.cpp 
bce185ec493054ec81d0764088d04f3e4147303d 
  CHANGELOG efaeb8a25e8cacf643a14c60e621ad606f92ee8d 
  CMakeLists.txt 4ba6dd04ad594b0087784d12b785894d9ae257ac 
  configure.ac fe5b20b53ea007dd8f2e3349e9904f6c4f43a14b 
  docs/getting-started.md 85a959c7e04866e31ac7e29c31808b363e1830d7 
  docs/powered-by-mesos.md d2eceb88f72c42b8191a46559bb3a66cd149a536 
  include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 
  src/master/constants.hpp 725680b1e5f1fb9ca87ec5130d0eff72309432ae 
  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/master/main.cpp 06c735e17801846876177221018a55735e4cf4df 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
e995ff450d710d847eab827b7021f454f475fcef 
  src/slave/http.cpp 5beaf91de1ba948658fa7fa299a364432afcf50b 
  src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
  src/slave/slave.cpp a37c6c888e7573209aadb07576cfb727fa1ec4ff 
  src/tests/hierarchical_allocator_tests.cpp 
33d5c0ea0182e09b3f3f30d20a33d46c23d81697 
  src/tests/master_tests.cpp ceee2f4a5d38e0f4200f444769e058d2173de821 
  src/tests/persistent_volume_endpoints_tests.cpp 
8c54372b7f6d94f0335561086f2a8cb90373e285 
  src/tests/persistent_volume_tests.cpp 
ef0085316d76bd05a86b7930446f6439b2101645 
  src/tests/reservation_endpoints_tests.cpp 
8c195eb7d788fbca8722d66d81c77d399702160a 


Diff: https://reviews.apache.org/r/58942/diff/3/

Changes: https://reviews.apache.org/r/58942/diff/2-3/


Testing
---


Thanks,

Michael Park

Re: Review Request 59001: Added volume secret isolator.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59001/#review174051
---




src/slave/containerizer/mesos/containerizer.cpp
Lines 228 (patched)


This might violate the assumption that there is only one filesystem 
isolator. Maybe check that 'filesystem/linux' is enabled in the creator of 
'volume/image' below?



src/slave/containerizer/mesos/isolators/volume/secret.cpp
Lines 286 (patched)


Can we just write the secret to `sandboxSecretPath`?


- Chun-Hung Hsiao


On May 5, 2017, 10:44 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59001/
> ---
> 
> (Updated May 5, 2017, 10:44 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
>   src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
>   src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59001/diff/2/
> 
> 
> Testing
> ---
> 
> Added new tests an ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58939: Filesystem isolation check for Mesos image provisioner.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58939/
---

(Updated May 5, 2017, 6:07 p.m.)


Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.


Changes
---

Move the checks for 'docker/runtime' into 
`DockerRuntimeIsolatorProcess::create()`.


Bugs: mesos-7374
https://issues.apache.org/jira/browse/mesos-7374


Repository: mesos


Description
---

Checked if the 'filesystem/linux' isolator is enabled and the 'linux'
launcher is used when launching a mesos containerizer with an image
under Linux. This prevents the executor from messing up with the host
filesystem. The check is in `MesosContainerizerProcess::prepare()`
after provisioning and before launching, since provisioning itself
does not depend on the filesystem isolator.

Also checked that the 'filesystem/linux' is enabled and the 'linux'
launcher is used when enabling the 'docker/runtime' isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
08350e638a0f20746e369cdc78c96126f2e1df3f 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
be45fc59027f176b43b767e9441fd8089ceec7b4 


Diff: https://reviews.apache.org/r/58939/diff/3/

Changes: https://reviews.apache.org/r/58939/diff/2-3/


Testing
---

sudo make check
Manually tested on a simplified case of mesos-7374.


Thanks,

Chun-Hung Hsiao

Re: Review Request 59019: Updated the documentation for '--docker_registry' option.

2017-05-05 Thread Mesos Reviewbot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59019/#review174058
---



Patch looks great!

Reviews applied: [59019]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On May 4, 2017, 11:26 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59019/
> ---
> 
> (Updated May 4, 2017, 11:26 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the documentation for '--docker_registry' option.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 79cada3c9403881bf257d653f721d32e55607a7f 
> 
> 
> Diff: https://reviews.apache.org/r/59019/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

2017-05-05 Thread Armand Grillet


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58720/
---

(Updated May 5, 2017, 5:02 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Constants moved to be accessible from the CLI and not just the tests.


Bugs: MESOS-7283
https://issues.apache.org/jira/browse/MESOS-7283


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs (updated)
-

  src/cli_new/lib/cli/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
  src/cli_new/lib/cli/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


Diff: https://reviews.apache.org/r/58720/diff/4/

Changes: https://reviews.apache.org/r/58720/diff/3-4/


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet

Re: Review Request 58999: Added --secret_resolver flag to agent.

2017-05-05 Thread Kapil Arya



> On May 4, 2017, 6:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?
> 
> Jie Yu wrote:
> Even default parameter is hacky. We should at least have a TODO 
> somewhere. I saw the isolator will simply call secretFetcher.get() assuming 
> it's Some(), while you're passing None() to containerizer in the test.
> 
> I don't like the way we inject `Fetcher` in tests also. That's the reason 
> why RAW pointer is evil. If you have a managed pointer, you probably don't 
> have this issue. Maybe we should use a managed pointer in the interface? The 
> fact that the module create returns a raw pointer is a bad decision in 
> retrospect. It should have been unique_ptr or Owned pointer.
> 
> Jie Yu wrote:
> Any reason this cannot be a Shared pointer? `Shared`?
> 
> just make the method in SecretFetcher const.
> 
> Kapil Arya wrote:
> Are you suggesting `SecretFecther::fetch(...) const;` ?
> 
> Kapil Arya wrote:
> I am not sure how we can use managed pointers with modules because a 
> module might just return a pointer to a static object without ever calling 
> `new`.
> 
> I think we'll need to use raw pointer with a default parameter instead.
> 
> Jie Yu wrote:
> As I said, the fact that module returns a raw pointer is a tech debt. We 
> should force it to return an Owned pointer or unique_ptr in the future.
> 
> Can you follow up with patches to fix that if possible? I think we can 
> change the module interface at anytime?
> 
> Jie Yu wrote:
> For the time being, can you just add a documentation to the SecretFetcher 
> saying that the create must returns a dynamically allocated object whose 
> lifecycle should be delegate to the caller?

Yes, I'll add a comment/TODO for now. I already spoke with Benjamin about 
making appropriate changes to the module interface in 1.4 timeframe. The module 
API is marked experimental and can change at any time.


- Kapil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58999/#review173969
---


On May 5, 2017, 6:41 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 5, 2017, 6:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretResolver.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_resolver flag to agent.

2017-05-05 Thread Jie Yu



> On May 4, 2017, 10:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?
> 
> Jie Yu wrote:
> Even default parameter is hacky. We should at least have a TODO 
> somewhere. I saw the isolator will simply call secretFetcher.get() assuming 
> it's Some(), while you're passing None() to containerizer in the test.
> 
> I don't like the way we inject `Fetcher` in tests also. That's the reason 
> why RAW pointer is evil. If you have a managed pointer, you probably don't 
> have this issue. Maybe we should use a managed pointer in the interface? The 
> fact that the module create returns a raw pointer is a bad decision in 
> retrospect. It should have been unique_ptr or Owned pointer.
> 
> Jie Yu wrote:
> Any reason this cannot be a Shared pointer? `Shared`?
> 
> just make the method in SecretFetcher const.
> 
> Kapil Arya wrote:
> Are you suggesting `SecretFecther::fetch(...) const;` ?
> 
> Kapil Arya wrote:
> I am not sure how we can use managed pointers with modules because a 
> module might just return a pointer to a static object without ever calling 
> `new`.
> 
> I think we'll need to use raw pointer with a default parameter instead.
> 
> Jie Yu wrote:
> As I said, the fact that module returns a raw pointer is a tech debt. We 
> should force it to return an Owned pointer or unique_ptr in the future.
> 
> Can you follow up with patches to fix that if possible? I think we can 
> change the module interface at anytime?

For the time being, can you just add a documentation to the SecretFetcher 
saying that the create must returns a dynamically allocated object whose 
lifecycle should be delegate to the caller?


- Jie


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58999/#review173969
---


On May 5, 2017, 10:41 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 5, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretResolver.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_resolver flag to agent.

2017-05-05 Thread Jie Yu



> On May 4, 2017, 10:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?
> 
> Jie Yu wrote:
> Even default parameter is hacky. We should at least have a TODO 
> somewhere. I saw the isolator will simply call secretFetcher.get() assuming 
> it's Some(), while you're passing None() to containerizer in the test.
> 
> I don't like the way we inject `Fetcher` in tests also. That's the reason 
> why RAW pointer is evil. If you have a managed pointer, you probably don't 
> have this issue. Maybe we should use a managed pointer in the interface? The 
> fact that the module create returns a raw pointer is a bad decision in 
> retrospect. It should have been unique_ptr or Owned pointer.
> 
> Jie Yu wrote:
> Any reason this cannot be a Shared pointer? `Shared`?
> 
> just make the method in SecretFetcher const.
> 
> Kapil Arya wrote:
> Are you suggesting `SecretFecther::fetch(...) const;` ?
> 
> Kapil Arya wrote:
> I am not sure how we can use managed pointers with modules because a 
> module might just return a pointer to a static object without ever calling 
> `new`.
> 
> I think we'll need to use raw pointer with a default parameter instead.

As I said, the fact that module returns a raw pointer is a tech debt. We should 
force it to return an Owned pointer or unique_ptr in the future.

Can you follow up with patches to fix that if possible? I think we can change 
the module interface at anytime?


- Jie


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58999/#review173969
---


On May 5, 2017, 10:41 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 5, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretResolver.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 59011: Fixed docker/appc store 'using' format.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59011/#review174028
---


Ship it!




Ship It!

- Chun-Hung Hsiao


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59011/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed docker/appc store 'using' format.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.hpp e9430df03b49f49d367e19b18991ead77fe5b830 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 09a40a5835454bb7519d11bae4a851337a89b935 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
> 
> 
> Diff: https://reviews.apache.org/r/59011/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59010: Updated protobuf comments for Image::Secret.

2017-05-05 Thread Chun-Hung Hsiao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59010/#review174027
---


Fix it, then Ship it!





include/mesos/mesos.proto
Line 2140 (original), 2140 (patched)


This field has never been...



include/mesos/v1/mesos.proto
Line 2134 (original), 2134 (patched)


Same here.


- Chun-Hung Hsiao


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59010/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf comments for Image::Secret.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
>   include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 
> 
> 
> Diff: https://reviews.apache.org/r/59010/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58974: Fixed flakiness in HierarchicalAllocatorTest.NestedRoleQuota.

2017-05-05 Thread Alexander Rukletsov


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58974/#review174020
---


Ship it!




Ship It!

- Alexander Rukletsov


On May 3, 2017, 8:46 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58974/
> ---
> 
> (Updated May 3, 2017, 8:46 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7457
> https://issues.apache.org/jira/browse/MESOS-7457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flakiness in HierarchicalAllocatorTest.NestedRoleQuota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> ebc4868a6b7e2a04cc202a74a4633969ade40aca 
> 
> 
> Diff: https://reviews.apache.org/r/58974/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> I wasn't able to repro the flakiness on my local box, but the previous test 
> coding does seem wrong, now that `setQuota` doesn't trigger a batch 
> allocation.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59001: Added volume secret isolator.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59001/
---

(Updated May 5, 2017, 6:44 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Rebased.


Bugs: MESOS-7418
https://issues.apache.org/jira/browse/MESOS-7418


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
  src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
  src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/59001/diff/2/

Changes: https://reviews.apache.org/r/59001/diff/1-2/


Testing (updated)
---

Added new tests an ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 59000: Added environment secret isolator.

2017-05-05 Thread Kapil Arya



> On May 4, 2017, 6:32 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/environment_secret.cpp
> > Lines 37-44 (patched)
> > 
> >
> > Why you need this? Can you make sure to avoid such small nits while 
> > copying pasting code? Please remove all unncessary headers, and do a sweep 
> > for other reviews as well!

Sorry for the noise - that was really stupid copy-paste. Fixed now.


- Kapil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59000/#review173970
---


On May 5, 2017, 6:43 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59000/
> ---
> 
> (Updated May 5, 2017, 6:43 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added environment secret isolator.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59000/diff/2/
> 
> 
> Testing
> ---
> 
> Added a new test and ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 59000: Added environment secret isolator.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59000/
---

(Updated May 5, 2017, 6:43 a.m.)


Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.


Changes
---

Rebased.


Repository: mesos


Description (updated)
---

Added environment secret isolator.


Diffs (updated)
-

  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
  src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/59000/diff/2/

Changes: https://reviews.apache.org/r/59000/diff/1-2/


Testing
---

Added a new test and ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 58999: Added --secret_resolver flag to agent.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58999/
---

(Updated May 5, 2017, 6:41 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

SecretFetcher -> SecretResolver; Option -> SecretResolver*


Summary (updated)
-

Added --secret_resolver flag to agent.


Repository: mesos


Description (updated)
---

Updated Containerizer to accept SecretResolver.


Diffs (updated)
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/containerizer/containerizer.hpp 
4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
  src/slave/containerizer/containerizer.cpp 
9024371b6c4228f0903cfeef3bbec736e1a425f8 
  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
  src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
  src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
b47a6b5081a63ac474ac4634701b1a572eb58137 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 


Diff: https://reviews.apache.org/r/58999/diff/2/

Changes: https://reviews.apache.org/r/58999/diff/1-2/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58760: Added default secret resolver module.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58760/
---

(Updated May 5, 2017, 6:38 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Rebased. Added `::create` method.


Summary (updated)
-

Added default secret resolver module.


Bugs: MESOS-7418
https://issues.apache.org/jira/browse/MESOS-7418


Repository: mesos


Description (updated)
---

Added default secret resolver module.


Diffs (updated)
-

  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/secret/resolver.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/58760/diff/4/

Changes: https://reviews.apache.org/r/58760/diff/3-4/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58759: Introduced SecretResolver module interface.

2017-05-05 Thread Kapil Arya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58759/
---

(Updated May 5, 2017, 6:37 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Renamed SecretFetcher to SecretResolver.


Summary (updated)
-

Introduced SecretResolver module interface.


Bugs: MESOS-7418
https://issues.apache.org/jira/browse/MESOS-7418


Repository: mesos


Description (updated)
---

Introduced SecretResolver module interface.


Diffs (updated)
-

  include/mesos/module/secret_resolver.hpp PRE-CREATION 
  include/mesos/secret/resolver.hpp PRE-CREATION 
  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 


Diff: https://reviews.apache.org/r/58759/diff/4/

Changes: https://reviews.apache.org/r/58759/diff/3-4/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58999: Added --secret_fetcher flag for agent.

2017-05-05 Thread Kapil Arya



> On May 4, 2017, 6:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?
> 
> Jie Yu wrote:
> Even default parameter is hacky. We should at least have a TODO 
> somewhere. I saw the isolator will simply call secretFetcher.get() assuming 
> it's Some(), while you're passing None() to containerizer in the test.
> 
> I don't like the way we inject `Fetcher` in tests also. That's the reason 
> why RAW pointer is evil. If you have a managed pointer, you probably don't 
> have this issue. Maybe we should use a managed pointer in the interface? The 
> fact that the module create returns a raw pointer is a bad decision in 
> retrospect. It should have been unique_ptr or Owned pointer.
> 
> Jie Yu wrote:
> Any reason this cannot be a Shared pointer? `Shared`?
> 
> just make the method in SecretFetcher const.
> 
> Kapil Arya wrote:
> Are you suggesting `SecretFecther::fetch(...) const;` ?

I am not sure how we can use managed pointers with modules because a module 
might just return a pointer to a static object without ever calling `new`.

I think we'll need to use raw pointer with a default parameter instead.


- Kapil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58999/#review173969
---


On May 4, 2017, 4:07 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 4, 2017, 4:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretFetcher.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

53 matches

Mail list logo