From 0829c3386439a239337090932ae9d382ed70cc1d Mon Sep 17 00:00:00 2001
From: Dongsu Park <dongsu@endocode.com>
Date: Fri, 22 Jul 2016 14:08:36 +0200
Subject: [PATCH] agent: check errors from start/stop-ing unit

Now that UnitManager.Trigger{Start,Stop}() could return an error, the
error should be delivered up to mapTaskToFunc(), not to be ignored by
the task manager. Doing that, we can fix a bug in fleet ignoring failed
attempt to start/stop a unit.

Fixes https://github.com/coreos/fleet/issues/998
---
 agent/agent.go      |  8 ++++----
 agent/agent_test.go | 10 ++++++++--
 agent/task.go       |  4 ++--
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/agent/agent.go b/agent/agent.go
index 310ecd6ca..3ae99db4e 100644
--- a/agent/agent.go
+++ b/agent/agent.go
@@ -119,20 +119,20 @@ func (a *Agent) unloadUnit(unitName string) error {
 	return errUnload
 }
 
-func (a *Agent) startUnit(unitName string) {
+func (a *Agent) startUnit(unitName string) error {
 	a.cache.setTargetState(unitName, job.JobStateLaunched)
 
 	machID := a.Machine.State().ID
 	a.registry.UnitHeartbeat(unitName, machID, a.ttl)
 
-	a.um.TriggerStart(unitName)
+	return a.um.TriggerStart(unitName)
 }
 
-func (a *Agent) stopUnit(unitName string) {
+func (a *Agent) stopUnit(unitName string) error {
 	a.cache.setTargetState(unitName, job.JobStateLoaded)
 	a.registry.ClearUnitHeartbeat(unitName)
 
-	a.um.TriggerStop(unitName)
+	return a.um.TriggerStop(unitName)
 }
 
 type unitState struct {
diff --git a/agent/agent_test.go b/agent/agent_test.go
index 9ed18a08f..297cbe02f 100644
--- a/agent/agent_test.go
+++ b/agent/agent_test.go
@@ -117,7 +117,10 @@ func TestAgentLoadStartStopUnit(t *testing.T) {
 		t.Fatalf("Failed calling Agent.loadUnit: %v", err)
 	}
 
-	a.startUnit("foo.service")
+	err = a.startUnit("foo.service")
+	if err != nil {
+		t.Fatalf("Failed starting unit foo.service: %v", err)
+	}
 
 	units, err := a.units()
 	if err != nil {
@@ -135,7 +138,10 @@ func TestAgentLoadStartStopUnit(t *testing.T) {
 		t.Fatalf("Received unexpected collection of Units: %#v\nExpected: %#v", units, expectUnits)
 	}
 
-	a.stopUnit("foo.service")
+	err = a.stopUnit("foo.service")
+	if err != nil {
+		t.Fatalf("Failed stopping unit foo.service: %v", err)
+	}
 
 	units, err = a.units()
 	if err != nil {
diff --git a/agent/task.go b/agent/task.go
index b4bd82cf0..d293218aa 100644
--- a/agent/task.go
+++ b/agent/task.go
@@ -106,9 +106,9 @@ func mapTaskToFunc(t task, a *Agent) (fn func() error, err error) {
 	case taskTypeUnloadUnit:
 		fn = func() error { return a.unloadUnit(t.unit.Name) }
 	case taskTypeStartUnit:
-		fn = func() error { a.startUnit(t.unit.Name); return nil }
+		fn = func() error { return a.startUnit(t.unit.Name) }
 	case taskTypeStopUnit:
-		fn = func() error { a.stopUnit(t.unit.Name); return nil }
+		fn = func() error { return a.stopUnit(t.unit.Name) }
 	case taskTypeReloadUnitFiles:
 		fn = func() error { return a.reloadUnitFiles() }
 	default: