[Kubelet] Use the custom mounter script for Nfs and Glusterfs only #35821

vishh · 2016-10-28T20:30:18Z

This patch reduces the scope for the containerized mounter to NFS and GlusterFS on GCE + GCI clusters

This patch also enabled the containerized mounter on GCI nodes

Shepherding multiple PRs through the submit queue is painful. Hence I combined them into this PR. Please review each commit individually.

cc @jingxu97 @saad-ali

#35652 has also been reverted as part of this PR

This change is

k8s-ci-robot · 2016-10-28T20:59:52Z

Jenkins unit/integration failed for commit f2f538c6956d3f9721aec1df4c297ea988c9573b. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-28T21:01:26Z

Jenkins GCE Node e2e failed for commit f2f538c6956d3f9721aec1df4c297ea988c9573b. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-28T21:01:38Z

Jenkins Kubemark GCE e2e failed for commit f2f538c6956d3f9721aec1df4c297ea988c9573b. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-28T21:02:57Z

Jenkins GCE e2e failed for commit f2f538c6956d3f9721aec1df4c297ea988c9573b. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-28T21:03:29Z

Jenkins GCE etcd3 e2e failed for commit f2f538c6956d3f9721aec1df4c297ea988c9573b. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

jingxu97 · 2016-10-28T23:39:11Z

pkg/util/mount/mount_linux.go

 func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error {
+	// Path to mounter binary. Set of mount accessible via $PATH by default.
+	mounterPath := "mount"


Can we put a constant for "mount" as DefaultMountPath so that both mount.go and mount_linux.go can use the constant?

jingxu97 · 2016-10-28T23:41:00Z

pkg/util/mount/mount_linux.go

 	}
+	// Use the custom mounter when mounting nfs or glusterfs only.
+	// TODO (storage-team) Remove this hack!
+	if strings.Contains(fstype, "nfs") || strings.Contains(fstype, "gluster") {


Could we put a constant list for "nfs", "gluster" so that it is easier to update the list? We can also do it later separately.

This is a TOTAL HACK. Is this just for risk mitigation? I'm all for being safe, but this seems a step too far to me...

#35821 (comment)

I don't like it either. It is the solution of choice currently for the goog storage team.

vishh · 2016-10-31T15:26:43Z

@jingxu97 rebased

vishh · 2016-10-31T15:28:57Z

For the record, this PR makes changes that are GCI specific (limiting scope of customized mounter to just NFS and GCI). In theory, we can make kubelet logic generic to use a customized mounter for all filesystem types excepting ext4, tmpfs and bind mounts and have the mounter script accept requests for "nfs" and "glusterfs". If this change is desirable, it can be done in a separate PR.
cc @jingxu97 @matchstick @saad-ali

thockin · 2016-10-31T15:43:38Z

cmd/kube-proxy/app/conntrack.go

@@ -67,7 +67,7 @@ func (realConntracker) SetTCPEstablishedTimeout(seconds int) error {
 func isSysFSWritable() (bool, error) {
 	const permWritable = "rw"
 	const sysfsDevice = "sysfs"
-	m := mount.New()
+	m := mount.New("" /* default mount path */)


Would prefer NewDefault()

thockin · 2016-10-31T15:45:19Z

pkg/util/mount/mount_linux.go

 	}
+	// Use the custom mounter when mounting nfs or glusterfs only.
+	// TODO (storage-team) Remove this hack!
+	if strings.Contains(fstype, "nfs") || strings.Contains(fstype, "gluster") {


This is a TOTAL HACK. Is this just for risk mitigation? I'm all for being safe, but this seems a step too far to me...

k8s-ci-robot · 2016-10-31T16:10:54Z

Jenkins verification failed for commit 31ec6ac. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

This reverts commit 765f7b9.

vishh · 2016-11-02T22:20:09Z

As per @jingxu97's offline message, I'm marking this PR LGTM since all the open comments have been addressed.

humblec · 2016-11-03T05:56:15Z

pkg/util/mount/mount_linux.go

 	}
+	// These filesystem types are expected to be supported by the mount utility on the host across all Linux distros.
+	var defaultMounterFsTypes = sets.NewString("tmpfs", "ext4", "ext3", "ext2")


What about XFS ? XFS is the recommended FS for GlusterFS.

k8s-github-robot · 2016-11-03T10:57:05Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2016-11-03T11:32:19Z

Automatic merge from submit-queue

…ope" This reverts commit 973fa6b, reversing changes made to 41b5fe8.

Revert "Merge pull request #35821 from vishh/gci-mounter-scope"

…unter-scope"" This reverts commit 402116a.

@mtaufen

Automatic merge from submit-queue Make GCI nodes mount non tmpfs, ext* & bind mounts using an external mounter This PR downloads the stage1 & gci-mounter ACIs as part of cluster bring up instead of downloading them dynamically from gcr.io, which was the cause for #36206. I have also optimized the containerized mounter to pre-load the mounter image once to avoid fetch latency while using it. Original PR which got reverted: #35821 ```release-note GCI nodes use an external mounter script to mount NFS & GlusterFS storage volumes ``` @mtaufen Node e2e is not re-enabled in this PR. cc @jingxu97

…unter-scope"" This reverts commit 402116a.

@mtaufen

Automatic merge from submit-queue Make GCI nodes mount non tmpfs, ext* & bind mounts using an external mounter This PR downloads the stage1 & gci-mounter ACIs as part of cluster bring up instead of downloading them dynamically from gcr.io, which was the cause for #36206. I have also optimized the containerized mounter to pre-load the mounter image once to avoid fetch latency while using it. Original PR which got reverted: kubernetes/kubernetes#35821 ```release-note GCI nodes use an external mounter script to mount NFS & GlusterFS storage volumes ``` @mtaufen Node e2e is not re-enabled in this PR. cc @jingxu97

@mtaufen

Automatic merge from submit-queue Make GCI nodes mount non tmpfs, ext* & bind mounts using an external mounter This PR downloads the stage1 & gci-mounter ACIs as part of cluster bring up instead of downloading them dynamically from gcr.io, which was the cause for #36206. I have also optimized the containerized mounter to pre-load the mounter image once to avoid fetch latency while using it. Original PR which got reverted: kubernetes/kubernetes#35821 ```release-note GCI nodes use an external mounter script to mount NFS & GlusterFS storage volumes ``` @mtaufen Node e2e is not re-enabled in this PR. cc @jingxu97

@mtaufen

Automatic merge from submit-queue Make GCI nodes mount non tmpfs, ext* & bind mounts using an external mounter This PR downloads the stage1 & gci-mounter ACIs as part of cluster bring up instead of downloading them dynamically from gcr.io, which was the cause for #36206. I have also optimized the containerized mounter to pre-load the mounter image once to avoid fetch latency while using it. Original PR which got reverted: kubernetes/kubernetes#35821 ```release-note GCI nodes use an external mounter script to mount NFS & GlusterFS storage volumes ``` @mtaufen Node e2e is not re-enabled in this PR. cc @jingxu97

@jingxu97

Automatic merge from submit-queue [Kubelet] Use the custom mounter script for Nfs and Glusterfs only This patch reduces the scope for the containerized mounter to NFS and GlusterFS on GCE + GCI clusters This patch also enabled the containerized mounter on GCI nodes Shepherding multiple PRs through the submit queue is painful. Hence I combined them into this PR. Please review each commit individually. cc @jingxu97 @saad-ali kubernetes#35652 has also been reverted as part of this PR

…ope" This reverts commit 973fa6b, reversing changes made to 41b5fe8.

Revert "Merge pull request kubernetes#35821 from vishh/gci-mounter-scope"

…unter-scope"" This reverts commit 402116a.

vishh added priority/backlog Higher priority than priority/awaiting-more-evidence. release-note-none Denotes a PR that doesn't merit a release note. labels Oct 28, 2016

vishh assigned jingxu97 Oct 28, 2016

googlebot added the cla: yes label Oct 28, 2016

k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 28, 2016

vishh mentioned this pull request Oct 28, 2016

Enable containerized storage plugins mounter on GCI - Attempt 2 #35814

Closed

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 28, 2016

vishh force-pushed the gci-mounter-scope branch 2 times, most recently from bfc1ee6 to 0baf799 Compare October 28, 2016 22:41

vishh changed the title ~~[Kubelet] Do not use custom mounter script for bind mounts, ext* and tmpfs mounts~~ [Kubelet] Use the custom mounter script for Nfs and Glusterfs only Oct 28, 2016

jingxu97 reviewed Oct 28, 2016

View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2016

vishh force-pushed the gci-mounter-scope branch from 0baf799 to c3001fd Compare October 31, 2016 15:26

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 31, 2016

vishh force-pushed the gci-mounter-scope branch 2 times, most recently from d985de2 to 31ec6ac Compare October 31, 2016 15:43

thockin reviewed Oct 31, 2016

View reviewed changes

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 31, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2016

Enable containerized storage plugins mounter on GCI

40fc804

This reverts commit 765f7b9.

vishh force-pushed the gci-mounter-scope branch from 3bd69a0 to 40fc804 Compare November 2, 2016 22:18

vishh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 2, 2016

vishh added this to the v1.5 milestone Nov 2, 2016

humblec reviewed Nov 3, 2016

View reviewed changes

k8s-github-robot merged commit 973fa6b into kubernetes:master Nov 3, 2016

This was referenced Nov 3, 2016

Identify missing storage drivers & tooling in GCI #34127

Closed

Docker: Error response from daemon: Container command not found or does not exist #35914

Closed

bprashanth mentioned this pull request Nov 4, 2016

kubernetes-e2e-gci-gke-slow: broken test run #34665

Closed

saad-ali added a commit to saad-ali/kubernetes that referenced this pull request Nov 4, 2016

Revert "Merge pull request kubernetes#35821 from vishh/gci-mounter-sc…

402116a

…ope" This reverts commit 973fa6b, reversing changes made to 41b5fe8.

saad-ali mentioned this pull request Nov 4, 2016

Revert "Merge pull request #35821 from vishh/gci-mounter-scope" #36206

Merged

saad-ali added a commit that referenced this pull request Nov 4, 2016

Merge pull request #36206 from saad-ali/revert35821

9270e38

Revert "Merge pull request #35821 from vishh/gci-mounter-scope"

vishh added a commit to vishh/kubernetes that referenced this pull request Nov 4, 2016

Revert "Revert "Merge pull request kubernetes#35821 from vishh/gci-mo…

07c3e63

…unter-scope"" This reverts commit 402116a.

vishh mentioned this pull request Nov 4, 2016

Make GCI nodes mount non tmpfs, ext* & bind mounts using an external mounter #36267

Merged

jingxu97 pushed a commit to jingxu97/kubernetes that referenced this pull request Nov 8, 2016

Revert "Revert "Merge pull request kubernetes#35821 from vishh/gci-mo…

51c5bc1

…unter-scope"" This reverts commit 402116a.

vishh added a commit to vishh/kubernetes that referenced this pull request Nov 8, 2016

Revert "Revert "Merge pull request kubernetes#35821 from vishh/gci-mo…

dd8ec91

…unter-scope"" This reverts commit 402116a.

jessfraz mentioned this pull request Nov 17, 2016

Cherry picking "mounter" patches on release-1.4 #35809

Closed

jingxu97 pushed a commit to jingxu97/kubernetes that referenced this pull request Nov 21, 2016

Revert "Revert "Merge pull request kubernetes#35821 from vishh/gci-mo…

7095b86

…unter-scope"" This reverts commit 402116a.

dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018

Revert "Merge pull request kubernetes#35821 from vishh/gci-mounter-sc…

8dd6f3a

…ope" This reverts commit 973fa6b, reversing changes made to 41b5fe8.

dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018

Merge pull request kubernetes#36206 from saad-ali/revert35821

e428476

Revert "Merge pull request kubernetes#35821 from vishh/gci-mounter-scope"

dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018

Revert "Revert "Merge pull request kubernetes#35821 from vishh/gci-mo…

4476976

…unter-scope"" This reverts commit 402116a.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Kubelet] Use the custom mounter script for Nfs and Glusterfs only #35821

[Kubelet] Use the custom mounter script for Nfs and Glusterfs only #35821

vishh commented Oct 28, 2016 •

edited

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

jingxu97 Oct 28, 2016

vishh Oct 31, 2016

jingxu97 Oct 28, 2016

vishh Oct 31, 2016

thockin Oct 31, 2016

vishh Oct 31, 2016

vishh commented Oct 31, 2016

vishh commented Oct 31, 2016

thockin Oct 31, 2016

thockin Oct 31, 2016

k8s-ci-robot commented Oct 31, 2016

vishh commented Nov 2, 2016

humblec Nov 3, 2016

k8s-github-robot commented Nov 3, 2016

k8s-github-robot commented Nov 3, 2016

[Kubelet] Use the custom mounter script for Nfs and Glusterfs only #35821

[Kubelet] Use the custom mounter script for Nfs and Glusterfs only #35821

Conversation

vishh commented Oct 28, 2016 • edited

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishh commented Oct 31, 2016

vishh commented Oct 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 31, 2016

vishh commented Nov 2, 2016

Choose a reason for hiding this comment

k8s-github-robot commented Nov 3, 2016

k8s-github-robot commented Nov 3, 2016

vishh commented Oct 28, 2016 •

edited