libct/cg/sd: return error from stopUnit

Historically, we never returned an error from failed startUnit
or stopUnit. The startUnit case was fixed by commit 3844789.

It is time to fix stopUnit, too. The reasons are:

1. Ignoring an error from stopUnit means an unexpected trouble down the
   road, for example a failure to create a container with the same name:

   > time="2021-05-07T19:51:27Z" level=error msg="container_linux.go:380: starting container process caused: process_linux.go:385: applying cgroup configuration for process caused: Unit runc-test_busybox.scope already exists."

2. A somewhat short timeout of 1 second means the cgroup might
   actually be removed a few seconds later but we might have a
   race between removing the cgroup and creating another one
   with the same name, resulting in the same error as amove.

So, return an error if removal failed, and increase the timeout.

Now, modify the systemd cgroup v1 manager to not mask the error from
stopUnit (stopErr) with the subsequent one from cgroups.RemovePath,
as stopErr is most probably the reason why RemovePath failed.

Note that for v1 we do want to remove the paths even in case of a
failure from stopUnit, as some were not created by systemd.
There's no need to do that for v2, thanks to unified hierarchy,
so no changes there.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

libcontainer/cgroups/systemd/common.go

@@ -355,6 +355,9 @@ func stopUnit(cm *dbusConnManager, unitName string) error {
 		return err
 	})
 	if err == nil {
+		timeout := time.NewTimer(30 * time.Second)
+		defer timeout.Stop()
+
 		select {
 		case s := <-statusChan:
 			close(statusChan)
@@ -362,8 +365,8 @@ func stopUnit(cm *dbusConnManager, unitName string) error {
 			if s != "done" {
 				logrus.Warnf("error removing unit `%s`: got `%s`. Continuing...", unitName, s)
 			}
-		case <-time.After(time.Second):
-			logrus.Warnf("Timed out while waiting for StopUnit(%s) completion signal from dbus. Continuing...", unitName)
+		case <-timeout.C:
+			return errors.New("Timed out while waiting for systemd to remove " + unitName)
 		}
 	}
 	return nil

libcontainer/cgroups/systemd/v1.go

@@ -207,9 +207,10 @@ func (m *legacyManager) Destroy() error {
 
 	stopErr := stopUnit(m.dbus, getUnitName(m.cgroups))
 
-	// Both on success and on error, cleanup all the cgroups we are aware of.
-	// Some of them were created directly by Apply() and are not managed by systemd.
-	if err := cgroups.RemovePaths(m.paths); err != nil {
+	// Both on success and on error, cleanup all the cgroups
+	// we are aware of, as some of them were created directly
+	// by Apply() and are not managed by systemd.
+	if err := cgroups.RemovePaths(m.paths); err != nil && stopErr == nil {
 		return err
 	}