Browse Source

idtools don't chown if not needed

In some cases (e.g. NFS), a chown may technically be a no-op but still
return `EPERM`, so only call `chown` when neccessary.

This is particularly problematic for docker users bind-mounting an NFS
share into a container.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Brian Goff 7 years ago
parent
commit
fa9709a3fc
1 changed files with 30 additions and 8 deletions
  1. 30 8
      pkg/idtools/idtools_unix.go

+ 30 - 8
pkg/idtools/idtools_unix.go

@@ -26,14 +26,19 @@ func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chown
 	// so that we can chown all of them properly at the end.  If chownExisting is false, we won't
 	// so that we can chown all of them properly at the end.  If chownExisting is false, we won't
 	// chown the full directory path if it exists
 	// chown the full directory path if it exists
 	var paths []string
 	var paths []string
-	if _, err := os.Stat(path); err != nil && os.IsNotExist(err) {
-		paths = []string{path}
-	} else if err == nil && chownExisting {
+
+	stat, err := system.Stat(path)
+	if err == nil {
+		if !chownExisting {
+			return nil
+		}
+
 		// short-circuit--we were called with an existing directory and chown was requested
 		// short-circuit--we were called with an existing directory and chown was requested
-		return os.Chown(path, ownerUID, ownerGID)
-	} else if err == nil {
-		// nothing to do; directory path fully exists already and chown was NOT requested
-		return nil
+		return lazyChown(path, ownerUID, ownerGID, stat)
+	}
+
+	if os.IsNotExist(err) {
+		paths = []string{path}
 	}
 	}
 
 
 	if mkAll {
 	if mkAll {
@@ -60,7 +65,7 @@ func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chown
 	// even if it existed, we will chown the requested path + any subpaths that
 	// even if it existed, we will chown the requested path + any subpaths that
 	// didn't exist when we called MkdirAll
 	// didn't exist when we called MkdirAll
 	for _, pathComponent := range paths {
 	for _, pathComponent := range paths {
-		if err := os.Chown(pathComponent, ownerUID, ownerGID); err != nil {
+		if err := lazyChown(pathComponent, ownerUID, ownerGID, nil); err != nil {
 			return err
 			return err
 		}
 		}
 	}
 	}
@@ -202,3 +207,20 @@ func callGetent(args string) (io.Reader, error) {
 	}
 	}
 	return bytes.NewReader(out), nil
 	return bytes.NewReader(out), nil
 }
 }
+
+// lazyChown performs a chown only if the uid/gid don't match what's requested
+// Normally a Chown is a no-op if uid/gid match, but in some cases this can still cause an error, e.g. if the
+// dir is on an NFS share, so don't call chown unless we absolutely must.
+func lazyChown(p string, uid, gid int, stat *system.StatT) error {
+	if stat == nil {
+		var err error
+		stat, err = system.Stat(p)
+		if err != nil {
+			return err
+		}
+	}
+	if stat.UID() == uint32(uid) && stat.GID() == uint32(gid) {
+		return nil
+	}
+	return os.Chown(p, uid, gid)
+}