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 for NotFound fault during DeleteVolume #80

Merged
merged 1 commit into from
Oct 31, 2019
Merged

Check for NotFound fault during DeleteVolume #80

merged 1 commit into from
Oct 31, 2019

Conversation

chethanv28
Copy link
Collaborator

@chethanv28 chethanv28 commented Oct 8, 2019

What this PR does / why we need it:
For delete operation invoked explicitly for PVCs which have no FCD in the backend, VC does not trigger a task and it returns "Fault". This PR will introduce a check for the type of FAULT ==> specifically NOTFOUND and write this info in the log and return nil to safely complete the delete operation.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:
Added logs below to indicate how not found scenarios will behave with the change

Release note:

Check for NotFound fault in DeleteVolume

Logs:

{"log":"I0905 21:23:10.700394       1 manager.go:311] DeleteVolume: volumeID: \"26e74e3f-98ac-45a9-ad20-812222d7e098\", opId: \"0fdd675e\"\n","stream":"stderr","time":"2019-09-05T21:23:10.701551387Z"}
{"log":"I0905 21:23:10.700499       1 manager.go:331] DeleteVolume: Volume deleted successfully. volumeID: \"26e74e3f-98ac-45a9-ad20-812222d7e098\", opId: \"0fdd675e\"\n","stream":"stderr","time":"2019-09-05T21:23:10.70155599Z"}
{"log":"I0905 21:23:10.700513       1 vsphereutil.go:161] Successfully deleted disk for volumeid: 26e74e3f-98ac-45a9-ad20-812222d7e098\n","stream":"stderr","time":"2019-09-05T21:23:10.701559396Z"}
{"log":"DeleteVolume called: %s 26e74e3f-98ac-45a9-ad20-812222d7e098\n","stream":"stdout","time":"2019-09-05T21:23:24.321170845Z"}
{"log":"I0905 21:23:24.313152       1 controller.go:306] ControllerGetCapabilities: called with args {XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}\n","stream":"stderr","time":"2019-09-05T21:23:24.321253147Z"}
{"log":"I0905 21:23:24.319077       1 controller.go:149] DeleteVolume: called with args: {VolumeId:26e74e3f-98ac-45a9-ad20-812222d7e098 Secrets:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}\n","stream":"stderr","time":"2019-09-05T21:23:24.32126154Z"}
{"log":"I0905 21:23:24.319128       1 vsphereutil.go:155] vSphere Cloud Provider deleting volume: 26e74e3f-98ac-45a9-ad20-812222d7e098\n","stream":"stderr","time":"2019-09-05T21:23:24.321264872Z"}
{"log":"I0905 21:23:24.410204       1 vsphereutil.go:161] Successfully deleted disk for volumeid: 26e74e3f-98ac-45a9-ad20-812222d7e098\n","stream":"stderr","time":"2019-09-05T21:23:24.414882662Z"}
****************************************************************************************************************************************************************************************************************************************************************
{"log":"E0906 06:46:24.706823       1 manager.go:292] VolumeID: \"c8e231a2-95ab-490e-8053-cc56609aba51\", not found. Returning success for this operation since the volume is not present\n","stream":"stderr","time":"2019-09-06T06:46:24.707203568Z"}
****************************************************************************************************************************************************************************************************************************************************************

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

Hi @chethanv28. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 8, 2019
@chethanv28 chethanv28 changed the title Check for NotFound fault during DeleteVolume [WIP] Check for NotFound fault during DeleteVolume Oct 8, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 8, 2019
@codenrhoden
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 8, 2019
@codenrhoden
Copy link
Contributor

/test pull-vsphere-csi-driver-build
^ nothing to do with this PR, I just want it to re-run so I can look at a log file. :)
I asked @divyenpatel about this, and he's asked to hold off on merging until after 1.0.1 is tagged.
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 8, 2019
@codenrhoden
Copy link
Contributor

/test pull-vsphere-csi-driver-build
one more try... I'm checking that the build is using the latest csi-ci image, which I recently updated. This will ensure that everything is built with Go 1.12.10. We were using an older version previously.

@chethanv28 chethanv28 changed the title [WIP] Check for NotFound fault during DeleteVolume Check for NotFound fault during DeleteVolume Oct 24, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 24, 2019
@chethanv28
Copy link
Collaborator Author

chethanv28 commented Oct 26, 2019

@codenrhoden and @divyenpatel Please review and see if the "do-not-merge/on-hold" tag can be removed on this PR

@chethanv28
Copy link
Collaborator Author

/assign @divyenpatel
/assign @codenrhoden
/assign @dvonthenen

@divyenpatel
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 31, 2019
@divyenpatel
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chethanv28, divyenpatel

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 31, 2019
@divyenpatel
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 31, 2019
@k8s-ci-robot k8s-ci-robot merged commit ea3b770 into kubernetes-sigs:master Oct 31, 2019
gnufied pushed a commit to gnufied/vsphere-csi-driver that referenced this pull request Jun 5, 2023
OCPBUGS-13386: Fix use of GetNodeByName when using node's VMUUID (kubernetes-sigs#2387)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants