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

fixes #5, add a finalizer to fix deletion during pv protection #6

Merged
merged 1 commit into from
Jan 7, 2019

Conversation

rmb938
Copy link
Contributor

@rmb938 rmb938 commented Dec 21, 2018

#5

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 21, 2018
@rmb938 rmb938 force-pushed the customfinalizer branch 3 times, most recently from 0cf25bc to 00f37cf Compare December 21, 2018 03:00
@rmb938
Copy link
Contributor Author

rmb938 commented Dec 21, 2018

It seems the tests are failing on getting the test-pod. Not sure what's going on any ideas?

@wongma7
Copy link
Contributor

wongma7 commented Dec 21, 2018

thanks for figuring out the issue and this patch, don't worry about the test I am looking into it

@wongma7
Copy link
Contributor

wongma7 commented Dec 21, 2018

@rmb938 the finalizer name needs changing
"I1221 20:56:35.169970 1 controller.go:1075] provision "default/hostpath-pvc" class "example-hostpath": failed to save persistentvolume "pvc-da7cac17-0562-11e9-b559-b2cbe663ac83": PersistentVolume "pvc-da7cac17-0562-11e9-b559-b2cbe663ac83" is invalid: metadata.finalizers[0]: Invalid value: "external-provisioner.volume.kubernetes.io": name is neither a standard finalizer name nor is it fully qualified"

How it worked locally for me, I have no idea!

@rmb938
Copy link
Contributor Author

rmb938 commented Dec 21, 2018

Huh strange...

It seems finalizers have changed in 1.13 to have the syntax of label names. So I'll have to change it to something like external-provisioner.volume.kubernetes.io/finalizer

@wongma7
Copy link
Contributor

wongma7 commented Dec 21, 2018

/lgtm
/approve
thanks again & happy holidays.

feel free to squash if you like, I will re-add lgtm.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 21, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rmb938, wongma7

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 Dec 21, 2018
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 21, 2018
@rmb938
Copy link
Contributor Author

rmb938 commented Dec 21, 2018

@wongma7 Squashed and rebased to latest master. What is the process on getting another release? I would like to update https://github.com/kubernetes-csi/external-provisioner to get this change incorporated.

@rmb938
Copy link
Contributor Author

rmb938 commented Jan 2, 2019

@wongma7 Could we get this merged and released?

@wongma7
Copy link
Contributor

wongma7 commented Jan 7, 2019

/lgtm
yes, sorry for the delay, a release will be tagged today

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 7, 2019
@k8s-ci-robot k8s-ci-robot merged commit 844c30e into kubernetes-sigs:master Jan 7, 2019
@rmb938 rmb938 deleted the customfinalizer branch January 7, 2019 16:39
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. 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.

3 participants