add non blocking systemd removal

Signed-off-by: Arjun Raja Yogidas <[email protected]>

Author: coderbirju

cmd/nerdctl/container/container_health_check_linux_test.go

@@ -695,8 +695,6 @@ func TestHealthCheck_SystemdIntegration_Advanced(t *testing.T) {
 					"--health-interval", "1s",
 					testutil.CommonImage, "sleep", "30")
 				nerdtest.EnsureContainerStarted(helpers, data.Identifier())
-				// Wait longer for systemd timer creation and first healthcheck execution
-				time.Sleep(3 * time.Second)
 			},
 			Cleanup: func(data test.Data, helpers test.Helpers) {
 				helpers.Anyhow("rm", "-f", data.Identifier())
@@ -724,7 +722,6 @@ func TestHealthCheck_SystemdIntegration_Advanced(t *testing.T) {
 						})
 						// Stop container and verify cleanup
 						helpers.Ensure("stop", data.Identifier())
-						time.Sleep(500 * time.Millisecond) // Allow cleanup to complete
 
 						// Check that timer is gone
 						result = helpers.Custom("systemctl", "list-timers", "--all", "--no-pager")
@@ -748,7 +745,6 @@ func TestHealthCheck_SystemdIntegration_Advanced(t *testing.T) {
 					"--health-interval", "2s",
 					testutil.CommonImage, "sleep", "60")
 				nerdtest.EnsureContainerStarted(helpers, data.Identifier())
-				time.Sleep(3 * time.Second) // Wait for initial timer creation
 			},
 			Cleanup: func(data test.Data, helpers test.Helpers) {
 				helpers.Anyhow("rm", "-f", data.Identifier())
@@ -770,7 +766,6 @@ func TestHealthCheck_SystemdIntegration_Advanced(t *testing.T) {
 
 				// Step 2: Stop container
 				helpers.Ensure("stop", data.Identifier())
-				time.Sleep(1 * time.Second) // Allow cleanup
 
 				// Step 3: Verify timer is removed after stop
 				result = helpers.Custom("systemctl", "list-timers", "--all", "--no-pager")
@@ -785,7 +780,6 @@ func TestHealthCheck_SystemdIntegration_Advanced(t *testing.T) {
 				// Step 4: Restart container
 				helpers.Ensure("start", data.Identifier())
 				nerdtest.EnsureContainerStarted(helpers, data.Identifier())
-				time.Sleep(3 * time.Second) // Wait for timer recreation
 
 				// Step 5: Verify timer is recreated after restart - this is our final verification
 				return helpers.Custom("systemctl", "list-timers", "--all", "--no-pager")

pkg/healthcheck/healthcheck_manager_darwin.go

@@ -38,6 +38,16 @@ func RemoveTransientHealthCheckFiles(ctx context.Context, container containerd.C
 }
 
 // RemoveTransientHealthCheckFilesByID stops and cleans up the transient timer and service using just the container ID.
+// This function is deprecated and no longer used. Use ForceRemoveTransientHealthCheckFiles instead.
+/*
 func RemoveTransientHealthCheckFilesByID(ctx context.Context, containerID string) error {
 	return nil
 }
+*/
+
+// ForceRemoveTransientHealthCheckFiles forcefully stops and cleans up the transient timer and service
+// using just the container ID. This function is non-blocking and uses timeouts to prevent hanging
+// on systemd operations. On Darwin, this is a no-op since systemd is not available.
+func ForceRemoveTransientHealthCheckFiles(ctx context.Context, containerID string) error {
+	return nil
+}

pkg/healthcheck/healthcheck_manager_linux.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"time"
 
 	"github.com/coreos/go-systemd/v22/dbus"
 
@@ -110,14 +111,13 @@ func RemoveTransientHealthCheckFiles(ctx context.Context, container containerd.C
 	if hc == nil {
 		return nil
 	}
-	if shouldSkipHealthCheckSystemd(hc) {
-		return nil
-	}
 
-	return RemoveTransientHealthCheckFilesByID(ctx, container.ID())
+	return ForceRemoveTransientHealthCheckFiles(ctx, container.ID())
 }
 
 // RemoveTransientHealthCheckFilesByID stops and cleans up the transient timer and service using just the container ID.
+// This function is deprecated and no longer used. Use ForceRemoveTransientHealthCheckFiles instead.
+/*
 func RemoveTransientHealthCheckFilesByID(ctx context.Context, containerID string) error {
 	log.G(ctx).Debugf("Removing healthcheck timer unit: %s", containerID)
 
@@ -151,6 +151,120 @@ func RemoveTransientHealthCheckFilesByID(ctx context.Context, containerID string
 	_ = conn.ResetFailedUnitContext(context.Background(), service)
 	return nil
 }
+*/
+
+// ForceRemoveTransientHealthCheckFiles forcefully stops and cleans up the transient timer and service
+// using just the container ID. This function is non-blocking and uses timeouts to prevent hanging
+// on systemd operations. It logs errors as warnings but continues cleanup attempts.
+func ForceRemoveTransientHealthCheckFiles(ctx context.Context, containerID string) error {
+	log.G(ctx).Debugf("Force removing healthcheck timer unit: %s", containerID)
+
+	// Create a timeout context for systemd operations (5 seconds default)
+	timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+	defer cancel()
+
+	unitName := hcUnitName(containerID, true)
+	timer := unitName + ".timer"
+	service := unitName + ".service"
+
+	// Channel to collect any critical errors (though we'll continue cleanup regardless)
+	errChan := make(chan error, 3)
+
+	// Goroutine for DBUS connection and cleanup operations
+	go func() {
+		defer close(errChan)
+
+		conn, err := dbus.NewSystemConnectionContext(timeoutCtx)
+		if err != nil {
+			log.G(ctx).Warnf("systemd DBUS connect error during force cleanup: %v", err)
+			errChan <- fmt.Errorf("systemd DBUS connect error: %w", err)
+			return
+		}
+		defer conn.Close()
+
+		// Stop timer with timeout
+		go func() {
+			select {
+			case <-timeoutCtx.Done():
+				log.G(ctx).Warnf("timeout stopping timer %s during force cleanup", timer)
+				return
+			default:
+				tChan := make(chan string, 1)
+				if _, err := conn.StopUnitContext(timeoutCtx, timer, "ignore-dependencies", tChan); err == nil {
+					select {
+					case msg := <-tChan:
+						if msg != "done" {
+							log.G(ctx).Warnf("timer stop message during force cleanup: %s", msg)
+						}
+					case <-timeoutCtx.Done():
+						log.G(ctx).Warnf("timeout waiting for timer stop confirmation: %s", timer)
+					}
+				} else {
+					log.G(ctx).Warnf("failed to stop timer %s during force cleanup: %v", timer, err)
+				}
+			}
+		}()
+
+		// Stop service with timeout
+		go func() {
+			select {
+			case <-timeoutCtx.Done():
+				log.G(ctx).Warnf("timeout stopping service %s during force cleanup", service)
+				return
+			default:
+				sChan := make(chan string, 1)
+				if _, err := conn.StopUnitContext(timeoutCtx, service, "ignore-dependencies", sChan); err == nil {
+					select {
+					case msg := <-sChan:
+						if msg != "done" {
+							log.G(ctx).Warnf("service stop message during force cleanup: %s", msg)
+						}
+					case <-timeoutCtx.Done():
+						log.G(ctx).Warnf("timeout waiting for service stop confirmation: %s", service)
+					}
+				} else {
+					log.G(ctx).Warnf("failed to stop service %s during force cleanup: %v", service, err)
+				}
+			}
+		}()
+
+		// Reset failed units (best effort, non-blocking)
+		go func() {
+			select {
+			case <-timeoutCtx.Done():
+				log.G(ctx).Warnf("timeout resetting failed unit %s during force cleanup", service)
+				return
+			default:
+				if err := conn.ResetFailedUnitContext(timeoutCtx, service); err != nil {
+					log.G(ctx).Warnf("failed to reset failed unit %s during force cleanup: %v", service, err)
+				}
+			}
+		}()
+
+		// Wait a short time for operations to complete, but don't block indefinitely
+		select {
+		case <-time.After(3 * time.Second):
+			log.G(ctx).Debugf("force cleanup operations completed for container %s", containerID)
+		case <-timeoutCtx.Done():
+			log.G(ctx).Warnf("force cleanup timed out for container %s", containerID)
+		}
+	}()
+
+	// Wait for the cleanup goroutine to finish or timeout
+	select {
+	case err := <-errChan:
+		if err != nil {
+			log.G(ctx).Warnf("force cleanup encountered errors but continuing: %v", err)
+		}
+	case <-timeoutCtx.Done():
+		log.G(ctx).Warnf("force cleanup timed out for container %s, but cleanup may continue in background", containerID)
+	}
+
+	// Always return nil - this function should never block the caller
+	// even if systemd operations fail or timeout
+	log.G(ctx).Debugf("force cleanup completed (non-blocking) for container %s", containerID)
+	return nil
+}
 
 // hcUnitName returns a systemd unit name for a container healthcheck.
 func hcUnitName(containerID string, bare bool) string {

pkg/healthcheck/healthcheck_manager_windows.go

@@ -38,6 +38,16 @@ func RemoveTransientHealthCheckFiles(ctx context.Context, container containerd.C
 }
 
 // RemoveTransientHealthCheckFilesByID stops and cleans up the transient timer and service using just the container ID.
+// This function is deprecated and no longer used. Use ForceRemoveTransientHealthCheckFiles instead.
+/*
 func RemoveTransientHealthCheckFilesByID(ctx context.Context, containerID string) error {
 	return nil
 }
+*/
+
+// ForceRemoveTransientHealthCheckFiles forcefully stops and cleans up the transient timer and service
+// using just the container ID. This function is non-blocking and uses timeouts to prevent hanging
+// on systemd operations. On Windows, this is a no-op since systemd is not available.
+func ForceRemoveTransientHealthCheckFiles(ctx context.Context, containerID string) error {
+	return nil
+}