You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alex R <al...@apache.org> on 2018/05/28 13:05:51 UTC
Re: [6/6] mesos git commit: Added `linux/devices` isolator whitelist tests.
This commit breaks the build on Ubuntu 14.04 with `gcc (Ubuntu
4.8.4-2ubuntu1~14.04.4) 4.8.4` due to what seems to me a compiler bug,
likely this one [1]. Ubuntu 14.04 is officially supported until mid-2019,
hence not sure we can ignore this issue.
James, can you commit a workaround?
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57824
On 25 May 2018 at 23:12, <jp...@apache.org> wrote:
> Added `linux/devices` isolator whitelist tests.
>
> Added test to verify that the `linux/devices` isolator supports
> populating devices that are whitelisted by the `allowed_devices`
> agent flag.
>
> Review: https://reviews.apache.org/r/67145/
>
>
> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ee6c6cfc
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ee6c6cfc
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ee6c6cfc
>
> Branch: refs/heads/master
> Commit: ee6c6cfcbe2b91eaf540afa38bab4521d23b747f
> Parents: 68db3f9
> Author: James Peach <jp...@apache.org>
> Authored: Fri May 25 13:38:14 2018 -0700
> Committer: James Peach <jp...@apache.org>
> Committed: Fri May 25 13:38:14 2018 -0700
>
> ----------------------------------------------------------------------
> src/Makefile.am | 1 +
> src/tests/CMakeLists.txt | 1 +
> .../linux_devices_isolator_tests.cpp | 231 +++++++++++++++++++
> 3 files changed, 233 insertions(+)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/ee6c6cfc/src/Makefile.am
> ----------------------------------------------------------------------
> diff --git a/src/Makefile.am b/src/Makefile.am
> index da0d683..b7184ce 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -2666,6 +2666,7 @@ mesos_tests_SOURCES +=
> \
> tests/containerizer/cgroups_tests.cpp \
> tests/containerizer/cni_isolator_tests.cpp \
> tests/containerizer/docker_volume_isolator_tests.cpp \
> + tests/containerizer/linux_devices_isolator_tests.cpp \
> tests/containerizer/linux_filesystem_isolator_tests.cpp \
> tests/containerizer/fs_tests.cpp \
> tests/containerizer/memory_pressure_tests.cpp \
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/ee6c6cfc/
> src/tests/CMakeLists.txt
> ----------------------------------------------------------------------
> diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
> index 1fef060..b9c906d 100644
> --- a/src/tests/CMakeLists.txt
> +++ b/src/tests/CMakeLists.txt
> @@ -224,6 +224,7 @@ if (LINUX)
> containerizer/docker_volume_isolator_tests.cpp
> containerizer/fs_tests.cpp
> containerizer/linux_capabilities_isolator_tests.cpp
> + containerizer/linux_devices_isolator_tests.cpp
> containerizer/linux_filesystem_isolator_tests.cpp
> containerizer/memory_pressure_tests.cpp
> containerizer/nested_mesos_containerizer_tests.cpp
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/ee6c6cfc/
> src/tests/containerizer/linux_devices_isolator_tests.cpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/containerizer/linux_devices_isolator_tests.cpp
> b/src/tests/containerizer/linux_devices_isolator_tests.cpp
> new file mode 100644
> index 0000000..efaa43b
> --- /dev/null
> +++ b/src/tests/containerizer/linux_devices_isolator_tests.cpp
> @@ -0,0 +1,231 @@
> +// Licensed to the Apache Software Foundation (ASF) under one
> +// or more contributor license agreements. See the NOTICE file
> +// distributed with this work for additional information
> +// regarding copyright ownership. The ASF licenses this file
> +// to you under the Apache License, Version 2.0 (the
> +// "License"); you may not use this file except in compliance
> +// with the License. You may obtain a copy of the License at
> +//
> +// http://www.apache.org/licenses/LICENSE-2.0
> +//
> +// Unless required by applicable law or agreed to in writing, software
> +// distributed under the License is distributed on an "AS IS" BASIS,
> +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> +// See the License for the specific language governing permissions and
> +// limitations under the License.
> +
> +#include <map>
> +#include <ostream>
> +#include <string>
> +
> +#include <process/future.hpp>
> +#include <process/gtest.hpp>
> +#include <process/owned.hpp>
> +
> +#include <stout/gtest.hpp>
> +#include <stout/option.hpp>
> +#include <stout/path.hpp>
> +#include <stout/strings.hpp>
> +
> +#include "common/parse.hpp"
> +
> +#include "tests/cluster.hpp"
> +#include "tests/mesos.hpp"
> +
> +#include "slave/containerizer/mesos/containerizer.hpp"
> +
> +#include "tests/containerizer/docker_archive.hpp"
> +
> +using process::Future;
> +using process::Owned;
> +
> +using std::map;
> +using std::ostream;
> +using std::string;
> +
> +using mesos::internal::slave::Containerizer;
> +using mesos::internal::slave::Fetcher;
> +using mesos::internal::slave::MesosContainerizer;
> +
> +using mesos::slave::ContainerTermination;
> +
> +namespace mesos {
> +namespace internal {
> +namespace tests {
> +
> +struct DevicesTestParam
> +{
> + DevicesTestParam(
> + const string& _containerCheck,
> + const string& _allowedDevices)
> + : containerCheck(_containerCheck),
> + allowedDevices(_allowedDevices) {}
> +
> + const string containerCheck;
> + const string allowedDevices;
> +};
> +
> +
> +ostream& operator<<(ostream& stream, const DevicesTestParam& param)
> +{
> + return stream << param.containerCheck;
> +}
> +
> +
> +class LinuxDevicesIsolatorTest
> + : public MesosTest,
> + public ::testing::WithParamInterface<DevicesTestParam>
> +{
> +public:
> + LinuxDevicesIsolatorTest()
> + : param(GetParam()) {}
> +
> + DevicesTestParam param;
> +};
> +
> +
> +TEST_P(LinuxDevicesIsolatorTest, ROOT_PopulateWhitelistedDevices)
> +{
> + // Verify that all the necessary devices are present on the host.
> + // All reasonable Linux configuration should have these devices.
> + ASSERT_TRUE(os::exists("/dev/kmsg"));
> + ASSERT_TRUE(os::exists("/dev/loop-control"));
> +
> + slave::Flags flags = CreateSlaveFlags();
> +
> + flags.isolation = "linux/devices,filesystem/linux,docker/runtime";
> + flags.docker_registry = path::join(sandbox.get(), "registry");
> + flags.docker_store_dir = path::join(sandbox.get(), "store");
> + flags.image_providers = "docker";
> +
> + flags.allowed_devices =
> + flags::parse<DeviceWhitelist>(param.allowedDevices).get();
> +
> + AWAIT_READY(DockerArchive::create(flags.docker_registry,
> "test_image"));
> +
> + Fetcher fetcher(flags);
> +
> + Try<MesosContainerizer*> create =
> + MesosContainerizer::create(flags, true, &fetcher);
> +
> + ASSERT_SOME(create);
> +
> + Owned<Containerizer> containerizer(create.get());
> +
> + ContainerID containerId;
> + containerId.set_value(id::UUID::random().toString());
> +
> + ExecutorInfo executor = createExecutorInfo(
> + "test_executor",
> + strings::join(";", "ls -l /dev", param.containerCheck));
> +
> + executor.mutable_container()->CopyFrom(createContainerInfo("
> test_image"));
> +
> + string directory = path::join(flags.work_dir, "sandbox");
> + ASSERT_SOME(os::mkdir(directory));
> +
> + // Launch the container check command as the non-root user. All the
> + // check commands are testing for device file access, but root will
> + // always have access.
> + Future<Containerizer::LaunchResult> launch = containerizer->launch(
> + containerId,
> + createContainerConfig(
> + None(), executor, directory, os::getenv("SUDO_USER").get()),
> + map<string, string>(),
> + None());
> +
> + AWAIT_ASSERT_EQ(Containerizer::LaunchResult::SUCCESS, launch);
> +
> + Future<Option<ContainerTermination>> wait = containerizer->wait(
> containerId);
> +
> + AWAIT_READY(wait);
> + ASSERT_SOME(wait.get());
> + ASSERT_TRUE(wait->get().has_status());
> + EXPECT_WEXITSTATUS_EQ(0, wait->get().status());
> +}
> +
> +
> +INSTANTIATE_TEST_CASE_P(
> + DevicesTestParam,
> + LinuxDevicesIsolatorTest,
> + ::testing::Values(
> + // Test that /dev/loop-control is a character device and that
> + // /dev/kmsg doesn't exist. The latter test ensures that we
> + // won't succeed by accidentally running on the host.
> + DevicesTestParam(
> + "test -c /dev/loop-control && test ! -e /dev/kmsg",
> + R"~({
> + "allowed_devices": [
> + {
> + "device": {
> + "path": "/dev/loop-control"
> + },
> + "access": {}
> + }
> + ]
> + })~"),
> + // Test that a device in a subdirectory is populated.
> + DevicesTestParam(
> + "test -r /dev/cpu/0/cpuid",
> + R"~({
> + "allowed_devices": [
> + {
> + "device": {
> + "path": "/dev/cpu/0/cpuid"
> + },
> + "access": {
> + "read": true
> + }
> + }
> + ]
> + })~"),
> + // Test that read-only devices are populated in read-only mode.
> + DevicesTestParam(
> + "test -r /dev/loop-control && test ! -w /dev/loop-control",
> + R"~({
> + "allowed_devices": [
> + {
> + "device": {
> + "path": "/dev/loop-control"
> + },
> + "access": {
> + "read": true
> + }
> + }
> + ]
> + })~"),
> + // Test that write-only devices are populated in write-only mode.
> + DevicesTestParam(
> + "test -w /dev/loop-control && test ! -r /dev/loop-control",
> + R"~({
> + "allowed_devices": [
> + {
> + "device": {
> + "path": "/dev/loop-control"
> + },
> + "access": {
> + "write": true
> + }
> + }
> + ]
> + })~"),
> + // Test that read-write devices are populated in read-write mode.
> + DevicesTestParam(
> + "test -w /dev/loop-control && test -r /dev/loop-control",
> + R"~({
> + "allowed_devices": [
> + {
> + "device": {
> + "path": "/dev/loop-control"
> + },
> + "access": {
> + "read": true,
> + "write": true
> + }
> + }
> + ]
> + })~")));
> +
> +} // namespace tests {
> +} // namespace internal {
> +} // namespace mesos {
>
>
Re: [6/6] mesos git commit: Added `linux/devices` isolator whitelist
tests.
Posted by James Peach <ja...@me.com>.
> On May 28, 2018, at 6:05 AM, Alex R <al...@apache.org> wrote:
>
> This commit breaks the build on Ubuntu 14.04 with `gcc (Ubuntu 4.8.4-2ubuntu1~14.04.4) 4.8.4` due to what seems to me a compiler bug, likely this one [1]. Ubuntu 14.04 is officially supported until mid-2019, hence not sure we can ignore this issue.
>
> James, can you commit a workaround?
Oh it’s the raw strings that break it. I’ll try to come up with something....
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57824
>
>> On 25 May 2018 at 23:12, <jp...@apache.org> wrote:
>> Added `linux/devices` isolator whitelist tests.
>>
>> Added test to verify that the `linux/devices` isolator supports
>> populating devices that are whitelisted by the `allowed_devices`
>> agent flag.
>>
>> Review: https://reviews.apache.org/r/67145/
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ee6c6cfc
>> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ee6c6cfc
>> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ee6c6cfc
>>
>> Branch: refs/heads/master
>> Commit: ee6c6cfcbe2b91eaf540afa38bab4521d23b747f
>> Parents: 68db3f9
>> Author: James Peach <jp...@apache.org>
>> Authored: Fri May 25 13:38:14 2018 -0700
>> Committer: James Peach <jp...@apache.org>
>> Committed: Fri May 25 13:38:14 2018 -0700
>>
>> ----------------------------------------------------------------------
>> src/Makefile.am | 1 +
>> src/tests/CMakeLists.txt | 1 +
>> .../linux_devices_isolator_tests.cpp | 231 +++++++++++++++++++
>> 3 files changed, 233 insertions(+)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/mesos/blob/ee6c6cfc/src/Makefile.am
>> ----------------------------------------------------------------------
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index da0d683..b7184ce 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -2666,6 +2666,7 @@ mesos_tests_SOURCES += \
>> tests/containerizer/cgroups_tests.cpp \
>> tests/containerizer/cni_isolator_tests.cpp \
>> tests/containerizer/docker_volume_isolator_tests.cpp \
>> + tests/containerizer/linux_devices_isolator_tests.cpp \
>> tests/containerizer/linux_filesystem_isolator_tests.cpp \
>> tests/containerizer/fs_tests.cpp \
>> tests/containerizer/memory_pressure_tests.cpp \
>>
>> http://git-wip-us.apache.org/repos/asf/mesos/blob/ee6c6cfc/src/tests/CMakeLists.txt
>> ----------------------------------------------------------------------
>> diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
>> index 1fef060..b9c906d 100644
>> --- a/src/tests/CMakeLists.txt
>> +++ b/src/tests/CMakeLists.txt
>> @@ -224,6 +224,7 @@ if (LINUX)
>> containerizer/docker_volume_isolator_tests.cpp
>> containerizer/fs_tests.cpp
>> containerizer/linux_capabilities_isolator_tests.cpp
>> + containerizer/linux_devices_isolator_tests.cpp
>> containerizer/linux_filesystem_isolator_tests.cpp
>> containerizer/memory_pressure_tests.cpp
>> containerizer/nested_mesos_containerizer_tests.cpp
>>
>> http://git-wip-us.apache.org/repos/asf/mesos/blob/ee6c6cfc/src/tests/containerizer/linux_devices_isolator_tests.cpp
>> ----------------------------------------------------------------------
>> diff --git a/src/tests/containerizer/linux_devices_isolator_tests.cpp b/src/tests/containerizer/linux_devices_isolator_tests.cpp
>> new file mode 100644
>> index 0000000..efaa43b
>> --- /dev/null
>> +++ b/src/tests/containerizer/linux_devices_isolator_tests.cpp
>> @@ -0,0 +1,231 @@
>> +// Licensed to the Apache Software Foundation (ASF) under one
>> +// or more contributor license agreements. See the NOTICE file
>> +// distributed with this work for additional information
>> +// regarding copyright ownership. The ASF licenses this file
>> +// to you under the Apache License, Version 2.0 (the
>> +// "License"); you may not use this file except in compliance
>> +// with the License. You may obtain a copy of the License at
>> +//
>> +// http://www.apache.org/licenses/LICENSE-2.0
>> +//
>> +// Unless required by applicable law or agreed to in writing, software
>> +// distributed under the License is distributed on an "AS IS" BASIS,
>> +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> +// See the License for the specific language governing permissions and
>> +// limitations under the License.
>> +
>> +#include <map>
>> +#include <ostream>
>> +#include <string>
>> +
>> +#include <process/future.hpp>
>> +#include <process/gtest.hpp>
>> +#include <process/owned.hpp>
>> +
>> +#include <stout/gtest.hpp>
>> +#include <stout/option.hpp>
>> +#include <stout/path.hpp>
>> +#include <stout/strings.hpp>
>> +
>> +#include "common/parse.hpp"
>> +
>> +#include "tests/cluster.hpp"
>> +#include "tests/mesos.hpp"
>> +
>> +#include "slave/containerizer/mesos/containerizer.hpp"
>> +
>> +#include "tests/containerizer/docker_archive.hpp"
>> +
>> +using process::Future;
>> +using process::Owned;
>> +
>> +using std::map;
>> +using std::ostream;
>> +using std::string;
>> +
>> +using mesos::internal::slave::Containerizer;
>> +using mesos::internal::slave::Fetcher;
>> +using mesos::internal::slave::MesosContainerizer;
>> +
>> +using mesos::slave::ContainerTermination;
>> +
>> +namespace mesos {
>> +namespace internal {
>> +namespace tests {
>> +
>> +struct DevicesTestParam
>> +{
>> + DevicesTestParam(
>> + const string& _containerCheck,
>> + const string& _allowedDevices)
>> + : containerCheck(_containerCheck),
>> + allowedDevices(_allowedDevices) {}
>> +
>> + const string containerCheck;
>> + const string allowedDevices;
>> +};
>> +
>> +
>> +ostream& operator<<(ostream& stream, const DevicesTestParam& param)
>> +{
>> + return stream << param.containerCheck;
>> +}
>> +
>> +
>> +class LinuxDevicesIsolatorTest
>> + : public MesosTest,
>> + public ::testing::WithParamInterface<DevicesTestParam>
>> +{
>> +public:
>> + LinuxDevicesIsolatorTest()
>> + : param(GetParam()) {}
>> +
>> + DevicesTestParam param;
>> +};
>> +
>> +
>> +TEST_P(LinuxDevicesIsolatorTest, ROOT_PopulateWhitelistedDevices)
>> +{
>> + // Verify that all the necessary devices are present on the host.
>> + // All reasonable Linux configuration should have these devices.
>> + ASSERT_TRUE(os::exists("/dev/kmsg"));
>> + ASSERT_TRUE(os::exists("/dev/loop-control"));
>> +
>> + slave::Flags flags = CreateSlaveFlags();
>> +
>> + flags.isolation = "linux/devices,filesystem/linux,docker/runtime";
>> + flags.docker_registry = path::join(sandbox.get(), "registry");
>> + flags.docker_store_dir = path::join(sandbox.get(), "store");
>> + flags.image_providers = "docker";
>> +
>> + flags.allowed_devices =
>> + flags::parse<DeviceWhitelist>(param.allowedDevices).get();
>> +
>> + AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
>> +
>> + Fetcher fetcher(flags);
>> +
>> + Try<MesosContainerizer*> create =
>> + MesosContainerizer::create(flags, true, &fetcher);
>> +
>> + ASSERT_SOME(create);
>> +
>> + Owned<Containerizer> containerizer(create.get());
>> +
>> + ContainerID containerId;
>> + containerId.set_value(id::UUID::random().toString());
>> +
>> + ExecutorInfo executor = createExecutorInfo(
>> + "test_executor",
>> + strings::join(";", "ls -l /dev", param.containerCheck));
>> +
>> + executor.mutable_container()->CopyFrom(createContainerInfo("test_image"));
>> +
>> + string directory = path::join(flags.work_dir, "sandbox");
>> + ASSERT_SOME(os::mkdir(directory));
>> +
>> + // Launch the container check command as the non-root user. All the
>> + // check commands are testing for device file access, but root will
>> + // always have access.
>> + Future<Containerizer::LaunchResult> launch = containerizer->launch(
>> + containerId,
>> + createContainerConfig(
>> + None(), executor, directory, os::getenv("SUDO_USER").get()),
>> + map<string, string>(),
>> + None());
>> +
>> + AWAIT_ASSERT_EQ(Containerizer::LaunchResult::SUCCESS, launch);
>> +
>> + Future<Option<ContainerTermination>> wait = containerizer->wait(containerId);
>> +
>> + AWAIT_READY(wait);
>> + ASSERT_SOME(wait.get());
>> + ASSERT_TRUE(wait->get().has_status());
>> + EXPECT_WEXITSTATUS_EQ(0, wait->get().status());
>> +}
>> +
>> +
>> +INSTANTIATE_TEST_CASE_P(
>> + DevicesTestParam,
>> + LinuxDevicesIsolatorTest,
>> + ::testing::Values(
>> + // Test that /dev/loop-control is a character device and that
>> + // /dev/kmsg doesn't exist. The latter test ensures that we
>> + // won't succeed by accidentally running on the host.
>> + DevicesTestParam(
>> + "test -c /dev/loop-control && test ! -e /dev/kmsg",
>> + R"~({
>> + "allowed_devices": [
>> + {
>> + "device": {
>> + "path": "/dev/loop-control"
>> + },
>> + "access": {}
>> + }
>> + ]
>> + })~"),
>> + // Test that a device in a subdirectory is populated.
>> + DevicesTestParam(
>> + "test -r /dev/cpu/0/cpuid",
>> + R"~({
>> + "allowed_devices": [
>> + {
>> + "device": {
>> + "path": "/dev/cpu/0/cpuid"
>> + },
>> + "access": {
>> + "read": true
>> + }
>> + }
>> + ]
>> + })~"),
>> + // Test that read-only devices are populated in read-only mode.
>> + DevicesTestParam(
>> + "test -r /dev/loop-control && test ! -w /dev/loop-control",
>> + R"~({
>> + "allowed_devices": [
>> + {
>> + "device": {
>> + "path": "/dev/loop-control"
>> + },
>> + "access": {
>> + "read": true
>> + }
>> + }
>> + ]
>> + })~"),
>> + // Test that write-only devices are populated in write-only mode.
>> + DevicesTestParam(
>> + "test -w /dev/loop-control && test ! -r /dev/loop-control",
>> + R"~({
>> + "allowed_devices": [
>> + {
>> + "device": {
>> + "path": "/dev/loop-control"
>> + },
>> + "access": {
>> + "write": true
>> + }
>> + }
>> + ]
>> + })~"),
>> + // Test that read-write devices are populated in read-write mode.
>> + DevicesTestParam(
>> + "test -w /dev/loop-control && test -r /dev/loop-control",
>> + R"~({
>> + "allowed_devices": [
>> + {
>> + "device": {
>> + "path": "/dev/loop-control"
>> + },
>> + "access": {
>> + "read": true,
>> + "write": true
>> + }
>> + }
>> + ]
>> + })~")));
>> +
>> +} // namespace tests {
>> +} // namespace internal {
>> +} // namespace mesos {
>>
>