Re: [kubernetes/kubernetes] Containerized subpath (#63143)

18 views
Skip to first unread message

Jan Šafránek

unread,
Apr 27, 2018, 10:49:46 AM4/27/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@kubernetes/sig-storage-pr-reviews, any review is highly appreciated, we don't want more regressions in subpath!

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Michelle Au

unread,
Apr 27, 2018, 1:26:41 PM4/27/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@msau42 commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

> @@ -33,7 +33,7 @@ import (
 
 const (
 	// hostProcMountsPath is the default mount path for rootfs
-	hostProcMountsPath = "/rootfs/proc/1/mounts"
+	HostProcMountsPath = "/rootfs/proc/1/mounts"

Does this need to be public?

In pkg/util/mount/mount_windows.go:

> @@ -436,8 +436,9 @@ func getAllParentLinks(path string) ([]string, error) {
 }
 
 // SafeMakeDir makes sure that the created directory does not escape given base directory mis-using symlinks.
-func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return doSafeMakeDir(pathname, base, perm)
+func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error {
+	fullPath := filepath.Join(base, subdir)

Does windows not need to eval symlinks?

In pkg/util/mount/nsenter_mount.go:

> +	newPath, err := mounter.evalHostSymlinks(subpath.Path, true /*mustExist*/)
+	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err)
+	}
+	glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, newPath, subpath.VolumePath)
+	subpath.VolumePath = mounter.getKubeletPath(filepath.Clean(newVolumePath))
+	subpath.Path = mounter.getKubeletPath(filepath.Clean(newPath))
+
+	// Check the subpath is correct and open it
+	fd, err := safeOpenSubPath(mounter, subpath)
+	if err != nil {
+		return "", err
+	}
+	defer syscall.Close(fd)
+
+	glog.Infof("JSAF before prepare")

Cleanup

In pkg/util/mount/nsenter_mount.go:

> @@ -282,7 +274,15 @@ func (mounter *NsenterMounter) MakeFile(pathname string) error {
 }
 
 func (mounter *NsenterMounter) ExistsPath(pathname string) (bool, error) {
-	_, err := mounter.Lstat(pathname)
+	// Evaluate symlinks on the host, but allow the path not to exist -
+	// evalHostSymlinks would return undistinguishable error when path did not
+	// exist and mustExist was true.
+	evaluatedPath, err := mounter.evalHostSymlinks(pathname, false /* mustExist */)
+	if err != nil {
+		return false, err
+	}
+	kubeletPath := mounter.getKubeletPath(evaluatedPath)
+	_, err = os.Lstat(kubeletPath)

what was wrong with mounter.Lstat()?

In hack/local-up-cluster.sh:

> @@ -800,7 +800,7 @@ function start_kubelet {
       all_kubelet_flags+=(--containerized)
 
       docker run --rm --name kubelet \
-        --volume=/:/rootfs:ro,rslave \
+        --volume=/:/rootfs:rw,rslave \

does this mean existing deployments will break if they upgrade to this fix and don't change this setting?

Previously, safemkdir still worked for non-hostpath types because it uses /var/lib/kubelet/.... instead of /rootfs/var/lib/kubelet

In pkg/util/mount/nsenter_mount.go:

>  	}
-	return strconv.Atoi(string(matches[1]))
+	kubeletBase := mounter.getKubeletPath(evaluatedBase)

Do the paths need to be cleaned?

In pkg/util/mount/mount_linux.go:

>  
-	// Check it's not already bind-mounted
+// prepareSubpathTarget creates target for bind-mount of subpath. It returns

Add comment this is also used by nsenter

In pkg/util/mount/nsenter_mount.go:

> +	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err)
+	}
+	glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, newPath, subpath.VolumePath)
+	subpath.VolumePath = mounter.getKubeletPath(filepath.Clean(newVolumePath))
+	subpath.Path = mounter.getKubeletPath(filepath.Clean(newPath))
+
+	// Check the subpath is correct and open it
+	fd, err := safeOpenSubPath(mounter, subpath)
+	if err != nil {
+		return "", err
+	}
+	defer syscall.Close(fd)
+
+	glog.Infof("JSAF before prepare")
+	alreadyMounted, bindPathTarget, err := prepareSubpathTarget(mounter, subpath)

should this be done first?

In pkg/util/mount/mount_linux.go:

>  	}
 	defer syscall.Close(fd)
 
+	alreadyMounted, bindPathTarget, err := prepareSubpathTarget(mounter, subpath)

Check this first?

Yecheng Fu

unread,
Apr 28, 2018, 12:58:26 PM4/28/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@cofyc commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

>  
-	// Bind-mount the subpath to avoid using symlinks in subpaths.
-	newHostPath, err = doBindSubPath(mounter, subPath, hostPid)
+// evalHostSymlinks returns the path name after the evaluation of any symbolic
+// links on the host. In other words, it does the same as filepath.EvalSymlinks()
+// would do when it ran on the host and not inside a container.
+//
+// mustExist makes evalHostSymlinks to return error when the path does not
+// exist. When it's false, it evaluates symlinks of the existing part and
+// blindly adds the non-existing part:
+// path: /mnt/volume/non/existing/directory. /mnt/volume exists and
+// non/existing/directory does not exist. It resolves symlinks in /mnt/volume
+// to say /mnt/foo and returns /mnt/foo/non/existing/directory.
+func (mounter *NsenterMounter) evalHostSymlinks(path string, mustExist bool) (string, error) {

I had added a similar function in https://github.com/kubernetes/kubernetes/pull/62903/files#diff-f6ffeca7a3b942208d644eaa4ec99642R127 as you and @msau42 suggested. How about merging #62903 first, then use Nsenter.HostPath to get the path name on the host after evaluating symlinks on the host (and adding mustExist parameter)?

k8s-ci-robot

unread,
Apr 28, 2018, 6:55:48 PM4/28/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Jan Šafránek

unread,
Apr 30, 2018, 5:01:25 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

> @@ -33,7 +33,7 @@ import (
 
 const (
 	// hostProcMountsPath is the default mount path for rootfs
-	hostProcMountsPath = "/rootfs/proc/1/mounts"
+	HostProcMountsPath = "/rootfs/proc/1/mounts"

It does not need to be public if we introduce Nsenter.HostPath() as in https://github.com/kubernetes/kubernetes/pull/62903/files#diff-f6ffeca7a3b942208d644eaa4ec99642R129

Jan Šafránek

unread,
Apr 30, 2018, 5:03:27 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/mount_windows.go:

> @@ -436,8 +436,9 @@ func getAllParentLinks(path string) ([]string, error) {
 }
 
 // SafeMakeDir makes sure that the created directory does not escape given base directory mis-using symlinks.
-func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return doSafeMakeDir(pathname, base, perm)
+func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error {
+	fullPath := filepath.Join(base, subdir)

good catch, fixed

Jan Šafránek

unread,
Apr 30, 2018, 5:04:47 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

> +	newPath, err := mounter.evalHostSymlinks(subpath.Path, true /*mustExist*/)
+	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err)
+	}
+	glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, newPath, subpath.VolumePath)
+	subpath.VolumePath = mounter.getKubeletPath(filepath.Clean(newVolumePath))
+	subpath.Path = mounter.getKubeletPath(filepath.Clean(newPath))
+
+	// Check the subpath is correct and open it
+	fd, err := safeOpenSubPath(mounter, subpath)
+	if err != nil {
+		return "", err
+	}
+	defer syscall.Close(fd)
+
+	glog.Infof("JSAF before prepare")

Cleaned up

Jan Šafránek

unread,
Apr 30, 2018, 5:08:59 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

> @@ -282,7 +274,15 @@ func (mounter *NsenterMounter) MakeFile(pathname string) error {
 }
 
 func (mounter *NsenterMounter) ExistsPath(pathname string) (bool, error) {
-	_, err := mounter.Lstat(pathname)
+	// Evaluate symlinks on the host, but allow the path not to exist -
+	// evalHostSymlinks would return undistinguishable error when path did not
+	// exist and mustExist was true.
+	evaluatedPath, err := mounter.evalHostSymlinks(pathname, false /* mustExist */)
+	if err != nil {
+		return false, err
+	}
+	kubeletPath := mounter.getKubeletPath(evaluatedPath)
+	_, err = os.Lstat(kubeletPath)

Reworked mounter.Lstat() to return NotExist when the target does not exist (it returned some indistinguishable error before).

Jan Šafránek

unread,
Apr 30, 2018, 5:35:40 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In hack/local-up-cluster.sh:

> @@ -800,7 +800,7 @@ function start_kubelet {
       all_kubelet_flags+=(--containerized)
 
       docker run --rm --name kubelet \
-        --volume=/:/rootfs:ro,rslave \
+        --volume=/:/rootfs:rw,rslave \

That's right, it will break.

I could pass podDir into SafeMakeDir() as a new parameter. NsenterMounter.SafeMakeDir could then check if the directory to create is subdir of podDir and add /rootfs only when it's not.

Any better ideas?

Jan Šafránek

unread,
Apr 30, 2018, 5:47:06 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

> +	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err)
+	}
+	glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, newPath, subpath.VolumePath)
+	subpath.VolumePath = mounter.getKubeletPath(filepath.Clean(newVolumePath))
+	subpath.Path = mounter.getKubeletPath(filepath.Clean(newPath))
+
+	// Check the subpath is correct and open it
+	fd, err := safeOpenSubPath(mounter, subpath)
+	if err != nil {
+		return "", err
+	}
+	defer syscall.Close(fd)
+
+	glog.Infof("JSAF before prepare")
+	alreadyMounted, bindPathTarget, err := prepareSubpathTarget(mounter, subpath)

IMO it does not really matter if we do safeOpenSubPath or prepareSubpathTarget first, I wanted the defer that cleans up volumes to affect as little code as possible.

Jan Šafránek

unread,
Apr 30, 2018, 5:53:25 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

>  	}
-	return strconv.Atoi(string(matches[1]))
+	kubeletBase := mounter.getKubeletPath(evaluatedBase)

Added to doSafeMakeDir, just to be sure.

Jan Šafránek

unread,
Apr 30, 2018, 5:55:32 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Push

@jsafrane pushed 1 commit.

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

k8s-ci-robot

unread,
Apr 30, 2018, 5:57:33 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or mute the thread.

k8s-ci-robot

unread,
Apr 30, 2018, 5:57:34 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e 9b6d859 link /test pull-kubernetes-local-e2e

k8s-ci-robot

unread,
Apr 30, 2018, 5:57:59 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-bazel-build 9b6d859 link /test pull-kubernetes-bazel-build

k8s-ci-robot

unread,
Apr 30, 2018, 5:58:00 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-e2e-gce 9b6d859 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu 9b6d859 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-node-e2e 9b6d859 link /test pull-kubernetes-node-e2e

k8s-ci-robot

unread,
Apr 30, 2018, 5:58:05 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 5:58:09 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 5:58:13 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 5:58:43 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e 9b6d859 link /test pull-kubernetes-local-e2e
pull-kubernetes-bazel-build 9b6d859 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce 9b6d859 link /test pull-kubernetes-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 5:58:58 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-e2e-gce-device-plugin-gpu 9b6d859 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-node-e2e 9b6d859 link /test pull-kubernetes-node-e2e
pull-kubernetes-integration 9b6d859 link /test pull-kubernetes-integration

k8s-ci-robot

unread,
Apr 30, 2018, 5:59:01 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-typecheck 9b6d859 link /test pull-kubernetes-typecheck

k8s-ci-robot

unread,
Apr 30, 2018, 5:59:04 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-verify 9b6d859 link /test pull-kubernetes-verify

Davanum Srinivas

unread,
Apr 30, 2018, 6:24:18 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane if you rebase to master, we can try the new e2e job

Jan Šafránek

unread,
Apr 30, 2018, 7:27:55 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Push

@jsafrane pushed 1 commit.

  • fdb50b5 Created directories in /var/lib/kubelet directly.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

k8s-ci-robot

unread,
Apr 30, 2018, 7:28:21 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jsafrane
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: dchen1107

Assign the PR to them by writing /assign @dchen1107 in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

—
You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub, or mute the thread.

k8s-ci-robot

unread,
Apr 30, 2018, 7:30:57 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-e2e-gce 9b6d859 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu 9b6d859 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-node-e2e 9b6d859 link /test pull-kubernetes-node-e2e
pull-kubernetes-integration 9b6d859 link /test pull-kubernetes-integration
pull-kubernetes-typecheck 9b6d859 link /test pull-kubernetes-typecheck
pull-kubernetes-verify 9b6d859 link /test pull-kubernetes-verify
pull-kubernetes-bazel-build fdb50b5 link /test pull-kubernetes-bazel-build

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

Jan Šafránek

unread,
Apr 30, 2018, 7:30:58 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In hack/local-up-cluster.sh:

> @@ -800,7 +800,7 @@ function start_kubelet {
       all_kubelet_flags+=(--containerized)
 
       docker run --rm --name kubelet \
-        --volume=/:/rootfs:ro,rslave \
+        --volume=/:/rootfs:rw,rslave \

I added a new argument to NewNsenterMounter so the mounter knows what's location of /var/lib/kubelet and can do a shortcut in SafeMakeDir. That was the easiest way I could find out. PTAL.

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:00 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-integration 9b6d859 link /test pull-kubernetes-integration
pull-kubernetes-typecheck 9b6d859 link /test pull-kubernetes-typecheck
pull-kubernetes-verify 9b6d859 link /test pull-kubernetes-verify
pull-kubernetes-bazel-build fdb50b5 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce fdb50b5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu fdb50b5 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-local-e2e-containerized fdb50b5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-node-e2e fdb50b5 link /test pull-kubernetes-node-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:01 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e 9b6d859 link /test pull-kubernetes-local-e2e
pull-kubernetes-node-e2e 9b6d859 link /test pull-kubernetes-node-e2e
pull-kubernetes-integration 9b6d859 link /test pull-kubernetes-integration
pull-kubernetes-typecheck 9b6d859 link /test pull-kubernetes-typecheck
pull-kubernetes-verify 9b6d859 link /test pull-kubernetes-verify
pull-kubernetes-bazel-build fdb50b5 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce fdb50b5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu fdb50b5 link /test pull-kubernetes-e2e-gce-device-plugin-gpu

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:02 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-local-e2e-containerized fdb50b5 link /test pull-kubernetes-local-e2e-containerized

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:02 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e 9b6d859 link /test pull-kubernetes-local-e2e
pull-kubernetes-e2e-gce-device-plugin-gpu 9b6d859 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-node-e2e 9b6d859 link /test pull-kubernetes-node-e2e
pull-kubernetes-integration 9b6d859 link /test pull-kubernetes-integration
pull-kubernetes-typecheck 9b6d859 link /test pull-kubernetes-typecheck
pull-kubernetes-verify 9b6d859 link /test pull-kubernetes-verify
pull-kubernetes-bazel-build fdb50b5 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce fdb50b5 link /test pull-kubernetes-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:03 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kops-aws 9b6d859 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-node-e2e 9b6d859 link /test pull-kubernetes-node-e2e
pull-kubernetes-integration 9b6d859 link /test pull-kubernetes-integration
pull-kubernetes-typecheck 9b6d859 link /test pull-kubernetes-typecheck
pull-kubernetes-verify 9b6d859 link /test pull-kubernetes-verify
pull-kubernetes-bazel-build fdb50b5 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce fdb50b5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu fdb50b5 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:29 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e-containerized fdb50b5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-node-e2e fdb50b5 link /test pull-kubernetes-node-e2e
pull-kubernetes-e2e-kops-aws fdb50b5 link /test pull-kubernetes-e2e-kops-aws

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:32 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test
pull-kubernetes-kubemark-e2e-gce 9b6d859 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-integration fdb50b5 link /test pull-kubernetes-integration

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:32 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-kubemark-e2e-gce fdb50b5 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-bazel-test fdb50b5 link /test pull-kubernetes-bazel-test
pull-kubernetes-typecheck fdb50b5 link /test pull-kubernetes-typecheck

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:33 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-typecheck 9b6d859 link /test pull-kubernetes-typecheck

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:33 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test 9b6d859 link /test pull-kubernetes-bazel-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
Apr 30, 2018, 7:31:35 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-bazel-test fdb50b5 link /test pull-kubernetes-bazel-test
pull-kubernetes-typecheck fdb50b5 link /test pull-kubernetes-typecheck
pull-kubernetes-verify fdb50b5 link /test pull-kubernetes-verify

Jan Šafránek

unread,
Apr 30, 2018, 7:34:11 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/mount_linux.go:

>  
-	// Check it's not already bind-mounted
+// prepareSubpathTarget creates target for bind-mount of subpath. It returns

Comment added

Jan Šafránek

unread,
Apr 30, 2018, 7:35:37 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In hack/local-up-cluster.sh:

> @@ -800,7 +800,7 @@ function start_kubelet {
       all_kubelet_flags+=(--containerized)
 
       docker run --rm --name kubelet \
-        --volume=/:/rootfs:ro,rslave \
+        --volume=/:/rootfs:rw,rslave \

And I removed changes in hack/local-up-cluster.sh

k8s-ci-robot

unread,
Apr 30, 2018, 7:35:53 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jsafrane
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: dchen1107

Assign the PR to them by writing /assign @dchen1107 in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

—

k8s-ci-robot

unread,
Apr 30, 2018, 7:41:20 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-verify fdb50b5 link /test pull-kubernetes-verify
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

Davanum Srinivas

unread,
Apr 30, 2018, 7:54:42 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

looks like we may break other platforms based on what pull-kubernetes-typecheck is saying ..

ERROR(darwin/386,darwin/amd64,windows/386,windows/amd64) cmd/kubelet/app/server.go:342:42: too many arguments

k8s-ci-robot

unread,
Apr 30, 2018, 8:14:03 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-e2e-gce fdb50b5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-local-e2e-containerized fdb50b5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-kubemark-e2e-gce fdb50b5 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-e2e-kops-aws 1e65c09 link /test pull-kubernetes-e2e-kops-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

k8s-ci-robot

unread,
Apr 30, 2018, 8:30:32 AM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-local-e2e-containerized fdb50b5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-e2e-kops-aws 1e65c09 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce 1e65c09 link /test pull-kubernetes-e2e-gce

Michelle Au

unread,
Apr 30, 2018, 1:52:05 PM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@msau42 commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

>  
+	// Base is somewhere on the host's filesystem. Add /rootfs and try to make
+	// the directory there.

Add comment that this requires /rootfs to be writeable.

Michelle Au

unread,
Apr 30, 2018, 2:31:26 PM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@msau42 commented on this pull request.

In pkg/util/mount/mount_linux.go:

> +	// Linux, kubelet runs on the host:
+	// - safely open the subpath
+	// - bind-mount /proc/<pid of kubelet>/fd/<fd> to subpath target
+	// User can't change /proc/<pid of kubelet>/fd/<fd> to point to a bad place.
+
+	// Evaluate all symlinks here once for all subsequent functions.
+	newVolumePath, err := filepath.EvalSymlinks(subpath.VolumePath)
+	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.VolumePath, err)
+	}
+	newPath, err := filepath.EvalSymlinks(subpath.Path)
+	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err)
+	}
+	glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, newPath, subpath.VolumePath)
+	subpath.VolumePath = filepath.Clean(newVolumePath)

Clean is not needed here since it's done in doSafeOpen()?

In pkg/util/mount/nsenter_mount.go:

> @@ -33,7 +33,7 @@ import (
 
 const (
 	// hostProcMountsPath is the default mount path for rootfs
-	hostProcMountsPath = "/rootfs/proc/1/mounts"
+	HostProcMountsPath = "/rootfs/proc/1/mounts"

I only see it being used in this file.

In pkg/util/mount/nsenter_mount.go:

>  	}
-	matches := pidRegExp.FindSubmatch(statusBytes)
-	if len(matches) < 2 {
-		return 0, fmt.Errorf("cannot parse %s: no Pid:", procStatusPath)
+	evaluatedBase = filepath.Clean(evaluatedBase)
+	evaluatedSubdirPath, err := mounter.evalHostSymlinks(fullSubdirPath, false /* mustExist */)
+	if err != nil {
+		return fmt.Errorf("error resolving symlinks in %s: %s", fullSubdirPath, err)
+	}
+	evaluatedSubdirPath = filepath.Clean(evaluatedSubdirPath)
+
+	rootDir := filepath.Clean(mounter.rootDir)
+	if pathWithinBase(evaluatedBase, rootDir) {

unit test for this?

In pkg/kubelet/kubelet_pods.go:

>  			if err != nil {
 				return nil, cleanupAction, err
 			}
 			perm := fileinfo.Mode()
 
-			volumePath, err := filepath.EvalSymlinks(hostPath)

Does windows PrepareSafeSupath need to evalsymlinks too?

Michelle Au

unread,
Apr 30, 2018, 2:34:21 PM4/30/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@msau42 commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

> +			if cleanErr := cleanSubPath(mounter, subpath); cleanErr != nil {
+				glog.Errorf("Failed to clean subpath %q: %v", bindPathTarget, cleanErr)
+			}
+		}
+	}()
+
+	// subpath.Path is in kubelet's mount namespace, translate it to the host
+	hostSubpath := strings.TrimPrefix(subpath.Path, nsenter.HostRootFsPath)
+	// Leap of faith: optimistically expect that nobody has modified previously
+	// expanded evalSubPath with evil symlinks and bind-mount it.
+	glog.V(5).Infof("bind mounting %q at %q", hostSubpath, bindPathTarget)
+	if err = mounter.Mount(hostSubpath, bindPathTarget, "" /*fstype*/, []string{"bind"}); err != nil {
+		return "", fmt.Errorf("error mounting %s: %s", hostSubpath, err)
+	}
+
+	// Check that the bind-mount target is the same inode and device as the

Is it possible to unit test this section?

Jan Šafránek

unread,
May 3, 2018, 6:15:21 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/mount_linux.go:

> +	// Linux, kubelet runs on the host:
+	// - safely open the subpath
+	// - bind-mount /proc/<pid of kubelet>/fd/<fd> to subpath target
+	// User can't change /proc/<pid of kubelet>/fd/<fd> to point to a bad place.
+
+	// Evaluate all symlinks here once for all subsequent functions.
+	newVolumePath, err := filepath.EvalSymlinks(subpath.VolumePath)
+	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.VolumePath, err)
+	}
+	newPath, err := filepath.EvalSymlinks(subpath.Path)
+	if err != nil {
+		return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err)
+	}
+	glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, newPath, subpath.VolumePath)
+	subpath.VolumePath = filepath.Clean(newVolumePath)

fixed

Jan Šafránek

unread,
May 3, 2018, 6:18:54 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/util/mount/nsenter_mount.go:

>  
+	// Base is somewhere on the host's filesystem. Add /rootfs and try to make
+	// the directory there.

added

Jan Šafránek

unread,
May 3, 2018, 6:20:22 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane commented on this pull request.

In pkg/kubelet/kubelet_pods.go:

>  			if err != nil {
 				return nil, cleanupAction, err
 			}
 			perm := fileinfo.Mode()
 
-			volumePath, err := filepath.EvalSymlinks(hostPath)

PrepareSafeSupath on Windows calls filepath.EvalSymlinks inside lockAndCheckSubPath

k8s-ci-robot

unread,
May 3, 2018, 7:16:48 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-e2e-kops-aws 1e65c09 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce 1e65c09 link /test pull-kubernetes-e2e-gce
pull-kubernetes-bazel-build 55232d5 link /test pull-kubernetes-bazel-build

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

k8s-ci-robot

unread,
May 3, 2018, 7:17:04 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-bazel-build 55232d5 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce 55232d5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu 55232d5 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws 55232d5 link /test pull-kubernetes-e2e-kops-aws

k8s-ci-robot

unread,
May 3, 2018, 7:17:06 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-e2e-kops-aws 1e65c09 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-bazel-build 55232d5 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce 55232d5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu 55232d5 link /test pull-kubernetes-e2e-gce-device-plugin-gpu

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
May 3, 2018, 7:17:46 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
May 3, 2018, 7:18:04 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-bazel-build 55232d5 link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce 55232d5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu 55232d5 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws 55232d5 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 55232d5 link /test pull-kubernetes-kubemark-e2e-gce

k8s-ci-robot

unread,
May 3, 2018, 7:18:45 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-local-e2e-containerized 55232d5 link /test pull-kubernetes-local-e2e-containerized

k8s-ci-robot

unread,
May 3, 2018, 7:19:08 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-bazel-test 55232d5 link /test pull-kubernetes-bazel-test

Jan Šafránek

unread,
May 3, 2018, 7:20:49 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

Rebased and reworked a bit to use new @cofyc's Nsenter methods. The overall logic is the same, but I removed mounter.Lstat in favor of mounter.GetMode - Lstat was needed only by kubelet_pods.go and it used it only to get file mode.

Jan Šafránek

unread,
May 3, 2018, 7:21:25 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@msau42, I am still working on unit tests

k8s-ci-robot

unread,
May 3, 2018, 7:23:49 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-e2e-gce 55232d5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu 55232d5 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws 55232d5 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 55232d5 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e-containerized 55232d5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-bazel-test 55232d5 link /test pull-kubernetes-bazel-test
pull-kubernetes-bazel-build f62e86f link /test pull-kubernetes-bazel-build

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

k8s-ci-robot

unread,
May 3, 2018, 7:24:08 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-e2e-gce 55232d5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-kubemark-e2e-gce 55232d5 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e-containerized 55232d5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-bazel-test 55232d5 link /test pull-kubernetes-bazel-test
pull-kubernetes-bazel-build f62e86f link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce-device-plugin-gpu f62e86f link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws f62e86f link /test pull-kubernetes-e2e-kops-aws

k8s-ci-robot

unread,
May 3, 2018, 7:24:21 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-e2e-gce 55232d5 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-kops-aws 55232d5 link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-kubemark-e2e-gce 55232d5 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e-containerized 55232d5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-bazel-test 55232d5 link /test pull-kubernetes-bazel-test
pull-kubernetes-bazel-build f62e86f link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce-device-plugin-gpu f62e86f link /test pull-kubernetes-e2e-gce-device-plugin-gpu

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
May 3, 2018, 7:24:44 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-kubemark-e2e-gce 55232d5 link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e-containerized 55232d5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-bazel-test 55232d5 link /test pull-kubernetes-bazel-test
pull-kubernetes-bazel-build f62e86f link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce-device-plugin-gpu f62e86f link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws f62e86f link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce f62e86f link /test pull-kubernetes-e2e-gce

k8s-ci-robot

unread,
May 3, 2018, 7:26:17 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-local-e2e-containerized 55232d5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-bazel-test 55232d5 link /test pull-kubernetes-bazel-test
pull-kubernetes-bazel-build f62e86f link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce-device-plugin-gpu f62e86f link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws f62e86f link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce f62e86f link /test pull-kubernetes-e2e-gce
pull-kubernetes-kubemark-e2e-gce f62e86f link /test pull-kubernetes-kubemark-e2e-gce

k8s-ci-robot

unread,
May 3, 2018, 7:26:52 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-local-e2e-containerized 55232d5 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-bazel-build f62e86f link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce-device-plugin-gpu f62e86f link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws f62e86f link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce f62e86f link /test pull-kubernetes-e2e-gce
pull-kubernetes-kubemark-e2e-gce f62e86f link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-bazel-test f62e86f link /test pull-kubernetes-bazel-test

k8s-ci-robot

unread,
May 3, 2018, 7:29:52 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 1e65c09 link /test pull-kubernetes-typecheck
pull-kubernetes-bazel-build f62e86f link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce-device-plugin-gpu f62e86f link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-kops-aws f62e86f link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce f62e86f link /test pull-kubernetes-e2e-gce
pull-kubernetes-kubemark-e2e-gce f62e86f link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-bazel-test f62e86f link /test pull-kubernetes-bazel-test
pull-kubernetes-local-e2e-containerized f62e86f link /test pull-kubernetes-local-e2e-containerized

k8s-ci-robot

unread,
May 3, 2018, 7:46:51 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-verify f62e86f link /test pull-kubernetes-verify

Andy Zhang

unread,
May 3, 2018, 8:37:31 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane you may also build on Windows and run windows unit test on pkg\util\mount, someone is working on making windows unit test as CI test, while currently we need to run it locally.

KUBE_BUILD_PLATFORMS=windows/amd64 make WHAT=cmd/kubelet

c:\Go\src\k8s.io\kubernetes\pkg\util\mount>go test -v
# k8s.io/kubernetes/pkg/util/mount
.\mount.go:147: cannot use Mounter literal (type *Mounter) as type Interface in assignment:
        *Mounter does not implement Interface (missing GetMode method)
.\mount_windows.go:48: cannot use Mounter literal (type *Mounter) as type Interface in return argument:
        *Mounter does not implement Interface (missing GetMode method)
.\mount_windows.go:139: cannot use mounter (type *Mounter) as type Interface in argument to IsNotMountPoint:
        *Mounter does not implement Interface (missing GetMode method)
.\mount_windows.go:158: cannot use mounter (type *Mounter) as type Interface in argument to getDeviceNameFromMount:
        *Mounter does not implement Interface (missing GetMode method)
.\mount_windows.go:444: cannot use mounter (type *Mounter) as type Interface in argument to getMountRefsByDev:
        *Mounter does not implement Interface (missing GetMode method)
.\mount_windows.go:453: undefined: fakeMounter
FAIL    k8s.io/kubernetes/pkg/util/mount [build failed]

Andy Zhang

unread,
May 3, 2018, 9:03:31 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@andyzhangx requested changes on this pull request.

@jsafrane and the unit test TestDoSafeMakeDir in mount_windows_test.go should also change since this PR use subdir instead of full path

Jan Šafránek

unread,
May 3, 2018, 11:27:17 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

/test pull-kubernetes-cross

Jan Šafránek

unread,
May 3, 2018, 11:27:29 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

/test pull-kubernetes-local-e2e-containerized

Jan Šafránek

unread,
May 3, 2018, 11:33:02 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

the unit test TestDoSafeMakeDir in mount_windows_test.go should also change since this PR use subdir instead of full path

@andyzhangx, SafeMakeDir joins the paths together and doSafeMakeDir did not change at all.

k8s-ci-robot

unread,
May 3, 2018, 11:39:34 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 7ee1f9c link /test pull-kubernetes-cross
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 81bc31c link /test pull-kubernetes-typecheck

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

k8s-ci-robot

unread,
May 3, 2018, 11:47:52 AM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-typecheck 81bc31c link /test pull-kubernetes-typecheck
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross

k8s-ci-robot

unread,
May 3, 2018, 12:29:55 PM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-e2e-gce-device-plugin-gpu 42359ed link /test pull-kubernetes-e2e-gce-device-plugin-gpu

k8s-ci-robot

unread,
May 3, 2018, 12:36:23 PM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-e2e-gce-device-plugin-gpu 42359ed link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-gce 42359ed link /test pull-kubernetes-e2e-gce

k8s-ci-robot

unread,
May 3, 2018, 1:13:21 PM5/3/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-e2e-gce-device-plugin-gpu 42359ed link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-e2e-gce 42359ed link /test pull-kubernetes-e2e-gce
pull-kubernetes-kubemark-e2e-gce 42359ed link /test pull-kubernetes-kubemark-e2e-gce

Jan Šafránek

unread,
May 4, 2018, 4:13:49 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

/retest

k8s-ci-robot

unread,
May 4, 2018, 6:29:03 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-bazel-build d27e6ca link /test pull-kubernetes-bazel-build

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

k8s-ci-robot

unread,
May 4, 2018, 6:30:08 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-bazel-build d27e6ca link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce d27e6ca link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-kops-aws d27e6ca link /test pull-kubernetes-e2e-kops-aws

k8s-ci-robot

unread,
May 4, 2018, 6:30:17 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-bazel-build d27e6ca link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce d27e6ca link /test pull-kubernetes-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
May 4, 2018, 6:30:36 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-bazel-build d27e6ca link /test pull-kubernetes-bazel-build
pull-kubernetes-e2e-gce d27e6ca link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-kops-aws d27e6ca link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-bazel-test d27e6ca link /test pull-kubernetes-bazel-test

k8s-ci-robot

unread,
May 4, 2018, 6:30:38 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-e2e-gce-device-plugin-gpu d27e6ca link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-typecheck d27e6ca link /test pull-kubernetes-typecheck

k8s-ci-robot

unread,
May 4, 2018, 6:31:18 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

k8s-ci-robot

unread,
May 4, 2018, 6:32:40 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-typecheck d27e6ca link /test pull-kubernetes-typecheck
pull-kubernetes-kubemark-e2e-gce d27e6ca link /test pull-kubernetes-kubemark-e2e-gce

k8s-ci-robot

unread,
May 4, 2018, 6:33:22 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-local-e2e-containerized d27e6ca link /test pull-kubernetes-local-e2e-containerized

Jan Šafránek

unread,
May 4, 2018, 6:38:14 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

Added bunch of unit tests for Nsenter and NsenterMounter.

I noticed that realpath -m does not report any error, i.e. on a symlink loop or "access denied" errors. It may happen that a symlink that can't be resolved on the host can be resolved in container. These two methods are affected:

  • ExistsPath() - it may report that a file exists when it's not.
  • SafeMakeDir will fail correctly.

I leave it as it is for now, @cofyc, do you still have your EvalSymlinks implementation that just reads files in /rootfs and resolves symlinks there?

k8s-ci-robot

unread,
May 4, 2018, 6:41:23 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention
pull-kubernetes-kubemark-e2e-gce d27e6ca link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e-containerized d27e6ca link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-typecheck 0d02deb link /test pull-kubernetes-typecheck

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

k8s-ci-robot

unread,
May 4, 2018, 7:04:14 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-e2e-gce d27e6ca link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-kops-aws d27e6ca link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce-device-plugin-gpu d27e6ca link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-kubemark-e2e-gce d27e6ca link /test pull-kubernetes-kubemark-e2e-gce
pull-kubernetes-local-e2e-containerized d27e6ca link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-typecheck 0d02deb link /test pull-kubernetes-typecheck
pull-kubernetes-verify 0d02deb link /test pull-kubernetes-verify

k8s-ci-robot

unread,
May 4, 2018, 8:10:15 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-local-e2e-containerized d27e6ca link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-verify 26195e2 link /test pull-kubernetes-verify

Jan Šafránek

unread,
May 4, 2018, 11:10:07 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

/test pull-kubernetes-cross

Jan Šafránek

unread,
May 4, 2018, 11:10:58 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

/retest

k8s-ci-robot

unread,
May 4, 2018, 11:40:07 AM5/4/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-cross 1357039 link /test pull-kubernetes-cross
pull-kubernetes-verify 26195e2 link /test pull-kubernetes-verify

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

Jan Šafránek

unread,
May 7, 2018, 4:56:49 AM5/7/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

/retest

k8s-ci-robot

unread,
May 7, 2018, 5:27:49 AM5/7/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-local-e2e fdb50b5 link /test pull-kubernetes-local-e2e
pull-kubernetes-verify 26195e2 link /test pull-kubernetes-verify

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

—

Yecheng Fu

unread,
May 7, 2018, 7:41:43 AM5/7/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@jsafrane b976ebd...cofyc:noexec_evalsymlinks, FYI.

BTW, does readlink work?

Jan Šafránek

unread,
May 7, 2018, 8:20:08 AM5/7/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

realpath works great for most of the time. It does not handle errors well - os.Stat returns nice ErrNotExist, while realpath returns a generic error that something went wrong. In addition, realpath -m does not fail on symlink loop or access denied error.

Michelle Au

unread,
May 7, 2018, 2:03:15 PM5/7/18
to kubernetes/kubernetes, k8s-mirror-storage-pr-reviews, Team mention

@msau42 commented on this pull request.

This can be squashed a bit. I just have some minor comments.

In pkg/util/mount/mount_linux.go:

>  	// There is no action when the container starts. Bind-mount will be cleaned
 	// when container stops by CleanSubPaths.
 	cleanupAction = nil
 	return newHostPath, cleanupAction, err
 }
 
-// This implementation is shared between Linux and NsEnterMounter
-// kubeletPid is PID of kubelet in the PID namespace where bind-mount is done,
-// i.e. pid on the *host* if kubelet runs in a container.
-func doBindSubPath(mounter Interface, subpath Subpath, kubeletPid int) (hostPath string, err error) {
-	// Check early for symlink. This is just a pre-check to avoid bind-mount
-	// before the final check.
-	evalSubPath, err := filepath.EvalSymlinks(subpath.Path)
-	if err != nil {
-		return "", fmt.Errorf("evalSymlinks %q failed: %v", subpath.Path, err)
+func safeOpenSubPath(mounter Interface, subpath Subpath) (int, error) {

Add comment this is also used by nsenter

In pkg/util/mount/mount_windows.go:

> @@ -452,9 +450,23 @@ func (mounter *Mounter) GetFSGroup(pathname string) (int64, error) {
 	return 0, nil
 }
 
+func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) {
+	info, err := os.Stat(pathname)

evalsymlinks?

In pkg/util/mount/nsenter_mount.go:

> @@ -281,65 +276,164 @@ func (mounter *NsenterMounter) MakeFile(pathname string) error {
 	return nil
 }
 
-func (mounter *NsenterMounter) ExistsPath(pathname string) bool {
-	args := []string{pathname}
-	_, err := mounter.ne.Exec("ls", args).CombinedOutput()
-	if err == nil {
-		return true
+func (mounter *NsenterMounter) ExistsPath(pathname string) (bool, error) {
+	// Resolve the symlinks but allow the target not to exist. EvalSymlinks
+	// would return an generic error when the target does not exist.
+	hostPath, err := mounter.ne.EvalSymlinks(pathname, false /* mustExist */)

Can you just do a stat on the host to check if it exists?

In pkg/util/mount/mount.go:

> -	// SafeMakeDir makes sure that the created directory does not escape given
-	// base directory mis-using symlinks. The directory is created in the same
-	// mount namespace as where kubelet is running. Note that the function makes
-	// sure that it creates the directory somewhere under the base, nothing
-	// else. E.g. if the directory already exists, it may exists outside of the
-	// base due to symlinks.
-	SafeMakeDir(pathname string, base string, perm os.FileMode) error
-	// ExistsPath checks whether the path exists.
-	// Will operate in the host mount namespace if kubelet is running in a container
-	ExistsPath(pathname string) bool
+	// SafeMakeDir creates subdir within given base. It makes sure that the
+	// created directory does not escape given base directory mis-using
+	// symlinks. Note that the function makes sure that it creates the directory
+	// somewhere under the base, nothing else. E.g. if the directory already
+	// exists, it may exist outside of the base due to symlinks.
+	SafeMakeDir(subdir string, base string, perm os.FileMode) error

Can you add a comment here this should be used if the path is inside the user's volume. (to distinguish it from MakeDir)

In pkg/util/mount/nsenter_mount.go:

> +				glog.Errorf("Failed to clean subpath %q: %v", bindPathTarget, cleanErr)
+			}
+		}
+	}()
+
+	// Leap of faith: optimistically expect that nobody has modified previously
+	// expanded evalSubPath with evil symlinks and bind-mount it.
+	// Mount is done on the host! don't use kubelet path!
+	glog.V(5).Infof("bind mounting %q at %q", evaluatedHostSubpath, bindPathTarget)
+	if err = mounter.Mount(evaluatedHostSubpath, bindPathTarget, "" /*fstype*/, []string{"bind"}); err != nil {
+		return "", fmt.Errorf("error mounting %s: %s", evaluatedHostSubpath, err)
+	}
+
+	// Check that the bind-mount target is the same inode and device as the
+	// source that we keept open, i.e. we mounted the right thing.
+	var srcStat, dstStat unix.Stat_t

Can this section be put into a method for unit testing?

It is loading more messages.
0 new messages