Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if pathExists before performing Unmount #39311

Merged
merged 1 commit into from
Jan 5, 2017

Conversation

rkouj
Copy link
Contributor

@rkouj rkouj commented Dec 29, 2016

Unmount operation should not fail if path does not exist

Part two of: #38547
Plugins status captured here: #39251

cc: @saad-ali

@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 29, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins Bazel Build failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. Full PR test history.

The magic incantation to run this job again is @k8s-bot bazel 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
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. 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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci 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
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke 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
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. 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
Copy link
Contributor

Jenkins GCE e2e failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. 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
Copy link
Contributor

Jenkins CRI GCE Node e2e failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. Full PR test history.

The magic incantation to run this job again is @k8s-bot cri 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
Copy link
Contributor

Jenkins GCE Node e2e failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. 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-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Dec 29, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 264530f60d0ae3a018563e37335aee601cd46e1f. 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.

@rkouj rkouj force-pushed the refactor-tear-down-at branch from 264530f to f76fa39 Compare December 30, 2016 01:12
Copy link
Member

@saad-ali saad-ali left a comment

Choose a reason for hiding this comment

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

Also add the check to emptyDir itself

@saad-ali saad-ali assigned saad-ali and unassigned pmorie Dec 30, 2016
@rkouj rkouj force-pushed the refactor-tear-down-at branch from f76fa39 to 8cec46e Compare December 30, 2016 02:06
@rkouj
Copy link
Contributor Author

rkouj commented Dec 30, 2016

Ack

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 8cec46e. 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
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 8cec46e. Full PR test history.

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

@rkouj
Copy link
Contributor Author

rkouj commented Dec 30, 2016

@k8s-bot unit test this

@rkouj
Copy link
Contributor Author

rkouj commented Dec 30, 2016

@k8s-bot cvm gke e2e test this

@rkouj
Copy link
Contributor Author

rkouj commented Dec 30, 2016

@saad-ali PTAL

@saad-ali saad-ali added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jan 4, 2017
@saad-ali
Copy link
Member

saad-ali commented Jan 4, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 4, 2017
@saad-ali
Copy link
Member

saad-ali commented Jan 4, 2017

Can you make sure to cherry-pick this and #38547 to 1.5 and possibly 1.4

@rkouj
Copy link
Contributor Author

rkouj commented Jan 4, 2017

@k8s-bot aws e2e test this

@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit eb8739d into kubernetes:master Jan 5, 2017
@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@jingxu97 jingxu97 added this to the v1.5 milestone Jan 6, 2017
@saad-ali saad-ali added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jan 9, 2017
k8s-github-robot pushed a commit that referenced this pull request Jan 10, 2017
…upstream-release-1.5

Automatic merge from submit-queue

Automated cherry pick of #39311

Cherry pick of #39311 on release-1.5.

#39311: Check if pathExists before performing Unmount
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.5" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

k8s-github-robot pushed a commit that referenced this pull request Jan 12, 2017
Automatic merge from submit-queue (batch tested with PRs 39768, 39463)

Check if path exists before performing unmount

This is part 3 of an effort to check if path exists before performing an unmount operation.
[Part 1](#38547) and [part 2](#39311) involved auditing the different volume plugins and refactoring their `TearDownAt()s` to use the common util function/or create one if absent.

The ideal way to do this change would involve refactoring of the `TearDownAt()s` of these plugins and make a common util function that checks path. (The plugins involved in this PR use someway of unmounting a bind mount and unmounting a global path, there is also refactoring needed to consolidate disk_manager of fc, rbd and iscsi). A non-goal part of this effort can also involve refactoring all the `SetupAt()s`

In the interest of time and considering other higher priority issues that I am caught up with, I am unable to give the time the refactoring needs. Hence I've made the minimum change that would give the desired output.

I am tracking the work pending in this issue: #39251

```release-note
NONE
```
@jessfraz jessfraz added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jan 25, 2017
jessfraz added a commit that referenced this pull request Mar 14, 2017
dims pushed a commit to dims/openstack-cloud-controller-manager that referenced this pull request Sep 6, 2017
Automatic merge from submit-queue (batch tested with PRs 39768, 39463)

Check if path exists before performing unmount

This is part 3 of an effort to check if path exists before performing an unmount operation.
[Part 1](kubernetes/kubernetes#38547) and [part 2](kubernetes/kubernetes#39311) involved auditing the different volume plugins and refactoring their `TearDownAt()s` to use the common util function/or create one if absent.

The ideal way to do this change would involve refactoring of the `TearDownAt()s` of these plugins and make a common util function that checks path. (The plugins involved in this PR use someway of unmounting a bind mount and unmounting a global path, there is also refactoring needed to consolidate disk_manager of fc, rbd and iscsi). A non-goal part of this effort can also involve refactoring all the `SetupAt()s`

In the interest of time and considering other higher priority issues that I am caught up with, I am unable to give the time the refactoring needs. Hence I've made the minimum change that would give the desired output.

I am tracking the work pending in this issue: kubernetes/kubernetes#39251

```release-note
NONE
```
dims pushed a commit to dims/openstack-cloud-controller-manager that referenced this pull request Jan 13, 2018
Automatic merge from submit-queue (batch tested with PRs 39768, 39463)

Check if path exists before performing unmount

This is part 3 of an effort to check if path exists before performing an unmount operation.
[Part 1](kubernetes/kubernetes#38547) and [part 2](kubernetes/kubernetes#39311) involved auditing the different volume plugins and refactoring their `TearDownAt()s` to use the common util function/or create one if absent.

The ideal way to do this change would involve refactoring of the `TearDownAt()s` of these plugins and make a common util function that checks path. (The plugins involved in this PR use someway of unmounting a bind mount and unmounting a global path, there is also refactoring needed to consolidate disk_manager of fc, rbd and iscsi). A non-goal part of this effort can also involve refactoring all the `SetupAt()s`

In the interest of time and considering other higher priority issues that I am caught up with, I am unable to give the time the refactoring needs. Hence I've made the minimum change that would give the desired output.

I am tracking the work pending in this issue: kubernetes/kubernetes#39251

```release-note
NONE
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants