check that StartTransientUnit and StopUnit succeeds

Signed-off-by: lifubang <[email protected]>
opencontainers · Apr 25, 2020 · b0fffaa · b0fffaa
1 parent b19f9ce
commit b0fffaa
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 42 deletions.
diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go
@@ -5,10 +5,13 @@ import (
 	"os"
 	"strings"
 	"sync"
+	"time"
 
 	systemdDbus "github.com/coreos/go-systemd/v22/dbus"
 	dbus "github.com/godbus/dbus/v5"
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 var (
@@ -99,3 +102,51 @@ func isUnitExists(err error) bool {
 	}
 	return false
 }
+
+func startUnit(unitName string, properties []systemdDbus.Property) error {
+	dbusConnection, err := getDbusConnection()
+	if err != nil {
+		return err
+	}
+
+	statusChan := make(chan string, 1)
+	if _, err := dbusConnection.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
+		select {
+		case s := <-statusChan:
+			close(statusChan)
+			// Please refer to https://godoc.org/github.com/coreos/go-systemd/dbus#Conn.StartUnit
+			if s != "done" {
+				dbusConnection.ResetFailedUnit(unitName)
+				return errors.Errorf("error creating systemd unit `%s`: got `%s`", unitName, s)
+			}
+		case <-time.After(time.Second):
+			logrus.Warnf("Timed out while waiting for StartTransientUnit(%s) completion signal from dbus. Continuing...", unitName)
+		}
+	} else if !isUnitExists(err) {
+		return err
+	}
+
+	return nil
+}
+
+func stopUnit(unitName string) error {
+	dbusConnection, err := getDbusConnection()
+	if err != nil {
+		return err
+	}
+
+	statusChan := make(chan string, 1)
+	if _, err := dbusConnection.StopUnit(unitName, "replace", statusChan); err == nil {
+		select {
+		case s := <-statusChan:
+			close(statusChan)
+			// Please refer to https://godoc.org/github.com/coreos/go-systemd/dbus#Conn.StartUnit
+			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)
+		}
+	}
+	return nil
+}
diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go
@@ -10,13 +10,11 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
-	"time"
 
 	systemdDbus "github.com/coreos/go-systemd/v22/dbus"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fs"
 	"github.com/opencontainers/runc/libcontainer/configs"
-	"github.com/sirupsen/logrus"
 )
 
 type LegacyManager struct {
@@ -184,18 +182,7 @@ func (m *LegacyManager) Apply(pid int) error {
 	properties = append(properties, resourcesProperties...)
 	properties = append(properties, c.SystemdProps...)
 
-	dbusConnection, err := getDbusConnection()
-	if err != nil {
-		return err
-	}
-	statusChan := make(chan string, 1)
-	if _, err := dbusConnection.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
-		select {
-		case <-statusChan:
-		case <-time.After(time.Second):
-			logrus.Warnf("Timed out while waiting for StartTransientUnit(%s) completion signal from dbus. Continuing...", unitName)
-		}
-	} else if !isUnitExists(err) {
+	if err := startUnit(unitName, properties); err != nil {
 		return err
 	}
 
@@ -226,14 +213,9 @@ func (m *LegacyManager) Destroy() error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	dbusConnection, err := getDbusConnection()
-	if err != nil {
-		return err
-	}
-
-	dbusConnection.StopUnit(getUnitName(m.Cgroups), "replace", nil)
-	if err := cgroups.RemovePaths(m.Paths); err != nil {
-		return err
+	unitName := getUnitName(m.Cgroups)
+	if err := stopUnit(unitName); err != nil {
+		return nil
 	}
 	m.Paths = make(map[string]string)
 	return nil

diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go
@@ -8,14 +8,12 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
-	"time"
 
 	systemdDbus "github.com/coreos/go-systemd/v22/dbus"
 	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fs2"
 	"github.com/opencontainers/runc/libcontainer/configs"
-	"github.com/sirupsen/logrus"
 )
 
 type unifiedManager struct {
@@ -142,19 +140,7 @@ func (m *unifiedManager) Apply(pid int) error {
 	properties = append(properties, resourcesProperties...)
 	properties = append(properties, c.SystemdProps...)
 
-	dbusConnection, err := getDbusConnection()
-	if err != nil {
-		return err
-	}
-
-	statusChan := make(chan string, 1)
-	if _, err := dbusConnection.StartTransientUnit(unitName, "replace", properties, statusChan); err == nil {
-		select {
-		case <-statusChan:
-		case <-time.After(time.Second):
-			logrus.Warnf("Timed out while waiting for StartTransientUnit(%s) completion signal from dbus. Continuing...", unitName)
-		}
-	} else if !isUnitExists(err) {
+	if err := startUnit(unitName, properties); err != nil {
 		return err
 	}
 
@@ -175,14 +161,13 @@ func (m *unifiedManager) Destroy() error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	dbusConnection, err := getDbusConnection()
-	if err != nil {
-		return err
+	unitName := getUnitName(m.cgroups)
+	if err := stopUnit(unitName); err != nil {
+		return nil
 	}
-	dbusConnection.StopUnit(getUnitName(m.cgroups), "replace", nil)
 
 	// XXX this is probably not needed, systemd should handle it
-	err = os.Remove(m.path)
+	err := os.Remove(m.path)
 	if err != nil && !os.IsNotExist(err) {
 		return err
 	}