cloud-provider-openstack icon indicating copy to clipboard operation
cloud-provider-openstack copied to clipboard

[cinder-csi-plugin] Rationalise all things topology

Open stephenfin opened this issue 8 months ago • 2 comments

What this PR does / why we need it:

This is a patch that came out of my work on https://github.com/kubernetes/cloud-provider-openstack/issues/2861. Currently, generation of the topology request for the Cinder Volume is logically separated from the generation of the topology request for the CSI Volume. This makes things harder to understand. Move some things around to resolve this and make everything a little easier to grok. This is kept separate from https://github.com/kubernetes/cloud-provider-openstack/pull/2862 since I don't expect we'll want to backport this.

Which issue this PR fixes(if applicable):

n/a

Special notes for reviewers:

I have a decoder of the logic embedded in the commit message. Hopefully this helps with review, but please don't assume the decoder itself is correct! :sweat_smile: I have also added additional unit tests that should capture any changes in logic.

Release note:

NONE

stephenfin avatar Mar 24 '25 18:03 stephenfin

I wonder if it's worth splitting the last commit, which actually changes the behaviour of --with-topology into a separate PR. Whether it's worth it might depend on the next point.

I also wonder if we need to guard this behaviour change somehow: has --with-topology been included in a release yet? If not I think we're good, but if it has I don't think we can make a breaking change like this without guardrails.

Good point. I've dropped this. We would want a release note for the change.

stephenfin avatar Apr 03 '25 17:04 stephenfin

/assign

mandre avatar Aug 29 '25 14:08 mandre

/unassign @mdbooth

stephenfin avatar Aug 29 '25 15:08 stephenfin

/approve

kayrus avatar Sep 18 '25 09:09 kayrus

[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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Sep 18 '25 09:09 k8s-ci-robot