From 0ad0c1f2b1aa7614cef7ede252cd94e3acd33d6f Mon Sep 17 00:00:00 2001
From: Sergiusz Urbaniak <sur@mesosphere.io>
Date: Tue, 20 Oct 2015 14:24:20 -0700
Subject: [PATCH] scheduler: remove error param from doSchedule func

doSchedule currently accepts err values from previous invocation delegating
error handling in a location different from the caller which can be hard
to debug and is not a good practice.

We still maintain the same invariants after the refactoring. If an err
happened in a previous invocation to Register, the returned task object
was nil causing task.AcceptedOffer() to return false.

By not invoking doSchedule in case of an error we can eliminate the
first `err == nil` check in doScheduler.
---
 contrib/mesos/pkg/scheduler/plugin.go | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/contrib/mesos/pkg/scheduler/plugin.go b/contrib/mesos/pkg/scheduler/plugin.go
index 37dedda7c6d..fbde9ca89eb 100644
--- a/contrib/mesos/pkg/scheduler/plugin.go
+++ b/contrib/mesos/pkg/scheduler/plugin.go
@@ -278,7 +278,11 @@ func (k *kubeScheduler) Schedule(pod *api.Pod, unused algorithm.NodeLister) (str
 			log.Infof("aborting Schedule, pod has been deleted %+v", pod)
 			return "", noSuchPodErr
 		}
-		return k.doSchedule(k.api.tasks().Register(k.api.createPodTask(ctx, pod)))
+		task, err := k.api.tasks().Register(k.api.createPodTask(ctx, pod))
+		if err != nil {
+			return "", err
+		}
+		return k.doSchedule(task)
 
 	//TODO(jdef) it's possible that the pod state has diverged from what
 	//we knew previously, we should probably update the task.Pod state here
@@ -294,7 +298,7 @@ func (k *kubeScheduler) Schedule(pod *api.Pod, unused algorithm.NodeLister) (str
 			// but we're going to let someone else handle it, probably the mesos task error handler
 			return "", fmt.Errorf("task %s has already been launched, aborting schedule", task.ID)
 		} else {
-			return k.doSchedule(task, nil)
+			return k.doSchedule(task)
 		}
 
 	default:
@@ -303,8 +307,10 @@ func (k *kubeScheduler) Schedule(pod *api.Pod, unused algorithm.NodeLister) (str
 }
 
 // Call ScheduleFunc and subtract some resources, returning the name of the machine the task is scheduled on
-func (k *kubeScheduler) doSchedule(task *podtask.T, err error) (string, error) {
+func (k *kubeScheduler) doSchedule(task *podtask.T) (string, error) {
 	var offer offers.Perishable
+	var err error
+
 	if task.HasAcceptedOffer() {
 		// verify that the offer is still on the table
 		offerId := task.GetOfferId()
@@ -319,7 +325,7 @@ func (k *kubeScheduler) doSchedule(task *podtask.T, err error) (string, error) {
 			}
 		}
 	}
-	if err == nil && offer == nil {
+	if offer == nil {
 		offer, err = k.api.algorithm().SchedulePod(k.api.offers(), k.api, task)
 	}
 	if err != nil {