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 <g...@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. >
