diff --git a/agent/agent.go b/agent/agent.go index e580a9ecd..3ae99db4e 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -94,7 +94,7 @@ func (a *Agent) loadUnit(u *job.Unit) error { return a.um.Load(u.Name, u.Unit) } -func (a *Agent) unloadUnit(unitName string) { +func (a *Agent) unloadUnit(unitName string) error { a.registry.ClearUnitHeartbeat(unitName) a.cache.dropTargetState(unitName) @@ -111,25 +111,28 @@ func (a *Agent) unloadUnit(unitName string) { // could be successfully stopped. Otherwise the unit could get into a state // where the unit cannot be stopped via fleet, because the unit file was // already removed. See also https://github.com/coreos/fleet/issues/1216. + var errUnload error if errStop == nil { - a.um.Unload(unitName) + errUnload = a.um.Unload(unitName) } + + 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 5ea4ba483..297cbe02f 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -87,7 +87,10 @@ func TestAgentLoadUnloadUnit(t *testing.T) { t.Fatalf("Received unexpected collection of Units: %#v\nExpected: %#v", units, expectUnits) } - a.unloadUnit("foo.service") + err = a.unloadUnit("foo.service") + if err != nil { + t.Fatalf("Failed calling Agent.unloadUnit: %v", err) + } units, err = a.units() if err != nil { @@ -114,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 { @@ -132,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 308973860..d293218aa 100644 --- a/agent/task.go +++ b/agent/task.go @@ -104,11 +104,11 @@ func mapTaskToFunc(t task, a *Agent) (fn func() error, err error) { case taskTypeLoadUnit: fn = func() error { return a.loadUnit(t.unit) } case taskTypeUnloadUnit: - fn = func() error { a.unloadUnit(t.unit.Name); return nil } + 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: diff --git a/systemd/manager.go b/systemd/manager.go index a7b4dd542..ad9a82530 100644 --- a/systemd/manager.go +++ b/systemd/manager.go @@ -114,11 +114,11 @@ func (m *systemdUnitManager) Load(name string, u unit.UnitFile) error { // Unload removes the indicated unit from the filesystem, deletes its // associated Hash from the cache and clears its unit status in systemd -func (m *systemdUnitManager) Unload(name string) { +func (m *systemdUnitManager) Unload(name string) error { m.mutex.Lock() defer m.mutex.Unlock() delete(m.hashes, name) - m.removeUnit(name) + return m.removeUnit(name) } // TriggerStart asynchronously starts the unit identified by the given name. @@ -269,14 +269,31 @@ func (m *systemdUnitManager) writeUnit(name string, contents string) error { return err } -func (m *systemdUnitManager) removeUnit(name string) { +func (m *systemdUnitManager) removeUnit(name string) (err error) { log.Infof("Removing systemd unit %s", name) - m.systemd.DisableUnitFiles([]string{name}, true) - m.systemd.ResetFailedUnit(name) + // both DisableUnitFiles() and ResetFailedUnit() must be followed by + // removing the unit file. Otherwise "systemctl stop fleet" could end up + // hanging forever. + var errf error + func(name string) { + _, errf = m.systemd.DisableUnitFiles([]string{name}, true) + if errf != nil { + err = fmt.Errorf("%v, %v", err, errf) + } + }(name) + + func(name string) { + errf = m.systemd.ResetFailedUnit(name) + if errf != nil { + err = fmt.Errorf("%v, %v", err, errf) + } + }(name) ufPath := m.getUnitFilePath(name) os.Remove(ufPath) + + return err } func (m *systemdUnitManager) getUnitFilePath(name string) string { diff --git a/unit/fake.go b/unit/fake.go index 4a075764f..34bcd5b17 100644 --- a/unit/fake.go +++ b/unit/fake.go @@ -41,11 +41,12 @@ func (fum *FakeUnitManager) ReloadUnitFiles() error { return nil } -func (fum *FakeUnitManager) Unload(name string) { +func (fum *FakeUnitManager) Unload(name string) error { fum.Lock() defer fum.Unlock() delete(fum.u, name) + return nil } func (fum *FakeUnitManager) TriggerStart(string) error { return nil } diff --git a/unit/fake_test.go b/unit/fake_test.go index 60e89ce1e..a0590c66d 100644 --- a/unit/fake_test.go +++ b/unit/fake_test.go @@ -65,7 +65,10 @@ func TestFakeUnitManagerLoadUnload(t *testing.T) { t.Fatalf("Expected UnitState %v, got %v", eus, *us) } - fum.Unload("hello.service") + err = fum.Unload("hello.service") + if err != nil { + t.Fatalf("Expected no error from Unload(), got %v", err) + } units, err = fum.Units() if err != nil { diff --git a/unit/manager.go b/unit/manager.go index 40918c9da..b478005fb 100644 --- a/unit/manager.go +++ b/unit/manager.go @@ -20,7 +20,7 @@ import ( type UnitManager interface { Load(string, UnitFile) error - Unload(string) + Unload(string) error ReloadUnitFiles() error TriggerStart(string) error