Merge pull request #4362 from apostasie/2025-06-remove-lock

Simplify healthcheck file handling + fix locking

Author: AkihiroSuda

pkg/healthcheck/log.go

@@ -47,21 +47,22 @@ func writeHealthLog(ctx context.Context, container containerd.Container, result
 		return fmt.Errorf("failed to marshal health log: %w", err)
 	}
 
-	// Ensure file exists before writing
-	if err := ensureHealthLogFile(stateDir); err != nil {
-		return err
-	}
-
 	// Write the latest result to the file
 	logPath := filepath.Join(stateDir, HealthLogFilename)
-	return filesystem.WithAppendLock(logPath, func(file *os.File) error {
-		if _, err := file.Seek(0, io.SeekEnd); err != nil {
+	return filesystem.WithLock(stateDir, func() error {
+		file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY, 0o600)
+		if err != nil {
+			return err
+		}
+		defer file.Close()
+		if _, err = file.Seek(0, io.SeekEnd); err != nil {
 			return fmt.Errorf("seek error: %w", err)
 		}
-		if _, err := file.Write(append([]byte(data), '\n')); err != nil {
+		if _, err = file.Write(append([]byte(data), '\n')); err != nil {
 			return fmt.Errorf("failed to write health log: %w", err)
 		}
-		return nil
+
+		return file.Sync()
 	})
 }
 
@@ -182,23 +183,6 @@ func readHealthStateFromLabels(ctx context.Context, container containerd.Contain
 	return state, nil
 }
 
-// ensureHealthLogFile creates the health.json file if it doesn't exist.
-func ensureHealthLogFile(stateDir string) error {
-	healthLogPath := filepath.Join(stateDir, HealthLogFilename)
-
-	// Ensure container state directory exists
-	if _, err := os.Stat(stateDir); os.IsNotExist(err) {
-		return fmt.Errorf("container state directory does not exist: %s", stateDir)
-	}
-
-	// Create health.json if it doesn't exist
-	if _, err := os.Stat(healthLogPath); os.IsNotExist(err) {
-		return filesystem.WriteFile(healthLogPath, []byte{}, 0600)
-	}
-
-	return nil
-}
-
 // getContainerStateDir returns the container's state directory from labels.
 func getContainerStateDir(ctx context.Context, container containerd.Container) (string, error) {
 	info, err := container.Info(ctx)

pkg/internal/filesystem/lock.go

@@ -37,18 +37,18 @@ import (
 // If Lock returns nil, no other process will be able to place a read or write lock on the file until
 // this process exits, closes f, or calls Unlock on it.
 func Lock(path string) (file *os.File, err error) {
-	return commonlock(path, writeLock, false)
+	return commonlock(path, writeLock)
 }
 
 // ReadOnlyLock places an advisory read lock on the file, blocking until it can be locked.
 //
 // If ReadOnlyLock returns nil, no other process will be able to place a write lock on
 // the file until this process exits, closes f, or calls Unlock on it.
 func ReadOnlyLock(path string) (file *os.File, err error) {
-	return commonlock(path, readLock, false)
+	return commonlock(path, readLock)
 }
 
-func commonlock(path string, mode lockType, appendMode bool) (file *os.File, err error) {
+func commonlock(path string, mode lockType) (file *os.File, err error) {
 	defer func() {
 		if err != nil {
 			err = errors.Join(ErrLockFail, err, file.Close())
@@ -65,21 +65,12 @@ func commonlock(path string, mode lockType, appendMode bool) (file *os.File, err
 		}
 	}
 
-	// For append mode, open with specific flags
-	if appendMode {
-		file, err = os.OpenFile(path, os.O_WRONLY, privateFilePermission)
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		// Preserve the original behavior for non-append operations
-		file, err = os.Open(path)
-		if errors.Is(err, os.ErrNotExist) {
-			file, err = os.OpenFile(path, os.O_RDONLY|os.O_CREATE, privateFilePermission)
-		}
-		if err != nil {
-			return nil, err
-		}
+	file, err = os.Open(path)
+	if errors.Is(err, os.ErrNotExist) {
+		file, err = os.OpenFile(path, os.O_RDONLY|os.O_CREATE, privateFilePermission)
+	}
+	if err != nil {
+		return nil, err
 	}
 
 	if err = platformSpecificLock(file, mode); err != nil {
@@ -131,17 +122,3 @@ func WithReadOnlyLock(path string, function func() error) (err error) {
 
 	return function()
 }
-
-// WithAppendLock executes the function with a file opened in append mode
-func WithAppendLock(path string, function func(file *os.File) error) (err error) {
-	file, err := commonlock(path, writeLock, true)
-	if err != nil {
-		return err
-	}
-
-	defer func() {
-		err = errors.Join(Unlock(file), err)
-	}()
-
-	return function(file)
-}