Merge pull request #3932 from kolyshkin/1.1-3888

[1.1] runc delete: call systemd's reset-failed
opencontainers · Jul 12, 2023 · 828eb47 · 828eb47
2 parents ca73c9f + ef6491e
commit 828eb47
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 9 deletions.
diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go
@@ -370,7 +370,10 @@ retry:
 			// In case a unit with the same name exists, this may
 			// be a leftover failed unit. Reset it, so systemd can
 			// remove it, and retry once.
-			resetFailedUnit(cm, unitName)
+			err = resetFailedUnit(cm, unitName)
+			if err != nil {
+				logrus.Warnf("unable to reset failed unit: %v", err)
+			}
 			retry = false
 			goto retry
 		}
@@ -385,11 +388,11 @@ retry:
 		close(statusChan)
 		// Please refer to https://pkg.go.dev/github.com/coreos/go-systemd/v22/dbus#Conn.StartUnit
 		if s != "done" {
-			resetFailedUnit(cm, unitName)
+			_ = resetFailedUnit(cm, unitName)
 			return fmt.Errorf("error creating systemd unit `%s`: got `%s`", unitName, s)
 		}
 	case <-timeout.C:
-		resetFailedUnit(cm, unitName)
+		_ = resetFailedUnit(cm, unitName)
 		return errors.New("Timeout waiting for systemd to create " + unitName)
 	}
 
@@ -417,16 +420,17 @@ func stopUnit(cm *dbusConnManager, unitName string) error {
 			return errors.New("Timed out while waiting for systemd to remove " + unitName)
 		}
 	}
+
+	// In case of a failed unit, let systemd remove it.
+	_ = resetFailedUnit(cm, unitName)
+
 	return nil
 }
 
-func resetFailedUnit(cm *dbusConnManager, name string) {
-	err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) error {
+func resetFailedUnit(cm *dbusConnManager, name string) error {
+	return cm.retryOnDisconnect(func(c *systemdDbus.Conn) error {
 		return c.ResetFailedUnitContext(context.TODO(), name)
 	})
-	if err != nil {
-		logrus.Warnf("unable to reset failed unit: %v", err)
-	}
 }
 
 func getUnitTypeProperty(cm *dbusConnManager, unitName string, unitType string, propertyName string) (*systemdDbus.Property, error) {

diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats
@@ -233,7 +233,6 @@ function setup() {
 	set_cgroups_path
 	# CPU shares of 3333 corresponds to CPU weight of 128.
 	update_config '   .linux.resources.memory |= {"limit": 33554432}
-			| .linux.resources.memorySwap |= {"limit": 33554432}
 			| .linux.resources.cpu |= {
 				"shares": 3333,
 				"quota": 40000,

diff --git a/tests/integration/delete.bats b/tests/integration/delete.bats
@@ -168,3 +168,30 @@ EOF
 	# check delete subcgroups success
 	[ ! -d "$CGROUP_PATH"/foo ]
 }
+
+@test "runc delete removes failed systemd unit" {
+	requires systemd_v244 # Older systemd lacks RuntimeMaxSec support.
+
+	set_cgroups_path
+	# shellcheck disable=SC2016
+	update_config '	  .annotations += {
+				"org.systemd.property.RuntimeMaxSec": "2",
+				"org.systemd.property.TimeoutStopSec": "1"
+			   }
+			| .process.args |= ["/bin/sleep", "10"]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test-failed-unit
+	[ "$status" -eq 0 ]
+
+	wait_for_container 10 1 test-failed-unit stopped
+
+	local user=""
+	[ $EUID -ne 0 ] && user="--user"
+
+	# Expect "unit is not active" exit code.
+	run -3 systemctl status $user "$SD_UNIT_NAME"
+
+	runc delete test-failed-unit
+	# Expect "no such unit" exit code.
+	run -4 systemctl status $user "$SD_UNIT_NAME"
+}
diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash
@@ -441,6 +441,12 @@ function requires() {
 				skip_me=1
 			fi
 			;;
+		systemd_v*)
+			var=${var#systemd_v}
+			if [ "$(systemd_version)" -lt "$var" ]; then
+				skip "requires systemd >= v${var}"
+			fi
+			;;
 		no_systemd)
 			if [ -n "${RUNC_USE_SYSTEMD}" ]; then
 				skip_me=1