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
Update vSphere CSI driver to v2.7.0 #11517
Conversation
/retest |
1 similar comment
/retest |
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.
Please replace all occurrences of k8s.gcr.io
with registry.k8s.io
.
pkg/resources/csi/vsphere/secret.go
Outdated
@@ -50,7 +51,7 @@ func CloudConfigSecretNameReconciler(data *resources.TemplateData) reconciling.N | |||
return nil, err | |||
} | |||
|
|||
cloudConfig, err := CloudConfigCSIToString(vsphereCloudConfig) | |||
cloudConfig, err := vsphere.CloudConfigToString(vsphereCloudConfig) |
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.
Is this using the regular cloud-config? cloud-config for the CCM and the CSI is different, hence the CloudConfigCSIToString
function. I don't think we should get rid of this function.
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.
Your are probably right with this one. It makes more sense to unify this in another PR as well. Right now we have multiple places where we maintain the conversion.
/retest |
pkg/controller/user-cluster-controller-manager/resources/resources/csi/migration/secret.go
Outdated
Show resolved
Hide resolved
pkg/controller/user-cluster-controller-manager/resources/resources/csi/migration/webhook.go
Show resolved
Hide resolved
@WeirdMachine LGTM, but please fix the verify job. |
@xmudrii PTALA |
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
for manual testing
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: WeirdMachine, xmudrii The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@WeirdMachine: The following tests failed, say
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. I understand the commands that are listed here. |
@@ -264,6 +263,21 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, addo | |||
return reqeueAfter, nil | |||
} | |||
|
|||
// This handles manifests that should be removed manually, because kubectl --prune does not always delete obsolete resources. |
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.
Please explain more why this is here. Is kubectl broken? What is this "not always"?
/lgtm cancel
Name: "internal-delete", | ||
}, | ||
Spec: kubermaticv1.AddonSpec{ | ||
Name: "internal-delete", |
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 do not understand what is happening here... Why are we deleting an internal-delete
addon when the CSI addon is being deleted?
Looking at this PR I am ashamed of myself. Therefore the csi driver is updated here: #11724 And the namespace migration should be done in another way. |
What this PR does / why we need it:
Updates all CSI components for vsphere to version 2.7.0
This includes Snapshot controller.
In addition the namespace for the csi driver to work with all its features is required to be
vmware-system-csi
. This PR migrated the csi driver component to its desired namespace.Usually we remove obsolete addons via
kubectl --prune
however this does not work between namespaces. Therefore a manual deletion of the resources in the old namespace is needed.Note that
--prune
is an alpha/beta command. More information here:https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands
Which issue(s) this PR fixes:
Fixes #10719
What type of PR is this?
/kind cleanup
/kind chore
Special notes for your reviewer:
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: