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 coreos#998

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 {

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 {

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: