-
Notifications
You must be signed in to change notification settings - Fork 648
[release-1.31] [cinder-csi-plugin] Don't report topology capability when --with-topology=False #2872
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
[release-1.31] [cinder-csi-plugin] Don't report topology capability when --with-topology=False #2872
Conversation
These failures are due to the version of golangci-lint we use. I've proposed #2874 to fix this. We'll need to merge that first. |
%T prints the type. %t prints the word true/false, which is what we want. [1] [1] https://pkg.go.dev/fmt Conflicts: pkg/csi/manila/driver.go NOTE(stephenfin): Merge conflicts are due to the absence of PR kubernetes#2734, which we don't want to backport. Signed-off-by: Stephen Finucane <[email protected]>
…ogy=False The 'PluginCapability_Service_VOLUME_ACCESSIBILITY_CONSTRAINTS' capability flag determines whether the provisioner attempts to look up topology information from the node or not [1][2]. If we report it but don't return topology information from the node, we end up with failures to provision [3]. Fix the issue by optionally reporting the capability, like Manila already does. [1] https://github.com/kubernetes-csi/external-provisioner/blob/17e2429e9f/pkg/controller/controller.go#L685-L700 [2] https://github.com/kubernetes-csi/external-provisioner/blob/17e2429e9f/pkg/controller/controller.go#L994-L996 [3] https://github.com/kubernetes-csi/external-provisioner/blob/17e2429e9f/pkg/controller/topology.go#L177 Signed-off-by: Stephen Finucane <[email protected]>
Signed-off-by: Stephen Finucane <[email protected]>
6504e5c
to
e4f658a
Compare
That's merged now, so I've rebased and force pushed this. |
Jobs are failing because the |
Waiting on #2742 |
/test openstack-cloud-csi-manila-e2e-test-release-131 |
/test openstack-cloud-csi-cinder-e2e-test-release-131 |
/test openstack-cloud-csi-manila-e2e-test-release-131 |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kayrus 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 |
This is a manual backport of #2862 (
master
) and #2871 (release-1.32
). As noted in the first commit, the merge conflict was trivial and due to the absence of #2734, which added a log into the Manila CSI Driver.