Add volume reconstruct/cleanup logic in kubelet volume manager #27970

jingxu97 · 2016-06-23T20:21:10Z

Currently kubelet volume management works on the concept of desired
and actual world of states. The volume manager periodically compares the
two worlds and perform volume mount/unmount and/or attach/detach
operations. When kubelet restarts, the cache of those two worlds are
gone. Although desired world can be recovered through apiserver, actual
world can not be recovered which may cause some volumes cannot be cleaned
up if their information is deleted by apiserver. This change adds the
reconstruction of the actual world by reading the pod directories from
disk. The reconstructed volume information is added to both desired
world and actual world if it cannot be found in either world. The rest
logic would be as same as before, desired world populator may clean up
the volume entry if it is no longer in apiserver, and then volume
manager should invoke unmount to clean it up.

Fixes #27653

This change is

yujuhong · 2016-06-23T22:23:31Z

@jingxu97 this fails the unit test FAIL k8s.io/kubernetes/pkg/kubelet [build failed]

yujuhong · 2016-06-23T22:27:10Z

pkg/kubelet/kubelet.go

@@ -901,6 +907,74 @@ func (kl *Kubelet) setupDataDirs() error {
 	return nil
 }

+func (kl *Kubelet) getPodVolumes(podUID types.UID) ([]volumeTuple, error) {


A general question: Is it possible to move all these logic into the volume package?

/cc @saad-ali

erictune · 2016-06-25T14:39:32Z

Is this confirmed as fixing a bug?

jingxu97 · 2016-06-25T17:55:18Z

We decided to not use this clean up code. I found a race condition problem
related to this bug, will post it a little bit later. It is considered a
relatively lower priory for now.

On Sat, Jun 25, 2016 at 7:40 AM, Eric Tune notifications@github.com wrote:

Is this confirmed as fixing a bug?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#27970 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ASSNxemtyJSW-VBlqzVXr08839GuxhPjks5qPT3NgaJpZM4I9M0t
.

Jing

idvoretskyi · 2016-06-29T18:23:10Z

Proposed for 1.3.1

Currently kubelet volume management works on the concept of desired and actual world of states. The volume manager periodically compares the two worlds and perform volume mount/unmount and/or attach/detach operations. When kubelet restarts, the cache of those two worlds are gone. Although desired world can be recovered through apiserver, actual world can not be recovered which may cause some volumes cannot be cleaned up if their information is deleted by apiserver. This change adds the reconstruction of the actual world by reading the pod directories from disk. The reconstructed volume information is added to both desired world and actual world if it cannot be found in either world. The rest logic would be as same as before, desired world populator may clean up the volume entry if it is no longer in apiserver, and then volume manager should invoke unmount to clean it up.

k8s-bot · 2016-08-15T19:06:22Z

GCE e2e build/test passed for commit f19a114.

k8s-github-robot · 2016-08-15T20:10:29Z

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

k8s-bot · 2016-08-15T20:48:40Z

GCE e2e build/test passed for commit f19a114.

k8s-github-robot · 2016-08-15T20:48:43Z

Automatic merge from submit-queue

ixdy · 2016-08-16T22:42:14Z

This PR broke the Darwin and Windows builds, I think:

+++ [0816 15:23:49] darwin/amd64: go build started
# k8s.io/kubernetes/pkg/util/mount
pkg/util/mount/mount.go:89: cannot use Mounter literal (type *Mounter) as type Interface in return argument:
        *Mounter does not implement Interface (missing GetDeviceNameFromMount method)
+++ [0816 15:23:49] windows/amd64: go build started
# k8s.io/kubernetes/pkg/util/mount
pkg/util/mount/mount.go:89: cannot use Mounter literal (type *Mounter) as type Interface in return argument:
        *Mounter does not implement Interface (missing GetDeviceNameFromMount method)
# k8s.io/kubernetes/pkg/util/procfs
pkg/util/procfs/procfs.go:84: undefined: syscall.Kill

It looks like you're missing changes to pkg/util/mount/mount_unsupported.go?

ixdy · 2016-08-16T22:42:37Z

cc @k8s-oncall

ixdy · 2016-08-16T22:45:26Z

Note the breakage in pkg/util/procfs is due to #30087.

jingxu97 · 2016-08-16T23:17:58Z

Yes, I forgot to put the function into the pkg/util/mount/mount_unsupported.go.
Will you issue a PR for it now. Thank you!

Jing

On Tue, Aug 16, 2016 at 3:43 PM, Jeff Grafton notifications@github.com
wrote:

This PR broke the Darwin and Windows builds, I think:

+++ [0816 15:23:49] darwin/amd64: go build started

k8s.io/kubernetes/pkg/util/mount

pkg/util/mount/mount.go:89 http://k8s.io/kubernetes/pkg/util/mountpkg/util/mount/mount.go:89: cannot use Mounter literal (type *Mounter) as type Interface in return argument:
*Mounter does not implement Interface (missing GetDeviceNameFromMount method)
+++ [0816 15:23:49] windows/amd64: go build started

k8s.io/kubernetes/pkg/util/mount

pkg/util/mount/mount.go:89 http://k8s.io/kubernetes/pkg/util/mountpkg/util/mount/mount.go:89: cannot use Mounter literal (type *Mounter) as type Interface in return argument:
*Mounter does not implement Interface (missing GetDeviceNameFromMount method)

k8s.io/kubernetes/pkg/util/procfs

pkg/util/procfs/procfs.go:84 http://k8s.io/kubernetes/pkg/util/procfspkg/util/procfs/procfs.go:84: undefined: syscall.Kill

It looks like you're missing changes to pkg/util/mount/mount_
unsupported.go?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#27970 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ASSNxYkn7kpGz9wl1xKaYX7PYXHy8Mr1ks5qgjz5gaJpZM4I9M0t
.

Jing

#30724-upstream-release-1.3 Automatic merge from submit-queue Automated cherry pick of #27970 #30724 Cherry pick of #27970 #30724 on release-1.3.

ixdy · 2016-08-19T02:26:44Z

@jingxu97 did you have a chance to create a PR for this yet? build still seems broken.

jingxu97 · 2016-08-19T05:54:38Z

Hi Jeff

Yes, #30724
This PR is already merged.

From error log message you sent in previous email, there is an error which
I don't think related to my change

k8s.io/kubernetes/pkg/util/procfs

pkg/util/procfs/procfs.go:84
http://k8s.io/kubernetes/pkg/util/procfspkg/util/procfs/procfs.go:84:
undefined: syscall.Kill

Please let me know if there is any issue. Thank you!

On Thu, Aug 18, 2016 at 7:27 PM, Jeff Grafton notifications@github.com
wrote:

@jingxu97 https://github.com/jingxu97 did you have a chance to create a
PR for this yet? build still seems broken.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#27970 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ASSNxZN__MH5SgdHKXrMO79TiK5hSzQbks5qhRSXgaJpZM4I9M0t
.

Jing

luxas · 2016-08-31T05:14:12Z

pkg/kubelet/kubelet.go

 			glog.V(3).Infof("Orphaned pod %q found, but volumes are not cleaned up; err: %v", uid, err)
 			continue
 		}
+		// Check whether volume is still mounted on disk. If so, do not delete directory
+		if volumeNames, err := kl.getPodVolumeNameListFromDisk(uid); err != nil || len(volumeNames) != 0 {
+			glog.V(3).Infof("Orphaned pod %q found, but volumes are still mounted; err: %v, volumes: %v ", uid, err, volumeNames)


Why not unmount unused mounted volumes here?
I think that's what #31596 is asking for

@luxas https://github.com/luxas, we do unmount unused mounted volumes,
this logic is implemented in pkg/kubelet/volumemanager/reconciler/reconciler.go
because this is the place for all mount/unmount related logic.

Please let me know if you have any questions about it.

Jing

On Tue, Aug 30, 2016 at 10:15 PM, Lucas Käldström notifications@github.com
wrote:

In pkg/kubelet/kubelet.go
#27970 (comment)
:

glog.V(3).Infof("Orphaned pod %q found, but volumes are not cleaned up; err: %v", uid, err) continue }

// Check whether volume is still mounted on disk. If so, do not delete directory

if volumeNames, err := kl.getPodVolumeNameListFromDisk(uid); err != nil || len(volumeNames) != 0 {

glog.V(3).Infof("Orphaned pod %q found, but volumes are still mounted; err: %v, volumes: %v ", uid, err, volumeNames)

Why not unmount unused mounted volumes here?
I think that's what #31596
#31596 is asking for

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubernetes/pull/27970/files/f19a1148db1b7584be6b6b60abaf8c0bd1503ed3#r76925660,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ASSNxZ2PPiHykY7mWc1u4e7Uagxqpgsgks5qlQ3WgaJpZM4I9M0t
.

Jing

…ck-of-#27970-kubernetes#30724-upstream-release-1.3 Automatic merge from submit-queue Automated cherry pick of kubernetes#27970 kubernetes#30724 Cherry pick of kubernetes#27970 kubernetes#30724 on release-1.3.

Automatic merge from submit-queue Add volume reconstruct/cleanup logic in kubelet volume manager Currently kubelet volume management works on the concept of desired and actual world of states. The volume manager periodically compares the two worlds and perform volume mount/unmount and/or attach/detach operations. When kubelet restarts, the cache of those two worlds are gone. Although desired world can be recovered through apiserver, actual world can not be recovered which may cause some volumes cannot be cleaned up if their information is deleted by apiserver. This change adds the reconstruction of the actual world by reading the pod directories from disk. The reconstructed volume information is added to both desired world and actual world if it cannot be found in either world. The rest logic would be as same as before, desired world populator may clean up the volume entry if it is no longer in apiserver, and then volume manager should invoke unmount to clean it up. Fixes kubernetes#27653

googlebot added the cla: yes label Jun 23, 2016

k8s-github-robot assigned yujuhong Jun 23, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jun 23, 2016

yujuhong assigned saad-ali Jun 23, 2016

yujuhong added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jun 23, 2016

yujuhong added this to the v1.3 milestone Jun 23, 2016

yujuhong reviewed Jun 23, 2016
View reviewed changes

eparis added the cherrypick-candidate label Jun 24, 2016

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

erictune removed the cherrypick-candidate label Jun 29, 2016

matchstick modified the milestones: next-candidate, v1.3 Jun 29, 2016

matchstick added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jun 29, 2016

jingxu97 force-pushed the restartKubelet-6-22 branch from e867383 to f4857df Compare July 20, 2016 00:55

k8s-github-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 20, 2016

jingxu97 force-pushed the restartKubelet-6-22 branch from 8fd2e93 to f19a114 Compare August 15, 2016 18:29

thockin removed their assignment Aug 15, 2016

jingxu97 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2016

k8s-github-robot merged commit 79ed706 into kubernetes:master Aug 15, 2016

ixdy mentioned this pull request Aug 16, 2016

Remove kubelet pkill dependency #30087

Merged

ixdy mentioned this pull request Aug 16, 2016

We need a make release job kubernetes/test-infra#393

Closed

jingxu97 mentioned this pull request Aug 17, 2016

Automated cherry pick of #27970 #30724 #30824

Merged

fabioy added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed cherrypick-candidate labels Aug 18, 2016

saad-ali mentioned this pull request Aug 18, 2016

Unmount a path that is no longer mounted fails the following clean up sequence #30930

Closed

jingxu97 mentioned this pull request Aug 30, 2016

Error during cleaning up orphaned pods #31596

Closed

luxas reviewed Aug 31, 2016
View reviewed changes

jingxu97 mentioned this pull request Sep 29, 2016

Volume status out of sync when kubelet restarts #33203

Closed

childsb mentioned this pull request Nov 28, 2016

Add volume reconstruct/cleanup logic in kubelet volume manager openshift/origin#12024

Merged

NickrenREN mentioned this pull request Mar 22, 2017

cleanup: remove TODO(resolved) and var(unused) #43432

Merged

jsafrane mentioned this pull request Jun 14, 2017

Make kubelet never delete files on mounted filesystems #37698

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add volume reconstruct/cleanup logic in kubelet volume manager #27970

Add volume reconstruct/cleanup logic in kubelet volume manager #27970

jingxu97 commented Jun 23, 2016 •

edited by k8s-oncall

Loading

yujuhong commented Jun 23, 2016

yujuhong Jun 23, 2016

erictune commented Jun 25, 2016

jingxu97 commented Jun 25, 2016

idvoretskyi commented Jun 29, 2016

k8s-bot commented Aug 15, 2016

k8s-github-robot commented Aug 15, 2016

k8s-bot commented Aug 15, 2016

k8s-github-robot commented Aug 15, 2016

ixdy commented Aug 16, 2016

ixdy commented Aug 16, 2016

ixdy commented Aug 16, 2016

jingxu97 commented Aug 16, 2016

k8s.io/kubernetes/pkg/util/mount

k8s.io/kubernetes/pkg/util/mount

k8s.io/kubernetes/pkg/util/procfs

ixdy commented Aug 19, 2016

jingxu97 commented Aug 19, 2016

luxas Aug 31, 2016

jingxu97 Aug 31, 2016

Add volume reconstruct/cleanup logic in kubelet volume manager #27970

Add volume reconstruct/cleanup logic in kubelet volume manager #27970

Conversation

jingxu97 commented Jun 23, 2016 • edited by k8s-oncall Loading

yujuhong commented Jun 23, 2016

yujuhong Jun 23, 2016

Choose a reason for hiding this comment

erictune commented Jun 25, 2016

jingxu97 commented Jun 25, 2016

idvoretskyi commented Jun 29, 2016

k8s-bot commented Aug 15, 2016

k8s-github-robot commented Aug 15, 2016

k8s-bot commented Aug 15, 2016

k8s-github-robot commented Aug 15, 2016

ixdy commented Aug 16, 2016

ixdy commented Aug 16, 2016

ixdy commented Aug 16, 2016

jingxu97 commented Aug 16, 2016

k8s.io/kubernetes/pkg/util/mount

k8s.io/kubernetes/pkg/util/mount

k8s.io/kubernetes/pkg/util/procfs

ixdy commented Aug 19, 2016

jingxu97 commented Aug 19, 2016

k8s.io/kubernetes/pkg/util/procfs

luxas Aug 31, 2016

Choose a reason for hiding this comment

jingxu97 Aug 31, 2016

Choose a reason for hiding this comment

jingxu97 commented Jun 23, 2016 •

edited by k8s-oncall

Loading