Merge pull request #3831 from dgoodwin/revert-3825-master

Revert "daemon: Make binary writing idempotent"

pkg/daemon/daemon.go

@@ -450,54 +450,40 @@ func ReexecuteForTargetRoot(target string) error {
 			// Otherwise, we assume that there's no suffixing needed.  Hopefully
 			// by RHEL10 the MCD will have fundamentally changed and we won't be doing the
 			// chroot() thing anymore.
-			klog.Infof("not chrooting for source=rhel-%s target=rhel-%s", sourceMajor, targetMajor)
+			klog.Info("not chrooting for source=rhel-%s target=rhel-%s", sourceMajor, targetMajor)
 		}
 	} else {
 		klog.Info("assuming we can use container binary chroot() to host")
 	}
+	sourceBinary := "/usr/bin/machine-config-daemon" + sourceBinarySuffix
+	src, err := os.Open(sourceBinary)
+	if err != nil {
+		return fmt.Errorf("opening %s: %w", sourceBinary, err)
+	}
+	defer src.Close()
 
 	targetBinBase := "run/bin/machine-config-daemon"
 	targetBin := filepath.Join(target, targetBinBase)
+	targetBinDir := filepath.Dir(targetBin)
+	if _, err := os.Stat(targetBinDir); err != nil {
+		if err := os.Mkdir(targetBinDir, 0o755); err != nil {
+			return fmt.Errorf("mkdir %s: %w", targetBinDir, err)
+		}
+	}
 
-	// Be idempotent
-	targetBinExist, err := fileExists(targetBin)
+	f, err := os.Create(targetBin)
 	if err != nil {
-		return err
+		return fmt.Errorf("writing %s: %w", targetBin, err)
 	}
-	if !targetBinExist {
-		sourceBinary := "/usr/bin/machine-config-daemon" + sourceBinarySuffix
-		src, err := os.Open(sourceBinary)
-		if err != nil {
-			return fmt.Errorf("opening %s: %w", sourceBinary, err)
-		}
-		defer src.Close()
-
-		targetBinDir := filepath.Dir(targetBin)
-		// Before creating targetBinDir, ensure that it doesn't exist
-		targetBinDirExist, err := directoryExists(targetBinDir)
-		if err != nil {
-			return err
-		}
-		if !targetBinDirExist {
-			if err := os.Mkdir(targetBinDir, 0o755); err != nil {
-				return fmt.Errorf("mkdir %s: %w", targetBinDir, err)
-			}
-		}
-
-		f, err := os.Create(targetBin)
-		if err != nil {
-			return fmt.Errorf("writing %s: %w", targetBin, err)
-		}
-		if _, err := io.Copy(f, src); err != nil {
-			f.Close()
-			return fmt.Errorf("writing %s: %w", targetBin, err)
-		}
-		if err := f.Chmod(0o755); err != nil {
-			return err
-		}
-		// Must close our writable fd
+	if _, err := io.Copy(f, src); err != nil {
 		f.Close()
+		return fmt.Errorf("writing %s: %w", targetBin, err)
+	}
+	if err := f.Chmod(0o755); err != nil {
+		return err
 	}
+	// Must close our writable fd
+	f.Close()
 
 	if err := syscall.Chroot(target); err != nil {
 		return fmt.Errorf("failed to chroot to %s: %w", target, err)

pkg/daemon/update.go

@@ -1666,27 +1666,6 @@ func fileExists(path string) (bool, error) {
 	return false, fmt.Errorf("cannot stat file: %w", err)
 }
 
-// Determines if a directory exists by checking the returned error when we stat the file.
-// Also, check that it is a directory.
-func directoryExists(path string) (bool, error) {
-	info, err := os.Stat(path)
-	// If there is no error, check if it is a directory
-	if err == nil {
-		if info.IsDir() {
-			return true, nil
-		}
-		return false, fmt.Errorf("%s exists but it is not a directory", path)
-	}
-
-	// If the error matches fs.ErrNotExist, file definitely does not exist.
-	if errors.Is(err, fs.ErrNotExist) {
-		return false, nil
-	}
-
-	// An unexpected error occurred.
-	return false, fmt.Errorf("cannot stat file: %w", err)
-}
-
 // Removes the old SSH key path (/home/core/.ssh/authorized_keys), if found.
 func cleanSSHKeyPaths() error {
 	oldKeyExists, err := fileExists(constants.RHCOS8SSHKeyPath)