Skip to content

Conversation

@jsafrane
Copy link
Contributor

Recycler should return an error when a recycled volume is not empty - another pod might have written some data into it. This directory should be recycled again (and Kubernetes will do that in few seconds).

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1395271

Recycler should return an error when a recycled volume is not empty
- another pod might have written some data into it. This directory should
be recycled again (and Kubernetes will do that in few seconds).
@jsafrane jsafrane added this to the 1.4.0 milestone Nov 16, 2016
@jsafrane jsafrane force-pushed the fix-recycler-exit-code branch 2 times, most recently from 576849f to 85ce222 Compare November 16, 2016 12:51
@jsafrane
Copy link
Contributor Author

[test]

@jsafrane
Copy link
Contributor Author

[test]
flake: #10988

1 similar comment
@jsafrane
Copy link
Contributor Author

[test]
flake: #10988

@jsafrane
Copy link
Contributor Author

[test] flake: #9767

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 85ce222

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11547/) (Base Commit: 32c2948)

@jsafrane
Copy link
Contributor Author

@childsb, PTAL

@ncdc
Copy link
Contributor

ncdc commented Nov 22, 2016

Is this still for 1.4?

@jsafrane
Copy link
Contributor Author

@ncdc yes please!

@childsb
Copy link
Contributor

childsb commented Nov 22, 2016

This is a tricky PR. Code & design is great.. but there are filesystems that contain directories that are impossible to delete (.snapshot , .backup, etc..)

All that to say, this is another problem with the recycler. There's no correct behavior for every FS type.

@jsafrane
Copy link
Contributor Author

there are filesystems that contain directories that are impossible to delete (.snapshot , .backup, etc..)

What does rmdir on these directories actually do? Does it return EPERM or similar error? If so, then the volumes were not recyclable even before.

@childsb
Copy link
Contributor

childsb commented Nov 23, 2016

I think it does not return error, but the directory is never deleted. lets see if rootfs knows.

@eparis
Copy link
Member

eparis commented Jan 28, 2017

@jsafrane What do we need to do to get this merged? @rootfs to review?


// CheckEmpty returns error if specified directory is not empty.
func CheckEmpty(dir string) error {
return newWalker(func(path string, info os.FileInfo) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are directories with different fsid (e.g. snapshot volumes), ignore them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, it's single NFS mount, ".snapshot" (or whatever is it called) won't have a different FSID.

@smarterclayton smarterclayton modified the milestones: 1.4.0, 1.5.0 Jan 31, 2017
@eparis
Copy link
Member

eparis commented Feb 1, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 85ce222

@openshift-bot
Copy link
Contributor

openshift-bot commented Feb 1, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13472/) (Base Commit: 708ea98) (Image: devenv-rhel7_5831)

@openshift-bot openshift-bot merged commit 92ce2c9 into openshift:master Feb 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants