-
Notifications
You must be signed in to change notification settings - Fork 133
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
fix: do not emit VolumeResizeFailed
on conflict while trying to set FileSystemResizeRequired
or VolumeResizeSuccess
#392
Conversation
Welcome @jakobmoellerdev! |
Hi @jakobmoellerdev. Thanks for your PR. I'm waiting for a kubernetes-csi 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. |
873774d
to
cde4252
Compare
FileSystemResizeRequired
or VolumeResizeSuccess
VolumeResizeFailed
on conflict while trying to set FileSystemResizeRequired
or VolumeResizeSuccess
/ok-to-test |
cde4252
to
21d9bdd
Compare
21d9bdd
to
db19140
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jakobmoellerdev, jsafrane 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 |
A workaround to avoid the unexpected test failure in the resize test is no longer needed. cf. kubernetes-csi/external-resizer#392 Signed-off-by: Shinya Hayashi <[email protected]>
A workaround to avoid the unexpected test failure in the resize test is no longer needed. cf. kubernetes-csi/external-resizer#392 Signed-off-by: Shinya Hayashi <[email protected]>
A workaround to avoid the unexpected test failure in the resize test is no longer needed. cf. kubernetes-csi/external-resizer#392 Signed-off-by: Shinya Hayashi <[email protected]>
What type of PR is this?
What this PR does / why we need it:
When doing a CSI call to ExpandVolume, external-resizer tries to update the PVC with
FilesystemResizeRequired
. Usually this works fine, however, in case of a PVC conflict, the external-resizer emits an event on the PVC withVolumeResizeFailed
. This event is misleading because the Resize did not actually fail, merely the update to the PVC as requiring the Resize failed. Since this will almost always (unless we face endless races) recover gracefully for conflicts, we should avoid sending the event in this case, and rather recover normally. The logs will still emit a sync failure so admins will still be able to notice races, the event will just no longer be published.Here is an example we collected during test runs in the downstream CSI driver TopoLVM:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: