Merge pull request #3888 from kolyshkin/reset-failed

runc delete: call systemd's reset-failed

libcontainer/cgroups/systemd/common.go

@@ -147,7 +147,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
 		}
@@ -162,11 +165,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)
 	}
 
@@ -194,16 +197,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) {

tests/integration/cgroups.bats

@@ -245,7 +245,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,

tests/integration/delete.bats

@@ -168,3 +168,30 @@ EOF
 	# check delete subcgroups success
 	[ ! -d "$CGROUP_V2_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"
+}

tests/integration/helpers.bash

@@ -460,6 +460,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 [ -v RUNC_USE_SYSTEMD ]; then
 				skip_me=1

tests/integration/update.bats

@@ -474,11 +474,8 @@ EOF
 }
 
 @test "update cgroup cpu.idle via systemd v252+" {
-	requires cgroups_v2 systemd cgroups_cpu_idle
+	requires cgroups_v2 systemd_v252 cgroups_cpu_idle
 	[ $EUID -ne 0 ] && requires rootless_cgroup
-	if [ "$(systemd_version)" -lt 252 ]; then
-		skip "requires systemd >= v252"
-	fi
 
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_update
 	[ "$status" -eq 0 ]