Merge pull request #44266 from thaJeztah/pidfile_cleanup

pkg/pidfile: remove PIDFile type and cleanup

cmd/dockerd/daemon.go

@@ -139,13 +139,12 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 	potentiallyUnderRuntimeDir := []string{cli.Config.ExecRoot}
 
 	if cli.Pidfile != "" {
-		pf, err := pidfile.New(cli.Pidfile)
-		if err != nil {
+		if err := pidfile.Write(cli.Pidfile); err != nil {
 			return errors.Wrap(err, "failed to start daemon")
 		}
 		potentiallyUnderRuntimeDir = append(potentiallyUnderRuntimeDir, cli.Pidfile)
 		defer func() {
-			if err := pf.Remove(); err != nil {
+			if err := os.Remove(cli.Pidfile); err != nil {
 				logrus.Error(err)
 			}
 		}()

pkg/pidfile/pidfile.go

@@ -4,49 +4,39 @@
 package pidfile // import "github.com/docker/docker/pkg/pidfile"
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strconv"
-	"strings"
 
 	"github.com/docker/docker/pkg/system"
 )
 
-// PIDFile is a file used to store the process ID of a running process.
-type PIDFile struct {
-	path string
-}
-
 func checkPIDFileAlreadyExists(path string) error {
-	if pidByte, err := os.ReadFile(path); err == nil {
-		pidString := strings.TrimSpace(string(pidByte))
-		if pid, err := strconv.Atoi(pidString); err == nil {
-			if processExists(pid) {
-				return fmt.Errorf("pid file found, ensure docker is not running or delete %s", path)
-			}
+	pidByte, err := os.ReadFile(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
 		}
+		return err
+	}
+	pid, err := strconv.Atoi(string(bytes.TrimSpace(pidByte)))
+	if err == nil && processExists(pid) {
+		return fmt.Errorf("pid file found, ensure docker is not running or delete %s", path)
 	}
 	return nil
 }
 
-// New creates a PIDfile using the specified path.
-func New(path string) (*PIDFile, error) {
+// Write writes a "PID file" at the specified path. It returns an error if the
+// file exists and contains a valid PID of a running process, or when failing
+// to write the file.
+func Write(path string) error {
 	if err := checkPIDFileAlreadyExists(path); err != nil {
-		return nil, err
+		return err
 	}
-	// Note MkdirAll returns nil if a directory already exists
-	if err := system.MkdirAll(filepath.Dir(path), os.FileMode(0755)); err != nil {
-		return nil, err
+	if err := system.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		return err
 	}
-	if err := os.WriteFile(path, []byte(fmt.Sprintf("%d", os.Getpid())), 0644); err != nil {
-		return nil, err
-	}
-
-	return &PIDFile{path: path}, nil
-}
-
-// Remove removes the PIDFile.
-func (file PIDFile) Remove() error {
-	return os.Remove(file.path)
+	return os.WriteFile(path, []byte(strconv.Itoa(os.Getpid())), 0o644)
 }

pkg/pidfile/pidfile_test.go

@@ -1,37 +1,19 @@
 package pidfile // import "github.com/docker/docker/pkg/pidfile"
 
 import (
-	"os"
 	"path/filepath"
 	"testing"
 )
 
-func TestNewAndRemove(t *testing.T) {
-	dir, err := os.MkdirTemp(os.TempDir(), "test-pidfile")
-	if err != nil {
-		t.Fatal("Could not create test directory")
-	}
-
-	path := filepath.Join(dir, "testfile")
-	file, err := New(path)
+func TestWrite(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "testfile")
+	err := Write(path)
 	if err != nil {
 		t.Fatal("Could not create test file", err)
 	}
 
-	_, err = New(path)
+	err = Write(path)
 	if err == nil {
 		t.Fatal("Test file creation not blocked")
 	}
-
-	if err := file.Remove(); err != nil {
-		t.Fatal("Could not delete created test file")
-	}
-}
-
-func TestRemoveInvalidPath(t *testing.T) {
-	file := PIDFile{path: filepath.Join("foo", "bar")}
-
-	if err := file.Remove(); err == nil {
-		t.Fatal("Non-existing file doesn't give an error on delete")
-	}
 }