-
Notifications
You must be signed in to change notification settings - Fork 336
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
Add controller expand secret ref to PV #301
Add controller expand secret ref to PV #301
Conversation
Signed-off-by: Grant Griffiths <ggp493@gmail.com>
Signed-off-by: Grant Griffiths <ggp493@gmail.com>
Hi @ggriffiths. Thanks for your PR. I'm waiting for a kubernetes-csi or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
/assign msau42 |
/ok-to-test |
Renaming the secret is fine since resizing is still an alpha feature. Can you add a release note for this and note that it's a breaking change? |
@@ -47,6 +47,10 @@ | |||
# because of the lack of semantic versioning in Kubernetes. Therefore we | |||
# have to pick a certain release, otherwise we (more or less randomly) end | |||
# up with something older or master. | |||
[[override]] | |||
name = "k8s.io/api" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm surprised client-go didn't need to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I found this override block makes it possible. It's more of a temporary fix until all other dependencies update to k8s.io/api 1.15.
https://github.com/golang/dep/blob/master/docs/Gopkg.toml.md#override
/approve Will let @gnufied lgtm Can you also specify in the release note what the old key names were? |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ggriffiths, msau42 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 |
/lgtm |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds in the ControllerExpandSecretRef to the PV.
Which issue(s) this PR fixes:
Fixes #278
Special notes for your reviewer:
prefixedResizerSecretNameKey
toprefixedControllerExpandSecretNameKey
. This naming matches the other secret key names (nodePublishSecretNameKey etc).controller-expand-secret-name-key
andcontroller-expand-secret-namespace-key
(to match the other key naming).[[override]]
instead of updating all dependencies to also have the release-1.15 dep constraint.Does this PR introduce a user-facing change?: