-
Notifications
You must be signed in to change notification settings - Fork 153
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
Migrate vSphere CSIDriver
resource on upgrade to match upstream
#12813
Conversation
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
/test pre-kubermatic-e2e-vsphere-ubuntu-1.28 |
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
/test pre-kubermatic-e2e-vsphere-ubuntu-1.28 |
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.
/lgtm
/approve
/hold
feel free to unhold if those nits are irrelevant
LGTM label has been added. Git tree hash: 93a93895b3e16b55a8df2751dab2436d2280c835
|
/hold cancel Since there's no full approval yet anyway. |
pkg/controller/seed-controller-manager/addon/addon_controller.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
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.
/approve
LGTM label has been added. Git tree hash: 06c6b9066de83b283c7d4ed124fc7ab8675a3e8f
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wozniakjan, xrstf 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 |
What this PR does / why we need it:
Settle down kids because this story might take a while.
Long back, #7280 added support for vSphere external CCM and CSI driver. CSI support was based on the 2.2.1 release of vsphere-csi-driver. For some reason that has been lost to history and the unwillingness to write proper PR descriptions, the original PR set
volumeLifecycleModes
on theCSIDriver
resource. This was not set in the corresponding upstream manifests for v2.2.1, and I cannot find any meaningful references tovolumeLifecycleModes
in the upstream or k8c issue trackers.We have run into this exact problem (as described in #12801) once already: #8769 reverted settings to ensure that the
CSIDriver
object could be reconciled. I also don't see any reason why this needs to be set in those PRs, I suspect it was done simply because of the exact same error this PR is now trying to solve permanently.#12593 then upgraded the CSI driver to 3.0, and removed the
volumeLifecycleModes
settings from theCSIDriver
again because the change was not documented and not matching upstream manifests.I've done some additional research and believe that the CSI driver does not support ephemeral volumes (which is indicated by
Ephemeral
involumeLifecycleModes
), so the oldCSIDriver
object is incorrect.This PR adapts logic implemented in #12432 to also clean up the vSphere
CSIDriver
object to allow redeployment.Which issue(s) this PR fixes:
Fixes #12801
What type of PR is this?
/kind bug
/kind regression
/kind cleanup
Special notes for your reviewer:
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: