mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Deshi Xiao <xia...@gmail.com>
Subject Re: Review Request 58200: Augmented release guide with GPG goodies and a formatting reminder.
Date Sat, 08 Apr 2017 11:03:12 GMT

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

(Updated 四月 8, 2017, 11:03 a.m.)


Review request for mesos, Alexander Rukletsov and haosdent huang.


Changes
-------

with haosdent helps. update new patch.


Summary (updated)
-----------------

Augmented release guide with GPG goodies and a formatting reminder.


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


Repository: mesos


Description (updated)
-------

Review: https://reviews.apache.org/r/58101

Replaced <br> tags with formatting in release guide for consistency.

Review: https://reviews.apache.org/r/58103

Used operator<< for `CheckStatusInfo` instead of manual printing.

Review: https://reviews.apache.org/r/58108

Fixed a regression hiding previously exposed master and agent flags.

In f441eb9 we in a number of places changed  how 'Flag's were added to
'Flags' by moving from ad-hoc invocations of 'FlagsBase::add' on
particular instances to proper 'Flags' member variables. This was needed
to ensure 'Flags' instances could always safely be copied. For that we
introduced local derived 'Flags' classes to support localized parsing
needs. At the same time, this implementation strategy led to these these
local variables not being accessible through instances of the original
class anymore (this was inevitable when making 'Flags' classes properly
copyable), which e.g., causes a regression in the flags displayed in a
master's '/flags' endpoint.

This commit moves the flags into the respective base class removing the
local classes so that all passed flags are exposed to users.

Review: https://reviews.apache.org/r/57994/

Updated the container image doc about the support of capabilities.

Review: https://reviews.apache.org/r/58117

Updated the container image doc about Docker networking support.

Review: https://reviews.apache.org/r/58118

Fixed a link in the container image doc.


Fixed a formatting issue in executor.proto.


Added Armand Grillet to the contributors list.


Documentation updates to reflect multi-role framework support.

This also updates the roles documentation to reflect the vision
of roles representing the resource consumer.

Review: https://reviews.apache.org/r/58055

Added MULTI_ROLE support to the upgrades documentation.

Review: https://reviews.apache.org/r/58061

Added the design doc for resource provider and CSI.


Removed an old comment from 'v1/mesos.proto'.

This patch removes an old comment in 'include/v1/mesos.proto'
regarding the `DiskInfo.persistence.principal` field. Commit
73790d3c141f91cc3a8f546091ac62594995b788 removed this comment
from 'include/mesos.proto', but neglected to update the v1
proto definitions.

Review: https://reviews.apache.org/r/58150/

Avoid various redundant copies.

Spotted using the "performance-unnecessary-copy-initialization"
clang-tidy check.

Review: https://reviews.apache.org/r/58040

Don't mark a value parameter `const`.

This has no effect on the function's signature.

Spotted using the "readability-avoid-const-params-in-decls" clang-tidy
check.

Review: https://reviews.apache.org/r/58041

Remove unused include in libprocess.

Review: https://reviews.apache.org/r/58042

Cleaned up usage of namespace-qualified identifiers.

Review: https://reviews.apache.org/r/58043

Removed unused "using" statements.

Review: https://reviews.apache.org/r/58044

Adjust `using` style in a few places.

When an identifier is used many times in an implementation file, we
typically add a `using` statement for that identifier.

Review: https://reviews.apache.org/r/58084

Fixed whitespace.


Fixed 'CombinedAuthenticator' to avoid an extra copy.

This patch adds a missing call to `std::move` in the constructor
for `CombinedAuthenticatorProcess` to avoid an unnecessary copy.

Review: https://reviews.apache.org/r/58173/

Fixed the signatures of some test helpers.

This patch fixes some test helpers for
'CombinedAuthenticatorTest.MultipleAuthenticators' to
accept their parameters as const ref.

Review: https://reviews.apache.org/r/58174/

Fixed an iterator bug in 'CombinedAuthenticator::authenticate()'.

This patch updates the `loop()` logic within
`CombinedAuthenticatorProcess::authenticate()` to fix a bug
in which an iterator is incremented past the end of its vector.

Review: https://reviews.apache.org/r/58175/

Fixed some consistency issues in the type utils.


Added resource provider API protobuf template.

Resource provider API will be Event/Call based, similar to the
scheduler or executor API. Resource providers will use this API to
interact with the master, sending Calls to the master and receiving
Event from the master.

Review: https://reviews.apache.org/r/58133

Linked task group doc from home.md.

Review: https://reviews.apache.org/r/58188

Windows: Stout: Rewrote job object wrappers.

`os::create_job` now returns a `Try<SharedHandle>` instead of a raw
`HANDLE`, forcing ownership of the job object handle onto the caller
of the function. `create_job` requires a `std::string name` for the
job object, which is mapped from a PID using `os::name_job`.

The assignment of a process to the job object is now done via
`Try<Nothing> os::assign_job(SharedHandle, pid_t)`.

The equivalent of killing a process tree with job object semantics
is simply to terminate the job object. This is done via
`os::kill_job(SharedHandle)`.

Review: https://reviews.apache.org/r/56364/

Windows: Stout: Adapted `os::killtree` to terminate job objects.

On Windows, a "process tree" is in fact a job object. Thus, the
implementation of `os::killtree` on Windows is an adapter which
terminates the job object corresponding to the process group
represented by the given PID.

Review: https://reviews.apache.org/r/56367/

Windows: Added `JobObjectManager` global actor.

This commit adds a Windows-specific actor for managing job objects.

A subprocess launched with the `ParentHook::CREATE_JOB()` is created
within the context of a named Windows job object. The `JobObjectManager`
takes ownership of the handle to the job object.

It is necessary to tie the lifetime of the job object to the actor by
ownership of the open handle so that the job object can be queried for
usage information even after the processes that were running within the
job object have ended. These semantics were not changed; previously the
same was achieved by leaking the handle and tying it to the lifetime of
the actual Mesos agent process, and implicitly depending on the
operating system to close the open handle at the death of the process.

We ensure the proper death of the job object process group by defering a
call to `cleanup()` to the process reaper for the given PID. This
function uses the Windows system call to terminate the job object via
`os::kill_job()`.

Review: https://reviews.apache.org/r/57973/

Windows: Combined Posix/Windows-Launcher into SubprocessLauncher.

This commit renames the `PosixLauncher` into the SubprocessLauncher`
and deletes the trivially derived class `WindowsLauncher`.  With the
improved job object support in stout/libprocess, the same launcher
is now suitable for both POSIX systems and Windows.  Thus, the previous
name became a misnomer (PosixLauncher).

Review: https://reviews.apache.org/r/57974/

Windows: Changed command executor to use Subprocess.

By encapsulating the job object logic inside a (Windows-only) libprocess
actor, we're able to reuse `Subprocess` for launching tasks on Windows.
This allows us to remove the entirety of `launchTaskWindows` and instead
reuse `launchTaskPosix`, which just uses `subprocess`. This also fixes
the `CommitSuicideOnTaskFailure` test, which is now enabled.

Much of the code in this commit will be moved in a subsequent commit,
as there is no longer any need to separate the Posix/Windows launch
paths in the command executor.

Review: https://reviews.apache.org/r/57975/

Refactored command executor to unify launch paths.

This commit reverses the file split done in e821978.
Since `launchTaskPosix` and `launchTaskWindows` were reconciled
using `Subprocess`, the files were pulled back into just
`src/launcher/executor.cpp` with `launchTaskSubprocess`.

Review: https://reviews.apache.org/r/57976/

Fixed comment in subprocess.


Stout: Added stringify for std::wstring.

Review: https://reviews.apache.org/r/58125/

Windows: Stout: Reimplemented `stringify_args`.

This was an unused function that ended up being the correct place to
implement proper `argv` concatenation and escaping.  It returns a
`std::wstring` for use (speifically) by `::CreateProcessW`.  This brings
us a bit closer to Unicode support within Mesos.

Review: https://reviews.apache.org/r/58126/

Windows: Rewrote subprocess helpers to use wide strings.

This command changes the Windows subprocess helpers to use
`std::wstring` internally and the related wide-string variants of
the Windows API (i.e. `CreateProcessW`) for proper Unicode support.

NOTE: `std::wstring` must to be used instead of `std::u16string`
due to an MSVC bug, see MESOS-7335.

This also fixes the following incorrect string-escaping algorithm:

    std::string command = strings::join(" ", argv);

By replacing it with the rewritten `stringify_args` from
`windows/shell.hpp`.  This resolves problems using when any
arguments that contain "special" characters (whitespace or quotes)
with `subprocess`.

Review: https://reviews.apache.org/r/58127/

Windows: Updated use of `getSystemEnvironment`.

The signature of `process::internal::getSystemEnvironment()` was
changed as part of https://reviews.apache.org/r/58127/
which rewrote the Windows subprocess helpers to use (and return)
UTF-16 strings.  This updates the containerizers' usages of this
function.

Review: https://reviews.apache.org/r/58128/

Fixed HealthyTaskNonShell test on Windows.

Instead of using `TRUE_COMMAND`, we needed to use `add_arguments` for
each argument to avoid smashing the set of arguments into one string.
That is, we cannot execute `"cmd /c exit 0"`, we have to execute `"cmd"`
with arguments `"cmd", "/c", "exit 0"`.

Review: https://reviews.apache.org/r/58161/

Stout: Removed `TRUE_COMMAND`.

This macro was not working because they are semantically different:
a single binary versus a binary with arguments.

Review: https://reviews.apache.org/r/58160/

Fix mesos runs with docker(pid namespace mismatch).

Becuase MESOS HTTP checks doesn't work when mesos runs with
--docker_mesos_image ( pid namespace mismatch ).So let docker
executor run with container add host pid mapping(--pid=host)

Review: https://reviews.apache.org/r/58200


Diffs (updated)
-----

  3rdparty/libprocess/include/process/windows/jobobject.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/windows/subprocess.hpp 1d93b08b035a5eaf677ead3356d0b4be808c39cc

  3rdparty/libprocess/src/process.cpp f6ee24e2db43d63d91222549efee85421bbf9bf3 
  3rdparty/libprocess/src/subprocess.cpp 6dfb939ec151f724d383870a806ca3fc8fb0d931 
  3rdparty/libprocess/src/tests/process_tests.cpp 287eb5577896b9d2c6ea7b49247ab54f66e66de8

  3rdparty/stout/include/stout/gtest.hpp 763fd7b06b240c4a2e1eeb6a283eb192e5a88df9 
  3rdparty/stout/include/stout/os/windows/killtree.hpp 15b2faa7b52feeb067004e9b556f06d1acfb9763

  3rdparty/stout/include/stout/os/windows/shell.hpp fdce93c2146ddec6117577b538dca77c416e0c01

  3rdparty/stout/include/stout/stringify.hpp e9588d8d940046791794100c53469288656a14f0 
  3rdparty/stout/include/stout/windows/os.hpp 0bedb2d63f5b36afdac2b5a29986f38be96b7c16 
  docs/authentication.md 1574db981d5f8ddd7d1f6bef1c2b032823d17297 
  docs/authorization.md b019120b1dde35d2cd5f613ddc3d6db32715c9ca 
  docs/container-image.md fc9e4a6af9d4ec5165185be051bd350b8296948d 
  docs/contributors.yaml e36af18927e17396c1190ec95851f08620c1bd3d 
  docs/design-docs.md ede399306b3c46158438525a01f3b76b0bc08f12 
  docs/home.md 20d136e4b9770db1c873205cb29e90a166a48244 
  docs/persistent-volume.md 410993fb69eb73db6ad17ef361f9b72cba5dc84d 
  docs/quota.md 931542b37ef398c015e33ca004650b7689e03adf 
  docs/release-guide.md b32c988da1d50fa83eae3c30121580e6ac4ee90d 
  docs/reservation.md ace5cef4bccb56126a5338ecde04b91c1a90ce58 
  docs/roles.md 344a0e375210e5e60e225276b715650dd0934d47 
  docs/scheduler-http-api.md 7f808f10a9f71fba44574079238fd6028ff6520c 
  docs/shared-resources.md 1b490e253447a9e263cb5a6bda10f360ea7138ce 
  docs/upgrades.md 249e8444b509d39e1e174ebaa746899b425436b6 
  include/mesos/executor/executor.proto d24f32b2bcc4aba349285452daa20522c8757fcc 
  include/mesos/type_utils.hpp 710c9948ba10c746272b5c023d07522c8ad683f7 
  include/mesos/v1/mesos.hpp b309a2783d1507b066e062f4c0022f165c6ca124 
  include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
  include/mesos/v1/resource_provider/resource_provider.proto PRE-CREATION 
  src/CMakeLists.txt 08f0ef16378ca3cb6a914f53c6696d0b8ebe2f89 
  src/Makefile.am 071656ad7354a802e8292140a7181cb70b68fe9e 
  src/authentication/executor/jwt_secret_generator.cpp 5530a845aa629cd2e9b790d9c958442ff8e7ef89

  src/authentication/http/combined_authenticator.cpp c734e7672e96087fa715501cfc594a0165f5bce7

  src/cli/execute.cpp 5842c478392a11bcd89d61aa88b08c6879b2fcff 
  src/common/build.cpp a87d2cf5463b13d59cf548e7e0edfca600e256b1 
  src/examples/dynamic_reservation_framework.cpp c1650dc978dddf14cf1e22a33752b58b06914ff4

  src/launcher/CMakeLists.txt f63f544f92924b92ef41382c40acabef59a56d8b 
  src/launcher/executor.hpp c7c134aed26d2116295d66100b3d6efaf610736c 
  src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
  src/launcher/posix/executor.hpp 2dd9766aa5b6e0550269ccaa79209d0a483fee76 
  src/launcher/posix/executor.cpp 7c4ef10390e7ecfe63e2fd0c813f91c896fc7a8d 
  src/launcher/windows/executor.hpp 6f02912c477105819b4c27ddf248b7289799eaa0 
  src/launcher/windows/executor.cpp b51fde7376c2083119f342ea599b446944ede000 
  src/master/allocator/mesos/hierarchical.cpp 8d54a8cca1bb478f4437f68c5e14f66a9f9bb9e9 
  src/master/flags.hpp 41a0edfaecf04759f1efa62a9851fbeeb214e84c 
  src/master/flags.cpp 496ca31e1c9bfd9035f7a1d6c715c11648f35cb0 
  src/master/main.cpp 3d2176c2166afd556354e6318c956aacabc82ffe 
  src/master/master.hpp 98364d736522cd3c7b1b406dda42d114cd1808e9 
  src/master/master.cpp ab071f56a66ae66c97a185b2377a9cfe9a5c1ade 
  src/oci/spec.cpp 06fceb4f0441431d756eb45c8aaf9730ef9e248a 
  src/slave/container_loggers/lib_logrotate.cpp ae5d0acdaf64304675ee0f144c8d0c0988ba9112 
  src/slave/containerizer/composing.cpp b5b66b3218a20dcdc7c3174e5ac8fe970ee6462a 
  src/slave/containerizer/docker.cpp ad9ab847cb3093724ef374d036c896b4e7f18b5e 
  src/slave/containerizer/mesos/containerizer.cpp 527c96d7637be863e734102ca431e1488d6397e4

  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 39da15a512de9138aa01e319def906ad9348811d

  src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
d419a9ffc2d2724f876cb05b3eca492b72e2f003 
  src/slave/containerizer/mesos/isolators/posix.hpp 627004663cbd7223a252b875c51454a20e645be6

  src/slave/containerizer/mesos/launch.cpp 395394f04982a7df58e32e9aeebb63756e85b89b 
  src/slave/containerizer/mesos/launcher.hpp 79f6eea0ee8e564e90b36208672df150dbc5d540 
  src/slave/containerizer/mesos/launcher.cpp 5114c130efbfb252dde1e85c081f5174e66f57af 
  src/slave/containerizer/mesos/linux_launcher.cpp 80c9ab2e297c88f1e75e6715b88ef9fa7e38b046

  src/slave/flags.hpp 224fac1d06d5a3914d4d1408e880458ac5be010e 
  src/slave/flags.cpp a7b23b570bb1eba0e4d3f4a10bced8582d87adff 
  src/slave/main.cpp 81d61b14accca7611d84db92663a63d5777edd83 
  src/slave/slave.cpp 1c164c9d2e22868ddec80d26a31504333f483633 
  src/tests/cluster.cpp 7f09a0c8b93e8fa945e7b2c809624b12e61b4198 
  src/tests/container_logger_tests.cpp 4ccb2e4abf5623f5918526bf39ad99fb87869eaa 
  src/tests/containerizer/composing_containerizer_tests.cpp 934824c61a857bdc4d2d8ef790b64a33d14eab9f

  src/tests/containerizer/mesos_containerizer_tests.cpp 9a5cfe49540fcb3364b4c57cb92fcd264e63cf36

  src/tests/default_executor_tests.cpp 6dadd8937eb6809bcb0aca55fab1cb7f17c3262f 
  src/tests/fetcher_cache_tests.cpp 49173af9fa1537b28784ae7543458e5b57e5180f 
  src/tests/health_check_tests.cpp 211f8b8578e811d3f2a229387cc0ce8327ae8cb6 
  src/tests/http_authentication_tests.cpp 36d2b73fa3d0f65aed5e37fe661c93dd46132f82 
  src/tests/http_fault_tolerance_tests.cpp 4040d24e4b910ebda9337f760633e64ede06acf8 
  src/tests/partition_tests.cpp 3de37d272389f4e33cc246ac2654728ddf59016d 
  src/tests/sorter_tests.cpp ec0636beb936d46a253d19322f2157abe95156b6 
  src/v1/mesos.cpp 5605ff22da77724a7947637bc17e12143ee34802 


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

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


Testing
-------

1. Build the image with latest code. Let's name the image with `mesos-build` here.

2. Launch mesos master.

```
$ docker run \
	-it \
	--pid host \
	--net host \
	--privileged \
	-v /var/run/docker.sock:/var/run/docker.sock \
	-v /sys/fs/cgroup:/sys/fs/cgroup \
	mesos-build \
	mesos-master \
	--hostname=127.0.0.1 \
	--ip=127.0.0.1 \
	--port=5050 \
	--work_dir=/tmp/mesos
```

3. Launch mesos agent.

```
$ docker run \
	-it \
	--pid host \
	--net host \
	--privileged \
	-v /var/run/docker.sock:/var/run/docker.sock \
	-v /sys/fs/cgroup:/sys/fs/cgroup \
	mesos-build \
	mesos-agent \
	--hostname=127.0.0.1 \
	--ip=127.0.0.1 \
	--master=127.0.0.1:5050 \
	--systemd_enable_support=false \
	--work_dir=/tmp/mesos \
	--containerizers=docker,mesos \
	--docker_mesos_image=mesos-build
```

4. Launch task with health check.

Define the task with health check.

```
$ cat /tmp/task.json
{
  "name": "test-health-check",
  "task_id": {"value" : "test-health-check"},
  "agent_id": {"value" : ""},
  "resources": [
    {
      "name": "cpus",
      "type": "SCALAR",
      "scalar": {
        "value": 0.1
      },
      "role": "*"
    },
    {
      "name": "mem",
      "type": "SCALAR",
      "scalar": {
        "value": 32
      },
      "role": "*"
    }
  ],
  "command": {
    "value": "sleep 1000"
  },
  "container": {
    "type": "DOCKER",
    "volumes": [],
    "docker": {
      "image": "mesos-build",
      "network": "HOST"
    }
  },
  "health_check": {
    "type": "HTTP",
    "http": {
      "scheme": "http",
      "port": 5050
    },
    "gracePeriodSeconds": 300,
    "intervalSeconds": 60,
    "timeoutSeconds": 20,
    "maxConsecutiveFailures": 3
  }
}
```

Lauch task

```
$ mesos-execute --master=127.0.0.1:5050 --task=/tmp/task.json
```

And verified the healthy status of task is correct.

```
I0407 16:29:57.258509 88767 health_checker.cpp:123] Entered the net namespace of task (pid:
'88727') successfully
I0407 16:29:57.334801 88643 health_checker.cpp:395] Performed HTTP health check for task 'test-health-check'
in 86.311186ms
I0407 16:29:57.334872 88643 health_checker.cpp:319] HTTP health check for task 'test-health-check'
passed
```


Thanks,

Deshi Xiao


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message