This has been addressed in https://github.com/apache/mesos/commit/7c833abbec9c9e4eb51d67f7a8e7a8d0870825f8 .
On Fri, Sep 23, 2016 at 9:34 PM, Greg Mann <[email protected]> wrote: > Sure thing! Review here: https://reviews.apache.org/r/52222/ > > On Fri, Sep 23, 2016 at 9:05 AM, Michael Park <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > 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, <[email protected]> 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 <[email protected]> > > > > > Authored: Thu Sep 22 15:47:31 2016 +0200 > > > > > Committer: Michael Park <[email protected]> > > > > > 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. > > > > > > > > > > > > > > >
