From bfa1b2aab39f7735db9319478b8f63b9adca26e3 Mon Sep 17 00:00:00 2001 From: lifubang Date: Sun, 26 Apr 2020 07:23:55 +0800 Subject: [PATCH] check that StartTransientUnit and StopUnit succeeds Signed-off-by: lifubang --- libcontainer/cgroups/systemd/common.go | 51 ++++++++++++++++++++++++++ libcontainer/cgroups/systemd/v1.go | 24 ++---------- libcontainer/cgroups/systemd/v2.go | 23 ++---------- 3 files changed, 58 insertions(+), 40 deletions(-) diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go index eeb0d532..5c9b4cbe 100644 --- 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 index af3ca798..8bac7ca2 100644 --- 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,13 +213,8 @@ 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 { + unitName := getUnitName(m.Cgroups) + if err := stopUnit(unitName); err != nil { return err } m.Paths = make(map[string]string) diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 3a8adfd9..77f7696e 100644 --- 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 { + unitName := getUnitName(m.cgroups) + if err := stopUnit(unitName); err != nil { return err } - 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 }