move systemd timer cleanup to oci hooks

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

Author: coderbirju

pkg/cmd/container/kill.go

@@ -35,7 +35,6 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/clientutil"
 	"github.com/containerd/nerdctl/v2/pkg/containerutil"
-	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/netutil"
@@ -113,9 +112,9 @@ func killContainer(ctx context.Context, container containerd.Container, signal s
 	}
 
 	// Clean up healthcheck systemd units
-	if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
-		log.G(ctx).Warnf("failed to clean up healthcheck units for container %s: %s", container.ID(), err)
-	}
+	// if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
+	// 	log.G(ctx).Warnf("failed to clean up healthcheck units for container %s: %s", container.ID(), err)
+	// }
 
 	// signal will be sent once resume is finished
 	if paused {

pkg/cmd/container/remove.go

@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"os"
 	"syscall"
 
@@ -181,9 +180,9 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions
 		retErr = nil
 
 		// Clean up healthcheck systemd units
-		if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, c); err != nil {
-			log.G(ctx).WithError(err).Warnf("failed to clean up healthcheck units for container %q", id)
-		}
+		// if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, c); err != nil {
+		// 	log.G(ctx).WithError(err).Warnf("failed to clean up healthcheck units for container %q", id)
+		// }
 
 		// Now, delete the actual container
 		var delOpts []containerd.DeleteOpts

pkg/containerutil/containerutil.go

@@ -362,9 +362,9 @@ func Stop(ctx context.Context, container containerd.Container, timeout *time.Dur
 	}()
 
 	// Clean up healthcheck units if configured.
-	if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
-		return fmt.Errorf("failed to clean up healthcheck units for container %s", container.ID())
-	}
+	// if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
+	// 	return fmt.Errorf("failed to clean up healthcheck units for container %s", container.ID())
+	// }
 
 	if timeout == nil {
 		t, ok := l[labels.StopTimeout]
@@ -505,9 +505,9 @@ func Pause(ctx context.Context, client *containerd.Client, id string) error {
 	}
 
 	// Clean up healthcheck units if configured.
-	if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
-		return fmt.Errorf("failed to clean up healthcheck units for container %s", container.ID())
-	}
+	// if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
+	// 	return fmt.Errorf("failed to clean up healthcheck units for container %s", container.ID())
+	// }
 
 	switch status.Status {
 	case containerd.Paused:

pkg/healthcheck/healthcheck_manager_linux.go

@@ -118,15 +118,25 @@ func RemoveTransientHealthCheckFiles(ctx context.Context, container containerd.C
 		return nil
 	}
 
-	fmt.Printf("⏱️  Removing healthcheck timer unit: %s\n", container.ID()) // TODO remove this later
+	return RemoveTransientHealthCheckFilesByID(ctx, container.ID())
+}
+
+// RemoveTransientHealthCheckFilesByID stops and cleans up the transient timer and service using just the container ID.
+func RemoveTransientHealthCheckFilesByID(ctx context.Context, containerID string) error {
+	// Don't proceed if systemd is unavailable or disabled
+	if !defaults.IsSystemdAvailable() || os.Getenv("DISABLE_HC_SYSTEMD") == "true" {
+		return nil
+	}
+
+	fmt.Printf("⏱️  Removing healthcheck timer unit: %s\n", containerID) // TODO remove this later
 
 	conn, err := dbus.NewSystemConnectionContext(context.Background())
 	if err != nil {
 		return fmt.Errorf("systemd DBUS connect error: %w", err)
 	}
 	defer conn.Close()
 
-	unitName := hcUnitName(container.ID(), true)
+	unitName := hcUnitName(containerID, true)
 	timer := unitName + ".timer"
 	service := unitName + ".service"
 

pkg/ocihook/ocihook.go

@@ -39,6 +39,7 @@ import (
 
 	"github.com/containerd/nerdctl/v2/pkg/bypass4netnsutil"
 	"github.com/containerd/nerdctl/v2/pkg/dnsutil/hostsstore"
+	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/namestore"
@@ -560,8 +561,11 @@ func onCreateRuntime(opts *handlerOpts) error {
 }
 
 func onPostStop(opts *handlerOpts) error {
+	log.L.Debugf("onPostStop hook triggered for container %s (namespace: %s)", opts.state.ID, opts.state.Annotations[labels.Namespace])
+
 	lf, err := state.New(opts.state.Annotations[labels.StateDir])
 	if err != nil {
+		log.L.WithError(err).Errorf("failed to create state store for container %s", opts.state.ID)
 		return err
 	}
 
@@ -585,6 +589,13 @@ func onPostStop(opts *handlerOpts) error {
 
 	ctx := context.Background()
 	ns := opts.state.Annotations[labels.Namespace]
+
+	if err := healthcheck.RemoveTransientHealthCheckFilesByID(ctx, opts.state.ID); err != nil {
+		log.L.WithError(err).Warnf("failed to clean up healthcheck units for container %s", opts.state.ID)
+	} else {
+		log.L.Infof("successfully cleaned up healthcheck files for container %s", opts.state.ID)
+	}
+
 	if opts.cni != nil {
 		var err error
 		b4nnEnabled, b4nnBindEnabled, err := bypass4netnsutil.IsBypass4netnsEnabled(opts.state.Annotations)