You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Neil Conway <ne...@gmail.com> on 2016/09/22 17:13:57 UTC
Re: mesos git commit: Added `DEFAULT_ROLE` constant to persistent
volume tests.
I'm not sure this is a good idea: the "default role" is actually "*".
That is also the default value for the "role" fields in the protobufs.
Perhaps we should name this new constant something like
DEFAULT_TEST_ROLE?
I wonder also if we should keep the definition local to
"persistent_volume_tests.cpp", unless there are imminent plans to use
it elsewhere.
Neil
On Thu, Sep 22, 2016 at 6:37 PM, <mp...@apache.org> wrote:
> Repository: mesos
> Updated Branches:
> refs/heads/master 4df496aaf -> c2b595e1c
>
>
> Added `DEFAULT_ROLE` constant to persistent volume tests.
>
> Review: https://reviews.apache.org/r/41613/
>
>
> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c2b595e1
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c2b595e1
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c2b595e1
>
> Branch: refs/heads/master
> Commit: c2b595e1c59bb23e3f87545a7e22a76ad232ae9f
> Parents: 4df496a
> Author: Greg Mann <gr...@mesosphere.io>
> Authored: Thu Sep 22 15:47:31 2016 +0200
> Committer: Michael Park <mp...@apache.org>
> Committed: Thu Sep 22 17:28:32 2016 +0200
>
> ----------------------------------------------------------------------
> src/tests/mesos.hpp | 1 +
> src/tests/persistent_volume_tests.cpp | 51 +++++++++++++++++-------------
> 2 files changed, 30 insertions(+), 22 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/src/tests/mesos.hpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
> index 7095101..3cd63bd 100644
> --- a/src/tests/mesos.hpp
> +++ b/src/tests/mesos.hpp
> @@ -93,6 +93,7 @@ namespace tests {
>
> constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] = "test-readonly-realm";
> constexpr char READWRITE_HTTP_AUTHENTICATION_REALM[] = "test-readwrite-realm";
> +constexpr char DEFAULT_ROLE[] = "default-role";
>
> // Forward declarations.
> class MockExecutor;
>
> http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/src/tests/persistent_volume_tests.cpp
> ----------------------------------------------------------------------
> diff --git a/src/tests/persistent_volume_tests.cpp b/src/tests/persistent_volume_tests.cpp
> index c38d848..a726d78 100644
> --- a/src/tests/persistent_volume_tests.cpp
> +++ b/src/tests/persistent_volume_tests.cpp
> @@ -166,7 +166,7 @@ protected:
> case NONE: {
> diskResource = createDiskResource(
> stringify(mb.megabytes()),
> - "role1",
> + DEFAULT_ROLE,
> None(),
> None());
>
> @@ -175,7 +175,7 @@ protected:
> case PATH: {
> diskResource = createDiskResource(
> stringify(mb.megabytes()),
> - "role1",
> + DEFAULT_ROLE,
> None(),
> None(),
> createDiskSourcePath(path::join(diskPath, "disk" + stringify(id))));
> @@ -185,7 +185,7 @@ protected:
> case MOUNT: {
> diskResource = createDiskResource(
> stringify(mb.megabytes()),
> - "role1",
> + DEFAULT_ROLE,
> None(),
> None(),
> createDiskSourceMount(
> @@ -254,7 +254,7 @@ TEST_P(PersistentVolumeTest, CreateAndDestroyPersistentVolumes)
> Clock::pause();
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> // Create a master.
> master::Flags masterFlags = CreateMasterFlags();
> @@ -429,7 +429,7 @@ TEST_P(PersistentVolumeTest, ResourcesCheckpointing)
> ASSERT_SOME(slave);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> MockScheduler sched;
> MesosSchedulerDriver driver(
> @@ -495,7 +495,7 @@ TEST_P(PersistentVolumeTest, PreparePersistentVolume)
> ASSERT_SOME(slave);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> MockScheduler sched;
> MesosSchedulerDriver driver(
> @@ -564,7 +564,7 @@ TEST_P(PersistentVolumeTest, MasterFailover)
> ASSERT_SOME(slave);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> MockScheduler sched;
> TestingMesosSchedulerDriver driver(&sched, &detector, frameworkInfo);
> @@ -659,7 +659,7 @@ TEST_P(PersistentVolumeTest, IncompatibleCheckpointedResources)
> spawn(slave1);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> MockScheduler sched;
> MesosSchedulerDriver driver(
> @@ -746,7 +746,7 @@ TEST_P(PersistentVolumeTest, AccessPersistentVolume)
> ASSERT_SOME(slave);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> MockScheduler sched;
> MesosSchedulerDriver driver(
> @@ -899,14 +899,15 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeMultipleTasks)
> ASSERT_SOME(master);
>
> slave::Flags slaveFlags = CreateSlaveFlags();
> - slaveFlags.resources = "cpus:2;mem:1024;disk(role1):1024";
> + slaveFlags.resources =
> + "cpus:2;mem:1024;disk(" + string(DEFAULT_ROLE) + "):1024";
>
> Owned<MasterDetector> detector = master.get()->createDetector();
> Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
> ASSERT_SOME(slave);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> MockScheduler sched;
> MesosSchedulerDriver driver(
> @@ -932,7 +933,7 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeMultipleTasks)
>
> Resources volume = createPersistentVolume(
> Megabytes(64),
> - "role1",
> + DEFAULT_ROLE,
> "id1",
> "path1",
> None(),
> @@ -940,15 +941,21 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeMultipleTasks)
> frameworkInfo.principal(),
> true); // Shared.
>
> + Try<Resources> taskResources1 =
> + Resources::parse("cpus:1;mem:128;disk(" + string(DEFAULT_ROLE) + "):32");
> +
> // Create 2 tasks which write distinct files in the shared volume.
> TaskInfo task1 = createTask(
> offer.slave_id(),
> - Resources::parse("cpus:1;mem:128;disk(role1):32").get() + volume,
> + taskResources1.get() + volume,
> "echo task1 > path1/file1");
>
> + Try<Resources> taskResources2 =
> + Resources::parse("cpus:1;mem:256;disk(" + string(DEFAULT_ROLE) + "):64");
> +
> TaskInfo task2 = createTask(
> offer.slave_id(),
> - Resources::parse("cpus:1;mem:256;disk(role1):64").get() + volume,
> + taskResources2.get() + volume,
> "echo task2 > path1/file2");
>
> // We should receive a TASK_RUNNING followed by a TASK_FINISHED for
> @@ -981,7 +988,7 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeMultipleTasks)
>
> const string& volumePath = slave::paths::getPersistentVolumePath(
> slaveFlags.work_dir,
> - "role1",
> + DEFAULT_ROLE,
> "id1");
>
> EXPECT_SOME_EQ("task1\n", os::read(path::join(volumePath, "file1")));
> @@ -1011,7 +1018,7 @@ TEST_P(PersistentVolumeTest, SlaveRecovery)
> ASSERT_SOME(slave);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
> frameworkInfo.set_checkpoint(true);
>
> MockScheduler sched;
> @@ -1145,7 +1152,7 @@ TEST_P(PersistentVolumeTest, GoodACLCreateThenDestroy)
> filters.set_refuse_seconds(0);
>
> FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> // Create a master.
> master::Flags masterFlags = CreateMasterFlags();
> @@ -1291,7 +1298,7 @@ TEST_P(PersistentVolumeTest, GoodACLNoPrincipal)
> FrameworkInfo frameworkInfo;
> frameworkInfo.set_name("no-principal");
> frameworkInfo.set_user(os::user().get());
> - frameworkInfo.set_role("role1");
> + frameworkInfo.set_role(DEFAULT_ROLE);
>
> // Create a master. Since the framework has no
> // principal, we don't authenticate frameworks.
> @@ -1445,11 +1452,11 @@ TEST_P(PersistentVolumeTest, BadACLNoPrincipal)
> FrameworkInfo frameworkInfo1;
> frameworkInfo1.set_name("no-principal");
> frameworkInfo1.set_user(os::user().get());
> - frameworkInfo1.set_role("role1");
> + frameworkInfo1.set_role(DEFAULT_ROLE);
>
> // Create a `FrameworkInfo` with a principal.
> FrameworkInfo frameworkInfo2 = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo2.set_role("role1");
> + frameworkInfo2.set_role(DEFAULT_ROLE);
>
> // Create a master. Since one framework has no
> // principal, we don't authenticate frameworks.
> @@ -1660,13 +1667,13 @@ TEST_P(PersistentVolumeTest, BadACLDropCreateAndDestroy)
>
> // Create a `FrameworkInfo` that cannot create or destroy volumes.
> FrameworkInfo frameworkInfo1 = DEFAULT_FRAMEWORK_INFO;
> - frameworkInfo1.set_role("role1");
> + frameworkInfo1.set_role(DEFAULT_ROLE);
>
> // Create a `FrameworkInfo` that can create volumes.
> FrameworkInfo frameworkInfo2;
> frameworkInfo2.set_name("creator-framework");
> frameworkInfo2.set_user(os::user().get());
> - frameworkInfo2.set_role("role1");
> + frameworkInfo2.set_role(DEFAULT_ROLE);
> frameworkInfo2.set_principal("creator-principal");
>
> // Create a master.
>
Re: mesos git commit: Added `DEFAULT_ROLE` constant to persistent
volume tests.
Posted by Michael Park <mp...@apache.org>.
This has been addressed in
https://github.com/apache/mesos/commit/7c833abbec9c9e4eb51d67f7a8e7a8d0870825f8
.
On Fri, Sep 23, 2016 at 9:34 PM, Greg Mann <gr...@mesosphere.io> wrote:
> Sure thing! Review here: https://reviews.apache.org/r/52222/
>
> On Fri, Sep 23, 2016 at 9:05 AM, Michael Park <mp...@apache.org> wrote:
>
> > Oy. Thanks for bringing this up Neil. Of course the name stemmed from
> > `DEFAULT_FRAMEWORK` and family.
> > I think I agree that `DEFAULT_TEST_ROLE` is better.
> >
> > In terms of whether it should live just within
> > `persistent_volume_tests.cpp`, I think it has broader applicability,
> > so I'm inclined to keep it in `src/tests/mesos.hpp`. For example, it
> would
> > be useful in `reservation_tests.cpp`.
> >
> > Greg, could you submit a patch to update it to `DEFAULT_TEST_ROLE`?
> >
> > Thanks!
> >
> > MPark
> >
> > On Thu, Sep 22, 2016 at 9:37 PM, Greg Mann <gr...@mesosphere.io> wrote:
> >
> > > Good point, I think `DEFAULT_TEST_ROLE` would be better. I was indeed
> > > hoping to make use of this in other tests, but we could keep them local
> > to
> > > the tests for now, and move it later if it becomes a common pattern.
> > >
> > > Greg
> > >
> > > On Thu, Sep 22, 2016 at 10:13 AM, Neil Conway <ne...@gmail.com>
> > > wrote:
> > >
> > > > I'm not sure this is a good idea: the "default role" is actually "*".
> > > > That is also the default value for the "role" fields in the
> protobufs.
> > > > Perhaps we should name this new constant something like
> > > > DEFAULT_TEST_ROLE?
> > > >
> > > > I wonder also if we should keep the definition local to
> > > > "persistent_volume_tests.cpp", unless there are imminent plans to use
> > > > it elsewhere.
> > > >
> > > > Neil
> > > >
> > > > On Thu, Sep 22, 2016 at 6:37 PM, <mp...@apache.org> wrote:
> > > > > Repository: mesos
> > > > > Updated Branches:
> > > > > refs/heads/master 4df496aaf -> c2b595e1c
> > > > >
> > > > >
> > > > > Added `DEFAULT_ROLE` constant to persistent volume tests.
> > > > >
> > > > > Review: https://reviews.apache.org/r/41613/
> > > > >
> > > > >
> > > > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> > > > > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/
> c2b595e1
> > > > > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c2b595e1
> > > > > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c2b595e1
> > > > >
> > > > > Branch: refs/heads/master
> > > > > Commit: c2b595e1c59bb23e3f87545a7e22a76ad232ae9f
> > > > > Parents: 4df496a
> > > > > Author: Greg Mann <gr...@mesosphere.io>
> > > > > Authored: Thu Sep 22 15:47:31 2016 +0200
> > > > > Committer: Michael Park <mp...@apache.org>
> > > > > Committed: Thu Sep 22 17:28:32 2016 +0200
> > > > >
> > > > > ------------------------------------------------------------
> > ----------
> > > > > src/tests/mesos.hpp | 1 +
> > > > > src/tests/persistent_volume_tests.cpp | 51
> > > > +++++++++++++++++-------------
> > > > > 2 files changed, 30 insertions(+), 22 deletions(-)
> > > > > ------------------------------------------------------------
> > ----------
> > > > >
> > > > >
> > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> > > > src/tests/mesos.hpp
> > > > > ------------------------------------------------------------
> > ----------
> > > > > diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
> > > > > index 7095101..3cd63bd 100644
> > > > > --- a/src/tests/mesos.hpp
> > > > > +++ b/src/tests/mesos.hpp
> > > > > @@ -93,6 +93,7 @@ namespace tests {
> > > > >
> > > > > constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] =
> > > > "test-readonly-realm";
> > > > > constexpr char READWRITE_HTTP_AUTHENTICATION_REALM[] =
> > > > "test-readwrite-realm";
> > > > > +constexpr char DEFAULT_ROLE[] = "default-role";
> > > > >
> > > > > // Forward declarations.
> > > > > class MockExecutor;
> > > > >
> > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> > > > src/tests/persistent_volume_tests.cpp
> > > > > ------------------------------------------------------------
> > ----------
> > > > > diff --git a/src/tests/persistent_volume_tests.cpp
> > > > b/src/tests/persistent_volume_tests.cpp
> > > > > index c38d848..a726d78 100644
> > > > > --- a/src/tests/persistent_volume_tests.cpp
> > > > > +++ b/src/tests/persistent_volume_tests.cpp
> > > > > @@ -166,7 +166,7 @@ protected:
> > > > > case NONE: {
> > > > > diskResource = createDiskResource(
> > > > > stringify(mb.megabytes()),
> > > > > - "role1",
> > > > > + DEFAULT_ROLE,
> > > > > None(),
> > > > > None());
> > > > >
> > > > > @@ -175,7 +175,7 @@ protected:
> > > > > case PATH: {
> > > > > diskResource = createDiskResource(
> > > > > stringify(mb.megabytes()),
> > > > > - "role1",
> > > > > + DEFAULT_ROLE,
> > > > > None(),
> > > > > None(),
> > > > > createDiskSourcePath(path::join(diskPath, "disk" +
> > > > stringify(id))));
> > > > > @@ -185,7 +185,7 @@ protected:
> > > > > case MOUNT: {
> > > > > diskResource = createDiskResource(
> > > > > stringify(mb.megabytes()),
> > > > > - "role1",
> > > > > + DEFAULT_ROLE,
> > > > > None(),
> > > > > None(),
> > > > > createDiskSourceMount(
> > > > > @@ -254,7 +254,7 @@ TEST_P(PersistentVolumeTest,
> > > > CreateAndDestroyPersistentVolumes)
> > > > > Clock::pause();
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > // Create a master.
> > > > > master::Flags masterFlags = CreateMasterFlags();
> > > > > @@ -429,7 +429,7 @@ TEST_P(PersistentVolumeTest,
> > > ResourcesCheckpointing)
> > > > > ASSERT_SOME(slave);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > MockScheduler sched;
> > > > > MesosSchedulerDriver driver(
> > > > > @@ -495,7 +495,7 @@ TEST_P(PersistentVolumeTest,
> > > PreparePersistentVolume)
> > > > > ASSERT_SOME(slave);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > MockScheduler sched;
> > > > > MesosSchedulerDriver driver(
> > > > > @@ -564,7 +564,7 @@ TEST_P(PersistentVolumeTest, MasterFailover)
> > > > > ASSERT_SOME(slave);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > MockScheduler sched;
> > > > > TestingMesosSchedulerDriver driver(&sched, &detector,
> > > frameworkInfo);
> > > > > @@ -659,7 +659,7 @@ TEST_P(PersistentVolumeTest,
> > > > IncompatibleCheckpointedResources)
> > > > > spawn(slave1);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > MockScheduler sched;
> > > > > MesosSchedulerDriver driver(
> > > > > @@ -746,7 +746,7 @@ TEST_P(PersistentVolumeTest,
> > > AccessPersistentVolume)
> > > > > ASSERT_SOME(slave);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > MockScheduler sched;
> > > > > MesosSchedulerDriver driver(
> > > > > @@ -899,14 +899,15 @@ TEST_P(PersistentVolumeTest,
> > > > SharedPersistentVolumeMultipleTasks)
> > > > > ASSERT_SOME(master);
> > > > >
> > > > > slave::Flags slaveFlags = CreateSlaveFlags();
> > > > > - slaveFlags.resources = "cpus:2;mem:1024;disk(role1):1024";
> > > > > + slaveFlags.resources =
> > > > > + "cpus:2;mem:1024;disk(" + string(DEFAULT_ROLE) + "):1024";
> > > > >
> > > > > Owned<MasterDetector> detector = master.get()->createDetector()
> ;
> > > > > Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(),
> > > > slaveFlags);
> > > > > ASSERT_SOME(slave);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > MockScheduler sched;
> > > > > MesosSchedulerDriver driver(
> > > > > @@ -932,7 +933,7 @@ TEST_P(PersistentVolumeTest,
> > > > SharedPersistentVolumeMultipleTasks)
> > > > >
> > > > > Resources volume = createPersistentVolume(
> > > > > Megabytes(64),
> > > > > - "role1",
> > > > > + DEFAULT_ROLE,
> > > > > "id1",
> > > > > "path1",
> > > > > None(),
> > > > > @@ -940,15 +941,21 @@ TEST_P(PersistentVolumeTest,
> > > > SharedPersistentVolumeMultipleTasks)
> > > > > frameworkInfo.principal(),
> > > > > true); // Shared.
> > > > >
> > > > > + Try<Resources> taskResources1 =
> > > > > + Resources::parse("cpus:1;mem:128;disk(" +
> string(DEFAULT_ROLE)
> > +
> > > > "):32");
> > > > > +
> > > > > // Create 2 tasks which write distinct files in the shared
> volume.
> > > > > TaskInfo task1 = createTask(
> > > > > offer.slave_id(),
> > > > > - Resources::parse("cpus:1;mem:128;disk(role1):32").get() +
> > > volume,
> > > > > + taskResources1.get() + volume,
> > > > > "echo task1 > path1/file1");
> > > > >
> > > > > + Try<Resources> taskResources2 =
> > > > > + Resources::parse("cpus:1;mem:256;disk(" +
> string(DEFAULT_ROLE)
> > +
> > > > "):64");
> > > > > +
> > > > > TaskInfo task2 = createTask(
> > > > > offer.slave_id(),
> > > > > - Resources::parse("cpus:1;mem:256;disk(role1):64").get() +
> > > volume,
> > > > > + taskResources2.get() + volume,
> > > > > "echo task2 > path1/file2");
> > > > >
> > > > > // We should receive a TASK_RUNNING followed by a TASK_FINISHED
> > for
> > > > > @@ -981,7 +988,7 @@ TEST_P(PersistentVolumeTest,
> > > > SharedPersistentVolumeMultipleTasks)
> > > > >
> > > > > const string& volumePath = slave::paths::
> getPersistentVolumePath(
> > > > > slaveFlags.work_dir,
> > > > > - "role1",
> > > > > + DEFAULT_ROLE,
> > > > > "id1");
> > > > >
> > > > > EXPECT_SOME_EQ("task1\n", os::read(path::join(volumePath,
> > > "file1")));
> > > > > @@ -1011,7 +1018,7 @@ TEST_P(PersistentVolumeTest, SlaveRecovery)
> > > > > ASSERT_SOME(slave);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > > frameworkInfo.set_checkpoint(true);
> > > > >
> > > > > MockScheduler sched;
> > > > > @@ -1145,7 +1152,7 @@ TEST_P(PersistentVolumeTest,
> > > > GoodACLCreateThenDestroy)
> > > > > filters.set_refuse_seconds(0);
> > > > >
> > > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > // Create a master.
> > > > > master::Flags masterFlags = CreateMasterFlags();
> > > > > @@ -1291,7 +1298,7 @@ TEST_P(PersistentVolumeTest,
> > GoodACLNoPrincipal)
> > > > > FrameworkInfo frameworkInfo;
> > > > > frameworkInfo.set_name("no-principal");
> > > > > frameworkInfo.set_user(os::user().get());
> > > > > - frameworkInfo.set_role("role1");
> > > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > >
> > > > > // Create a master. Since the framework has no
> > > > > // principal, we don't authenticate frameworks.
> > > > > @@ -1445,11 +1452,11 @@ TEST_P(PersistentVolumeTest,
> > BadACLNoPrincipal)
> > > > > FrameworkInfo frameworkInfo1;
> > > > > frameworkInfo1.set_name("no-principal");
> > > > > frameworkInfo1.set_user(os::user().get());
> > > > > - frameworkInfo1.set_role("role1");
> > > > > + frameworkInfo1.set_role(DEFAULT_ROLE);
> > > > >
> > > > > // Create a `FrameworkInfo` with a principal.
> > > > > FrameworkInfo frameworkInfo2 = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo2.set_role("role1");
> > > > > + frameworkInfo2.set_role(DEFAULT_ROLE);
> > > > >
> > > > > // Create a master. Since one framework has no
> > > > > // principal, we don't authenticate frameworks.
> > > > > @@ -1660,13 +1667,13 @@ TEST_P(PersistentVolumeTest,
> > > > BadACLDropCreateAndDestroy)
> > > > >
> > > > > // Create a `FrameworkInfo` that cannot create or destroy
> volumes.
> > > > > FrameworkInfo frameworkInfo1 = DEFAULT_FRAMEWORK_INFO;
> > > > > - frameworkInfo1.set_role("role1");
> > > > > + frameworkInfo1.set_role(DEFAULT_ROLE);
> > > > >
> > > > > // Create a `FrameworkInfo` that can create volumes.
> > > > > FrameworkInfo frameworkInfo2;
> > > > > frameworkInfo2.set_name("creator-framework");
> > > > > frameworkInfo2.set_user(os::user().get());
> > > > > - frameworkInfo2.set_role("role1");
> > > > > + frameworkInfo2.set_role(DEFAULT_ROLE);
> > > > > frameworkInfo2.set_principal("creator-principal");
> > > > >
> > > > > // Create a master.
> > > > >
> > > >
> > >
> >
>
Re: mesos git commit: Added `DEFAULT_ROLE` constant to persistent
volume tests.
Posted by Greg Mann <gr...@mesosphere.io>.
Sure thing! Review here: https://reviews.apache.org/r/52222/
On Fri, Sep 23, 2016 at 9:05 AM, Michael Park <mp...@apache.org> wrote:
> Oy. Thanks for bringing this up Neil. Of course the name stemmed from
> `DEFAULT_FRAMEWORK` and family.
> I think I agree that `DEFAULT_TEST_ROLE` is better.
>
> In terms of whether it should live just within
> `persistent_volume_tests.cpp`, I think it has broader applicability,
> so I'm inclined to keep it in `src/tests/mesos.hpp`. For example, it would
> be useful in `reservation_tests.cpp`.
>
> Greg, could you submit a patch to update it to `DEFAULT_TEST_ROLE`?
>
> Thanks!
>
> MPark
>
> On Thu, Sep 22, 2016 at 9:37 PM, Greg Mann <gr...@mesosphere.io> wrote:
>
> > Good point, I think `DEFAULT_TEST_ROLE` would be better. I was indeed
> > hoping to make use of this in other tests, but we could keep them local
> to
> > the tests for now, and move it later if it becomes a common pattern.
> >
> > Greg
> >
> > On Thu, Sep 22, 2016 at 10:13 AM, Neil Conway <ne...@gmail.com>
> > wrote:
> >
> > > I'm not sure this is a good idea: the "default role" is actually "*".
> > > That is also the default value for the "role" fields in the protobufs.
> > > Perhaps we should name this new constant something like
> > > DEFAULT_TEST_ROLE?
> > >
> > > I wonder also if we should keep the definition local to
> > > "persistent_volume_tests.cpp", unless there are imminent plans to use
> > > it elsewhere.
> > >
> > > Neil
> > >
> > > On Thu, Sep 22, 2016 at 6:37 PM, <mp...@apache.org> wrote:
> > > > Repository: mesos
> > > > Updated Branches:
> > > > refs/heads/master 4df496aaf -> c2b595e1c
> > > >
> > > >
> > > > Added `DEFAULT_ROLE` constant to persistent volume tests.
> > > >
> > > > Review: https://reviews.apache.org/r/41613/
> > > >
> > > >
> > > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> > > > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c2b595e1
> > > > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c2b595e1
> > > > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c2b595e1
> > > >
> > > > Branch: refs/heads/master
> > > > Commit: c2b595e1c59bb23e3f87545a7e22a76ad232ae9f
> > > > Parents: 4df496a
> > > > Author: Greg Mann <gr...@mesosphere.io>
> > > > Authored: Thu Sep 22 15:47:31 2016 +0200
> > > > Committer: Michael Park <mp...@apache.org>
> > > > Committed: Thu Sep 22 17:28:32 2016 +0200
> > > >
> > > > ------------------------------------------------------------
> ----------
> > > > src/tests/mesos.hpp | 1 +
> > > > src/tests/persistent_volume_tests.cpp | 51
> > > +++++++++++++++++-------------
> > > > 2 files changed, 30 insertions(+), 22 deletions(-)
> > > > ------------------------------------------------------------
> ----------
> > > >
> > > >
> > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> > > src/tests/mesos.hpp
> > > > ------------------------------------------------------------
> ----------
> > > > diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
> > > > index 7095101..3cd63bd 100644
> > > > --- a/src/tests/mesos.hpp
> > > > +++ b/src/tests/mesos.hpp
> > > > @@ -93,6 +93,7 @@ namespace tests {
> > > >
> > > > constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] =
> > > "test-readonly-realm";
> > > > constexpr char READWRITE_HTTP_AUTHENTICATION_REALM[] =
> > > "test-readwrite-realm";
> > > > +constexpr char DEFAULT_ROLE[] = "default-role";
> > > >
> > > > // Forward declarations.
> > > > class MockExecutor;
> > > >
> > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> > > src/tests/persistent_volume_tests.cpp
> > > > ------------------------------------------------------------
> ----------
> > > > diff --git a/src/tests/persistent_volume_tests.cpp
> > > b/src/tests/persistent_volume_tests.cpp
> > > > index c38d848..a726d78 100644
> > > > --- a/src/tests/persistent_volume_tests.cpp
> > > > +++ b/src/tests/persistent_volume_tests.cpp
> > > > @@ -166,7 +166,7 @@ protected:
> > > > case NONE: {
> > > > diskResource = createDiskResource(
> > > > stringify(mb.megabytes()),
> > > > - "role1",
> > > > + DEFAULT_ROLE,
> > > > None(),
> > > > None());
> > > >
> > > > @@ -175,7 +175,7 @@ protected:
> > > > case PATH: {
> > > > diskResource = createDiskResource(
> > > > stringify(mb.megabytes()),
> > > > - "role1",
> > > > + DEFAULT_ROLE,
> > > > None(),
> > > > None(),
> > > > createDiskSourcePath(path::join(diskPath, "disk" +
> > > stringify(id))));
> > > > @@ -185,7 +185,7 @@ protected:
> > > > case MOUNT: {
> > > > diskResource = createDiskResource(
> > > > stringify(mb.megabytes()),
> > > > - "role1",
> > > > + DEFAULT_ROLE,
> > > > None(),
> > > > None(),
> > > > createDiskSourceMount(
> > > > @@ -254,7 +254,7 @@ TEST_P(PersistentVolumeTest,
> > > CreateAndDestroyPersistentVolumes)
> > > > Clock::pause();
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > // Create a master.
> > > > master::Flags masterFlags = CreateMasterFlags();
> > > > @@ -429,7 +429,7 @@ TEST_P(PersistentVolumeTest,
> > ResourcesCheckpointing)
> > > > ASSERT_SOME(slave);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > MockScheduler sched;
> > > > MesosSchedulerDriver driver(
> > > > @@ -495,7 +495,7 @@ TEST_P(PersistentVolumeTest,
> > PreparePersistentVolume)
> > > > ASSERT_SOME(slave);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > MockScheduler sched;
> > > > MesosSchedulerDriver driver(
> > > > @@ -564,7 +564,7 @@ TEST_P(PersistentVolumeTest, MasterFailover)
> > > > ASSERT_SOME(slave);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > MockScheduler sched;
> > > > TestingMesosSchedulerDriver driver(&sched, &detector,
> > frameworkInfo);
> > > > @@ -659,7 +659,7 @@ TEST_P(PersistentVolumeTest,
> > > IncompatibleCheckpointedResources)
> > > > spawn(slave1);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > MockScheduler sched;
> > > > MesosSchedulerDriver driver(
> > > > @@ -746,7 +746,7 @@ TEST_P(PersistentVolumeTest,
> > AccessPersistentVolume)
> > > > ASSERT_SOME(slave);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > MockScheduler sched;
> > > > MesosSchedulerDriver driver(
> > > > @@ -899,14 +899,15 @@ TEST_P(PersistentVolumeTest,
> > > SharedPersistentVolumeMultipleTasks)
> > > > ASSERT_SOME(master);
> > > >
> > > > slave::Flags slaveFlags = CreateSlaveFlags();
> > > > - slaveFlags.resources = "cpus:2;mem:1024;disk(role1):1024";
> > > > + slaveFlags.resources =
> > > > + "cpus:2;mem:1024;disk(" + string(DEFAULT_ROLE) + "):1024";
> > > >
> > > > Owned<MasterDetector> detector = master.get()->createDetector();
> > > > Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(),
> > > slaveFlags);
> > > > ASSERT_SOME(slave);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > MockScheduler sched;
> > > > MesosSchedulerDriver driver(
> > > > @@ -932,7 +933,7 @@ TEST_P(PersistentVolumeTest,
> > > SharedPersistentVolumeMultipleTasks)
> > > >
> > > > Resources volume = createPersistentVolume(
> > > > Megabytes(64),
> > > > - "role1",
> > > > + DEFAULT_ROLE,
> > > > "id1",
> > > > "path1",
> > > > None(),
> > > > @@ -940,15 +941,21 @@ TEST_P(PersistentVolumeTest,
> > > SharedPersistentVolumeMultipleTasks)
> > > > frameworkInfo.principal(),
> > > > true); // Shared.
> > > >
> > > > + Try<Resources> taskResources1 =
> > > > + Resources::parse("cpus:1;mem:128;disk(" + string(DEFAULT_ROLE)
> +
> > > "):32");
> > > > +
> > > > // Create 2 tasks which write distinct files in the shared volume.
> > > > TaskInfo task1 = createTask(
> > > > offer.slave_id(),
> > > > - Resources::parse("cpus:1;mem:128;disk(role1):32").get() +
> > volume,
> > > > + taskResources1.get() + volume,
> > > > "echo task1 > path1/file1");
> > > >
> > > > + Try<Resources> taskResources2 =
> > > > + Resources::parse("cpus:1;mem:256;disk(" + string(DEFAULT_ROLE)
> +
> > > "):64");
> > > > +
> > > > TaskInfo task2 = createTask(
> > > > offer.slave_id(),
> > > > - Resources::parse("cpus:1;mem:256;disk(role1):64").get() +
> > volume,
> > > > + taskResources2.get() + volume,
> > > > "echo task2 > path1/file2");
> > > >
> > > > // We should receive a TASK_RUNNING followed by a TASK_FINISHED
> for
> > > > @@ -981,7 +988,7 @@ TEST_P(PersistentVolumeTest,
> > > SharedPersistentVolumeMultipleTasks)
> > > >
> > > > const string& volumePath = slave::paths::getPersistentVolumePath(
> > > > slaveFlags.work_dir,
> > > > - "role1",
> > > > + DEFAULT_ROLE,
> > > > "id1");
> > > >
> > > > EXPECT_SOME_EQ("task1\n", os::read(path::join(volumePath,
> > "file1")));
> > > > @@ -1011,7 +1018,7 @@ TEST_P(PersistentVolumeTest, SlaveRecovery)
> > > > ASSERT_SOME(slave);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > > frameworkInfo.set_checkpoint(true);
> > > >
> > > > MockScheduler sched;
> > > > @@ -1145,7 +1152,7 @@ TEST_P(PersistentVolumeTest,
> > > GoodACLCreateThenDestroy)
> > > > filters.set_refuse_seconds(0);
> > > >
> > > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > // Create a master.
> > > > master::Flags masterFlags = CreateMasterFlags();
> > > > @@ -1291,7 +1298,7 @@ TEST_P(PersistentVolumeTest,
> GoodACLNoPrincipal)
> > > > FrameworkInfo frameworkInfo;
> > > > frameworkInfo.set_name("no-principal");
> > > > frameworkInfo.set_user(os::user().get());
> > > > - frameworkInfo.set_role("role1");
> > > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > >
> > > > // Create a master. Since the framework has no
> > > > // principal, we don't authenticate frameworks.
> > > > @@ -1445,11 +1452,11 @@ TEST_P(PersistentVolumeTest,
> BadACLNoPrincipal)
> > > > FrameworkInfo frameworkInfo1;
> > > > frameworkInfo1.set_name("no-principal");
> > > > frameworkInfo1.set_user(os::user().get());
> > > > - frameworkInfo1.set_role("role1");
> > > > + frameworkInfo1.set_role(DEFAULT_ROLE);
> > > >
> > > > // Create a `FrameworkInfo` with a principal.
> > > > FrameworkInfo frameworkInfo2 = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo2.set_role("role1");
> > > > + frameworkInfo2.set_role(DEFAULT_ROLE);
> > > >
> > > > // Create a master. Since one framework has no
> > > > // principal, we don't authenticate frameworks.
> > > > @@ -1660,13 +1667,13 @@ TEST_P(PersistentVolumeTest,
> > > BadACLDropCreateAndDestroy)
> > > >
> > > > // Create a `FrameworkInfo` that cannot create or destroy volumes.
> > > > FrameworkInfo frameworkInfo1 = DEFAULT_FRAMEWORK_INFO;
> > > > - frameworkInfo1.set_role("role1");
> > > > + frameworkInfo1.set_role(DEFAULT_ROLE);
> > > >
> > > > // Create a `FrameworkInfo` that can create volumes.
> > > > FrameworkInfo frameworkInfo2;
> > > > frameworkInfo2.set_name("creator-framework");
> > > > frameworkInfo2.set_user(os::user().get());
> > > > - frameworkInfo2.set_role("role1");
> > > > + frameworkInfo2.set_role(DEFAULT_ROLE);
> > > > frameworkInfo2.set_principal("creator-principal");
> > > >
> > > > // Create a master.
> > > >
> > >
> >
>
Re: mesos git commit: Added `DEFAULT_ROLE` constant to persistent
volume tests.
Posted by Michael Park <mp...@apache.org>.
Oy. Thanks for bringing this up Neil. Of course the name stemmed from
`DEFAULT_FRAMEWORK` and family.
I think I agree that `DEFAULT_TEST_ROLE` is better.
In terms of whether it should live just within
`persistent_volume_tests.cpp`, I think it has broader applicability,
so I'm inclined to keep it in `src/tests/mesos.hpp`. For example, it would
be useful in `reservation_tests.cpp`.
Greg, could you submit a patch to update it to `DEFAULT_TEST_ROLE`?
Thanks!
MPark
On Thu, Sep 22, 2016 at 9:37 PM, Greg Mann <gr...@mesosphere.io> wrote:
> Good point, I think `DEFAULT_TEST_ROLE` would be better. I was indeed
> hoping to make use of this in other tests, but we could keep them local to
> the tests for now, and move it later if it becomes a common pattern.
>
> Greg
>
> On Thu, Sep 22, 2016 at 10:13 AM, Neil Conway <ne...@gmail.com>
> wrote:
>
> > I'm not sure this is a good idea: the "default role" is actually "*".
> > That is also the default value for the "role" fields in the protobufs.
> > Perhaps we should name this new constant something like
> > DEFAULT_TEST_ROLE?
> >
> > I wonder also if we should keep the definition local to
> > "persistent_volume_tests.cpp", unless there are imminent plans to use
> > it elsewhere.
> >
> > Neil
> >
> > On Thu, Sep 22, 2016 at 6:37 PM, <mp...@apache.org> wrote:
> > > Repository: mesos
> > > Updated Branches:
> > > refs/heads/master 4df496aaf -> c2b595e1c
> > >
> > >
> > > Added `DEFAULT_ROLE` constant to persistent volume tests.
> > >
> > > Review: https://reviews.apache.org/r/41613/
> > >
> > >
> > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> > > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c2b595e1
> > > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c2b595e1
> > > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c2b595e1
> > >
> > > Branch: refs/heads/master
> > > Commit: c2b595e1c59bb23e3f87545a7e22a76ad232ae9f
> > > Parents: 4df496a
> > > Author: Greg Mann <gr...@mesosphere.io>
> > > Authored: Thu Sep 22 15:47:31 2016 +0200
> > > Committer: Michael Park <mp...@apache.org>
> > > Committed: Thu Sep 22 17:28:32 2016 +0200
> > >
> > > ----------------------------------------------------------------------
> > > src/tests/mesos.hpp | 1 +
> > > src/tests/persistent_volume_tests.cpp | 51
> > +++++++++++++++++-------------
> > > 2 files changed, 30 insertions(+), 22 deletions(-)
> > > ----------------------------------------------------------------------
> > >
> > >
> > > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> > src/tests/mesos.hpp
> > > ----------------------------------------------------------------------
> > > diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
> > > index 7095101..3cd63bd 100644
> > > --- a/src/tests/mesos.hpp
> > > +++ b/src/tests/mesos.hpp
> > > @@ -93,6 +93,7 @@ namespace tests {
> > >
> > > constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] =
> > "test-readonly-realm";
> > > constexpr char READWRITE_HTTP_AUTHENTICATION_REALM[] =
> > "test-readwrite-realm";
> > > +constexpr char DEFAULT_ROLE[] = "default-role";
> > >
> > > // Forward declarations.
> > > class MockExecutor;
> > >
> > > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> > src/tests/persistent_volume_tests.cpp
> > > ----------------------------------------------------------------------
> > > diff --git a/src/tests/persistent_volume_tests.cpp
> > b/src/tests/persistent_volume_tests.cpp
> > > index c38d848..a726d78 100644
> > > --- a/src/tests/persistent_volume_tests.cpp
> > > +++ b/src/tests/persistent_volume_tests.cpp
> > > @@ -166,7 +166,7 @@ protected:
> > > case NONE: {
> > > diskResource = createDiskResource(
> > > stringify(mb.megabytes()),
> > > - "role1",
> > > + DEFAULT_ROLE,
> > > None(),
> > > None());
> > >
> > > @@ -175,7 +175,7 @@ protected:
> > > case PATH: {
> > > diskResource = createDiskResource(
> > > stringify(mb.megabytes()),
> > > - "role1",
> > > + DEFAULT_ROLE,
> > > None(),
> > > None(),
> > > createDiskSourcePath(path::join(diskPath, "disk" +
> > stringify(id))));
> > > @@ -185,7 +185,7 @@ protected:
> > > case MOUNT: {
> > > diskResource = createDiskResource(
> > > stringify(mb.megabytes()),
> > > - "role1",
> > > + DEFAULT_ROLE,
> > > None(),
> > > None(),
> > > createDiskSourceMount(
> > > @@ -254,7 +254,7 @@ TEST_P(PersistentVolumeTest,
> > CreateAndDestroyPersistentVolumes)
> > > Clock::pause();
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > // Create a master.
> > > master::Flags masterFlags = CreateMasterFlags();
> > > @@ -429,7 +429,7 @@ TEST_P(PersistentVolumeTest,
> ResourcesCheckpointing)
> > > ASSERT_SOME(slave);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > MockScheduler sched;
> > > MesosSchedulerDriver driver(
> > > @@ -495,7 +495,7 @@ TEST_P(PersistentVolumeTest,
> PreparePersistentVolume)
> > > ASSERT_SOME(slave);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > MockScheduler sched;
> > > MesosSchedulerDriver driver(
> > > @@ -564,7 +564,7 @@ TEST_P(PersistentVolumeTest, MasterFailover)
> > > ASSERT_SOME(slave);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > MockScheduler sched;
> > > TestingMesosSchedulerDriver driver(&sched, &detector,
> frameworkInfo);
> > > @@ -659,7 +659,7 @@ TEST_P(PersistentVolumeTest,
> > IncompatibleCheckpointedResources)
> > > spawn(slave1);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > MockScheduler sched;
> > > MesosSchedulerDriver driver(
> > > @@ -746,7 +746,7 @@ TEST_P(PersistentVolumeTest,
> AccessPersistentVolume)
> > > ASSERT_SOME(slave);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > MockScheduler sched;
> > > MesosSchedulerDriver driver(
> > > @@ -899,14 +899,15 @@ TEST_P(PersistentVolumeTest,
> > SharedPersistentVolumeMultipleTasks)
> > > ASSERT_SOME(master);
> > >
> > > slave::Flags slaveFlags = CreateSlaveFlags();
> > > - slaveFlags.resources = "cpus:2;mem:1024;disk(role1):1024";
> > > + slaveFlags.resources =
> > > + "cpus:2;mem:1024;disk(" + string(DEFAULT_ROLE) + "):1024";
> > >
> > > Owned<MasterDetector> detector = master.get()->createDetector();
> > > Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(),
> > slaveFlags);
> > > ASSERT_SOME(slave);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > MockScheduler sched;
> > > MesosSchedulerDriver driver(
> > > @@ -932,7 +933,7 @@ TEST_P(PersistentVolumeTest,
> > SharedPersistentVolumeMultipleTasks)
> > >
> > > Resources volume = createPersistentVolume(
> > > Megabytes(64),
> > > - "role1",
> > > + DEFAULT_ROLE,
> > > "id1",
> > > "path1",
> > > None(),
> > > @@ -940,15 +941,21 @@ TEST_P(PersistentVolumeTest,
> > SharedPersistentVolumeMultipleTasks)
> > > frameworkInfo.principal(),
> > > true); // Shared.
> > >
> > > + Try<Resources> taskResources1 =
> > > + Resources::parse("cpus:1;mem:128;disk(" + string(DEFAULT_ROLE) +
> > "):32");
> > > +
> > > // Create 2 tasks which write distinct files in the shared volume.
> > > TaskInfo task1 = createTask(
> > > offer.slave_id(),
> > > - Resources::parse("cpus:1;mem:128;disk(role1):32").get() +
> volume,
> > > + taskResources1.get() + volume,
> > > "echo task1 > path1/file1");
> > >
> > > + Try<Resources> taskResources2 =
> > > + Resources::parse("cpus:1;mem:256;disk(" + string(DEFAULT_ROLE) +
> > "):64");
> > > +
> > > TaskInfo task2 = createTask(
> > > offer.slave_id(),
> > > - Resources::parse("cpus:1;mem:256;disk(role1):64").get() +
> volume,
> > > + taskResources2.get() + volume,
> > > "echo task2 > path1/file2");
> > >
> > > // We should receive a TASK_RUNNING followed by a TASK_FINISHED for
> > > @@ -981,7 +988,7 @@ TEST_P(PersistentVolumeTest,
> > SharedPersistentVolumeMultipleTasks)
> > >
> > > const string& volumePath = slave::paths::getPersistentVolumePath(
> > > slaveFlags.work_dir,
> > > - "role1",
> > > + DEFAULT_ROLE,
> > > "id1");
> > >
> > > EXPECT_SOME_EQ("task1\n", os::read(path::join(volumePath,
> "file1")));
> > > @@ -1011,7 +1018,7 @@ TEST_P(PersistentVolumeTest, SlaveRecovery)
> > > ASSERT_SOME(slave);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > > frameworkInfo.set_checkpoint(true);
> > >
> > > MockScheduler sched;
> > > @@ -1145,7 +1152,7 @@ TEST_P(PersistentVolumeTest,
> > GoodACLCreateThenDestroy)
> > > filters.set_refuse_seconds(0);
> > >
> > > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > // Create a master.
> > > master::Flags masterFlags = CreateMasterFlags();
> > > @@ -1291,7 +1298,7 @@ TEST_P(PersistentVolumeTest, GoodACLNoPrincipal)
> > > FrameworkInfo frameworkInfo;
> > > frameworkInfo.set_name("no-principal");
> > > frameworkInfo.set_user(os::user().get());
> > > - frameworkInfo.set_role("role1");
> > > + frameworkInfo.set_role(DEFAULT_ROLE);
> > >
> > > // Create a master. Since the framework has no
> > > // principal, we don't authenticate frameworks.
> > > @@ -1445,11 +1452,11 @@ TEST_P(PersistentVolumeTest, BadACLNoPrincipal)
> > > FrameworkInfo frameworkInfo1;
> > > frameworkInfo1.set_name("no-principal");
> > > frameworkInfo1.set_user(os::user().get());
> > > - frameworkInfo1.set_role("role1");
> > > + frameworkInfo1.set_role(DEFAULT_ROLE);
> > >
> > > // Create a `FrameworkInfo` with a principal.
> > > FrameworkInfo frameworkInfo2 = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo2.set_role("role1");
> > > + frameworkInfo2.set_role(DEFAULT_ROLE);
> > >
> > > // Create a master. Since one framework has no
> > > // principal, we don't authenticate frameworks.
> > > @@ -1660,13 +1667,13 @@ TEST_P(PersistentVolumeTest,
> > BadACLDropCreateAndDestroy)
> > >
> > > // Create a `FrameworkInfo` that cannot create or destroy volumes.
> > > FrameworkInfo frameworkInfo1 = DEFAULT_FRAMEWORK_INFO;
> > > - frameworkInfo1.set_role("role1");
> > > + frameworkInfo1.set_role(DEFAULT_ROLE);
> > >
> > > // Create a `FrameworkInfo` that can create volumes.
> > > FrameworkInfo frameworkInfo2;
> > > frameworkInfo2.set_name("creator-framework");
> > > frameworkInfo2.set_user(os::user().get());
> > > - frameworkInfo2.set_role("role1");
> > > + frameworkInfo2.set_role(DEFAULT_ROLE);
> > > frameworkInfo2.set_principal("creator-principal");
> > >
> > > // Create a master.
> > >
> >
>
Re: mesos git commit: Added `DEFAULT_ROLE` constant to persistent
volume tests.
Posted by Greg Mann <gr...@mesosphere.io>.
Good point, I think `DEFAULT_TEST_ROLE` would be better. I was indeed
hoping to make use of this in other tests, but we could keep them local to
the tests for now, and move it later if it becomes a common pattern.
Greg
On Thu, Sep 22, 2016 at 10:13 AM, Neil Conway <ne...@gmail.com> wrote:
> I'm not sure this is a good idea: the "default role" is actually "*".
> That is also the default value for the "role" fields in the protobufs.
> Perhaps we should name this new constant something like
> DEFAULT_TEST_ROLE?
>
> I wonder also if we should keep the definition local to
> "persistent_volume_tests.cpp", unless there are imminent plans to use
> it elsewhere.
>
> Neil
>
> On Thu, Sep 22, 2016 at 6:37 PM, <mp...@apache.org> wrote:
> > Repository: mesos
> > Updated Branches:
> > refs/heads/master 4df496aaf -> c2b595e1c
> >
> >
> > Added `DEFAULT_ROLE` constant to persistent volume tests.
> >
> > Review: https://reviews.apache.org/r/41613/
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c2b595e1
> > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c2b595e1
> > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c2b595e1
> >
> > Branch: refs/heads/master
> > Commit: c2b595e1c59bb23e3f87545a7e22a76ad232ae9f
> > Parents: 4df496a
> > Author: Greg Mann <gr...@mesosphere.io>
> > Authored: Thu Sep 22 15:47:31 2016 +0200
> > Committer: Michael Park <mp...@apache.org>
> > Committed: Thu Sep 22 17:28:32 2016 +0200
> >
> > ----------------------------------------------------------------------
> > src/tests/mesos.hpp | 1 +
> > src/tests/persistent_volume_tests.cpp | 51
> +++++++++++++++++-------------
> > 2 files changed, 30 insertions(+), 22 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> src/tests/mesos.hpp
> > ----------------------------------------------------------------------
> > diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
> > index 7095101..3cd63bd 100644
> > --- a/src/tests/mesos.hpp
> > +++ b/src/tests/mesos.hpp
> > @@ -93,6 +93,7 @@ namespace tests {
> >
> > constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] =
> "test-readonly-realm";
> > constexpr char READWRITE_HTTP_AUTHENTICATION_REALM[] =
> "test-readwrite-realm";
> > +constexpr char DEFAULT_ROLE[] = "default-role";
> >
> > // Forward declarations.
> > class MockExecutor;
> >
> > http://git-wip-us.apache.org/repos/asf/mesos/blob/c2b595e1/
> src/tests/persistent_volume_tests.cpp
> > ----------------------------------------------------------------------
> > diff --git a/src/tests/persistent_volume_tests.cpp
> b/src/tests/persistent_volume_tests.cpp
> > index c38d848..a726d78 100644
> > --- a/src/tests/persistent_volume_tests.cpp
> > +++ b/src/tests/persistent_volume_tests.cpp
> > @@ -166,7 +166,7 @@ protected:
> > case NONE: {
> > diskResource = createDiskResource(
> > stringify(mb.megabytes()),
> > - "role1",
> > + DEFAULT_ROLE,
> > None(),
> > None());
> >
> > @@ -175,7 +175,7 @@ protected:
> > case PATH: {
> > diskResource = createDiskResource(
> > stringify(mb.megabytes()),
> > - "role1",
> > + DEFAULT_ROLE,
> > None(),
> > None(),
> > createDiskSourcePath(path::join(diskPath, "disk" +
> stringify(id))));
> > @@ -185,7 +185,7 @@ protected:
> > case MOUNT: {
> > diskResource = createDiskResource(
> > stringify(mb.megabytes()),
> > - "role1",
> > + DEFAULT_ROLE,
> > None(),
> > None(),
> > createDiskSourceMount(
> > @@ -254,7 +254,7 @@ TEST_P(PersistentVolumeTest,
> CreateAndDestroyPersistentVolumes)
> > Clock::pause();
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > // Create a master.
> > master::Flags masterFlags = CreateMasterFlags();
> > @@ -429,7 +429,7 @@ TEST_P(PersistentVolumeTest, ResourcesCheckpointing)
> > ASSERT_SOME(slave);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > MockScheduler sched;
> > MesosSchedulerDriver driver(
> > @@ -495,7 +495,7 @@ TEST_P(PersistentVolumeTest, PreparePersistentVolume)
> > ASSERT_SOME(slave);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > MockScheduler sched;
> > MesosSchedulerDriver driver(
> > @@ -564,7 +564,7 @@ TEST_P(PersistentVolumeTest, MasterFailover)
> > ASSERT_SOME(slave);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > MockScheduler sched;
> > TestingMesosSchedulerDriver driver(&sched, &detector, frameworkInfo);
> > @@ -659,7 +659,7 @@ TEST_P(PersistentVolumeTest,
> IncompatibleCheckpointedResources)
> > spawn(slave1);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > MockScheduler sched;
> > MesosSchedulerDriver driver(
> > @@ -746,7 +746,7 @@ TEST_P(PersistentVolumeTest, AccessPersistentVolume)
> > ASSERT_SOME(slave);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > MockScheduler sched;
> > MesosSchedulerDriver driver(
> > @@ -899,14 +899,15 @@ TEST_P(PersistentVolumeTest,
> SharedPersistentVolumeMultipleTasks)
> > ASSERT_SOME(master);
> >
> > slave::Flags slaveFlags = CreateSlaveFlags();
> > - slaveFlags.resources = "cpus:2;mem:1024;disk(role1):1024";
> > + slaveFlags.resources =
> > + "cpus:2;mem:1024;disk(" + string(DEFAULT_ROLE) + "):1024";
> >
> > Owned<MasterDetector> detector = master.get()->createDetector();
> > Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(),
> slaveFlags);
> > ASSERT_SOME(slave);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > MockScheduler sched;
> > MesosSchedulerDriver driver(
> > @@ -932,7 +933,7 @@ TEST_P(PersistentVolumeTest,
> SharedPersistentVolumeMultipleTasks)
> >
> > Resources volume = createPersistentVolume(
> > Megabytes(64),
> > - "role1",
> > + DEFAULT_ROLE,
> > "id1",
> > "path1",
> > None(),
> > @@ -940,15 +941,21 @@ TEST_P(PersistentVolumeTest,
> SharedPersistentVolumeMultipleTasks)
> > frameworkInfo.principal(),
> > true); // Shared.
> >
> > + Try<Resources> taskResources1 =
> > + Resources::parse("cpus:1;mem:128;disk(" + string(DEFAULT_ROLE) +
> "):32");
> > +
> > // Create 2 tasks which write distinct files in the shared volume.
> > TaskInfo task1 = createTask(
> > offer.slave_id(),
> > - Resources::parse("cpus:1;mem:128;disk(role1):32").get() + volume,
> > + taskResources1.get() + volume,
> > "echo task1 > path1/file1");
> >
> > + Try<Resources> taskResources2 =
> > + Resources::parse("cpus:1;mem:256;disk(" + string(DEFAULT_ROLE) +
> "):64");
> > +
> > TaskInfo task2 = createTask(
> > offer.slave_id(),
> > - Resources::parse("cpus:1;mem:256;disk(role1):64").get() + volume,
> > + taskResources2.get() + volume,
> > "echo task2 > path1/file2");
> >
> > // We should receive a TASK_RUNNING followed by a TASK_FINISHED for
> > @@ -981,7 +988,7 @@ TEST_P(PersistentVolumeTest,
> SharedPersistentVolumeMultipleTasks)
> >
> > const string& volumePath = slave::paths::getPersistentVolumePath(
> > slaveFlags.work_dir,
> > - "role1",
> > + DEFAULT_ROLE,
> > "id1");
> >
> > EXPECT_SOME_EQ("task1\n", os::read(path::join(volumePath, "file1")));
> > @@ -1011,7 +1018,7 @@ TEST_P(PersistentVolumeTest, SlaveRecovery)
> > ASSERT_SOME(slave);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> > frameworkInfo.set_checkpoint(true);
> >
> > MockScheduler sched;
> > @@ -1145,7 +1152,7 @@ TEST_P(PersistentVolumeTest,
> GoodACLCreateThenDestroy)
> > filters.set_refuse_seconds(0);
> >
> > FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > // Create a master.
> > master::Flags masterFlags = CreateMasterFlags();
> > @@ -1291,7 +1298,7 @@ TEST_P(PersistentVolumeTest, GoodACLNoPrincipal)
> > FrameworkInfo frameworkInfo;
> > frameworkInfo.set_name("no-principal");
> > frameworkInfo.set_user(os::user().get());
> > - frameworkInfo.set_role("role1");
> > + frameworkInfo.set_role(DEFAULT_ROLE);
> >
> > // Create a master. Since the framework has no
> > // principal, we don't authenticate frameworks.
> > @@ -1445,11 +1452,11 @@ TEST_P(PersistentVolumeTest, BadACLNoPrincipal)
> > FrameworkInfo frameworkInfo1;
> > frameworkInfo1.set_name("no-principal");
> > frameworkInfo1.set_user(os::user().get());
> > - frameworkInfo1.set_role("role1");
> > + frameworkInfo1.set_role(DEFAULT_ROLE);
> >
> > // Create a `FrameworkInfo` with a principal.
> > FrameworkInfo frameworkInfo2 = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo2.set_role("role1");
> > + frameworkInfo2.set_role(DEFAULT_ROLE);
> >
> > // Create a master. Since one framework has no
> > // principal, we don't authenticate frameworks.
> > @@ -1660,13 +1667,13 @@ TEST_P(PersistentVolumeTest,
> BadACLDropCreateAndDestroy)
> >
> > // Create a `FrameworkInfo` that cannot create or destroy volumes.
> > FrameworkInfo frameworkInfo1 = DEFAULT_FRAMEWORK_INFO;
> > - frameworkInfo1.set_role("role1");
> > + frameworkInfo1.set_role(DEFAULT_ROLE);
> >
> > // Create a `FrameworkInfo` that can create volumes.
> > FrameworkInfo frameworkInfo2;
> > frameworkInfo2.set_name("creator-framework");
> > frameworkInfo2.set_user(os::user().get());
> > - frameworkInfo2.set_role("role1");
> > + frameworkInfo2.set_role(DEFAULT_ROLE);
> > frameworkInfo2.set_principal("creator-principal");
> >
> > // Create a master.
> >
>