From 9d5cb64199503e36103cc145224c4c3197487366 Mon Sep 17 00:00:00 2001
From: James DeFelice <james@mesosphere.io>
Date: Fri, 15 Sep 2017 14:48:30 +0000
Subject: [PATCH] lib/scheduler: further simplify revive/suppress APIs

---
 api/v1/lib/scheduler/calls/calls.go      | 43 +++++++++++-------------
 api/v1/lib/scheduler/calls/calls_test.go | 15 +++++----
 api/v1/lib/scheduler/options.go          | 14 --------
 3 files changed, 28 insertions(+), 44 deletions(-)

diff --git a/api/v1/lib/scheduler/calls/calls.go b/api/v1/lib/scheduler/calls/calls.go
index 6a32fb97..9291c21f 100644
--- a/api/v1/lib/scheduler/calls/calls.go
+++ b/api/v1/lib/scheduler/calls/calls.go
@@ -200,39 +200,34 @@ func OpDestroy(rs ...mesos.Resource) mesos.Offer_Operation {
 	}
 }
 
-// OfferFlowControl is a marker interface for Call subtypes that adjust offer throttling.
-type OfferFlowControl interface {
-	SetRoles(roles ...string)
+// Revive returns a revive call.
+// Callers are expected to fill in the FrameworkID.
+func Revive() *scheduler.Call {
+	return &scheduler.Call{Type: scheduler.Call_REVIVE}
 }
 
-// OfferFlowOpt configures OfferFlowControl.
-type OfferFlowOpt func(OfferFlowControl)
-
-// Roles configures the roles for an OfferFlowControl.
-func Roles(roles ...string) OfferFlowOpt { return func(ofc OfferFlowControl) { ofc.SetRoles(roles...) } }
-
-// Revive returns a revive call.
+// Revive returns a revive call with the given filters.
 // Callers are expected to fill in the FrameworkID.
-func Revive(opts ...OfferFlowOpt) (c *scheduler.Call) {
-	c = &scheduler.Call{Type: scheduler.Call_REVIVE, Revive: &scheduler.Call_Revive{}}
-	for _, f := range opts {
-		if f != nil {
-			f(c.Revive)
-		}
+func ReviveWith(roles []string) *scheduler.Call {
+	return &scheduler.Call{
+		Type:   scheduler.Call_REVIVE,
+		Revive: &scheduler.Call_Revive{Roles: roles},
 	}
-	return
 }
 
 // Suppress returns a suppress call.
 // Callers are expected to fill in the FrameworkID.
-func Suppress(opts ...OfferFlowOpt) (c *scheduler.Call) {
-	c = &scheduler.Call{Type: scheduler.Call_SUPPRESS, Suppress: &scheduler.Call_Suppress{}}
-	for _, f := range opts {
-		if f != nil {
-			f(c.Suppress)
-		}
+func Suppress() *scheduler.Call {
+	return &scheduler.Call{Type: scheduler.Call_SUPPRESS}
+}
+
+// Suppress returns a suppress call with the given filters.
+// Callers are expected to fill in the FrameworkID.
+func SuppressWith(roles []string) *scheduler.Call {
+	return &scheduler.Call{
+		Type:     scheduler.Call_SUPPRESS,
+		Suppress: &scheduler.Call_Suppress{Roles: roles},
 	}
-	return
 }
 
 // Decline returns a decline call with the given parameters.
diff --git a/api/v1/lib/scheduler/calls/calls_test.go b/api/v1/lib/scheduler/calls/calls_test.go
index 07513952..19df2306 100644
--- a/api/v1/lib/scheduler/calls/calls_test.go
+++ b/api/v1/lib/scheduler/calls/calls_test.go
@@ -18,18 +18,21 @@ func TestRole(t *testing.T) {
 		call  *scheduler.Call
 		roles []string
 	}{
-		{calls.Revive(calls.Roles()), rolesNone},
-		{calls.Suppress(calls.Roles()), rolesNone},
+		{calls.Revive(), rolesNone},
+		{calls.Suppress(), rolesNone},
 
-		{calls.Revive(calls.Roles(roleX...)), roleX},
-		{calls.Suppress(calls.Roles(roleX...)), roleX},
+		{calls.ReviveWith(nil), rolesNone},
+		{calls.SuppressWith(nil), rolesNone},
+
+		{calls.ReviveWith(roleX), roleX},
+		{calls.SuppressWith(roleX), roleX},
 	} {
 		roles, hasRole := func() ([]string, bool) {
 			switch tc.call.Type {
 			case scheduler.Call_SUPPRESS:
-				return tc.call.Suppress.GetRoles(), len(tc.call.Suppress.Roles) > 0
+				return tc.call.GetSuppress().GetRoles(), len(tc.call.GetSuppress().GetRoles()) > 0
 			case scheduler.Call_REVIVE:
-				return tc.call.Revive.GetRoles(), len(tc.call.Revive.Roles) > 0
+				return tc.call.GetRevive().GetRoles(), len(tc.call.GetRevive().GetRoles()) > 0
 			default:
 				panic(fmt.Sprintf("test case %d failed: unsupported call type: %v", ti, tc.call.Type))
 			}
diff --git a/api/v1/lib/scheduler/options.go b/api/v1/lib/scheduler/options.go
index 772b22de..e7bc2ac1 100644
--- a/api/v1/lib/scheduler/options.go
+++ b/api/v1/lib/scheduler/options.go
@@ -37,17 +37,3 @@ func (co CallOptions) Copy() CallOptions {
 	copy(x, co)
 	return x
 }
-
-// SetRoles implements calls.OfferFlowControl
-func (c *Call_Revive) SetRoles(roles ...string) {
-	if c != nil {
-		c.Roles = roles
-	}
-}
-
-// SetRoles implements calls.OfferFlowControl
-func (c *Call_Suppress) SetRoles(roles ...string) {
-	if c != nil {
-		c.Roles = roles
-	}
-}