Revert "Use patch to replace update"#157
Conversation
This reverts commit f953647. PATCH needs new RBAC rules and that effectively breaks API.
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
|
/assign @davidz627 @ddebroy |
davidz627
left a comment
davidz627
left a comment
There was a problem hiding this comment.
LGTM mod comment fix. Odd that we don't retry Update errors but I remember something about just letting the caller retry the whole attach operation (?) Anyway it's not related to this revert.
| func (h *csiHandler) saveVA(va *storage.VolumeAttachment, patch []byte) (*storage.VolumeAttachment, error) { | ||
| newVA, err := h.client.StorageV1beta1().VolumeAttachments().Patch(va.Name, types.MergePatchType, patch) | ||
| func (h *csiHandler) saveVA(va *storage.VolumeAttachment) (*storage.VolumeAttachment, error) { | ||
| // TODO: use patch to save us from VersionError |
There was a problem hiding this comment.
can you link an issue (with a link to the original reverted Patch PR) for context
|
BTW, opened #158 to keep track of changes we should do when we're ready for a 2.0 |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
5 participants
This reverts commit f953647.
PATCH usage introduced in #149 needs new RBAC rules and that effectively breaks API. We don't want to bump the image to v2.0 (yet).
ddebroy @davidz627 PTAL, I think I fixed in-line attachment unit tests correctly.
@cwdsuzhou, sorry, your contribution is great, however we'd prefer to be on the safe side. We'll keep your patch around when v2.0 time comes.
/kind api-change
(?)
Does this PR introduce a user-facing change?: