Skip to content

Conversation

@engedaam
Copy link
Contributor

What type of PR is this?

/kind feature

This change extends the EBS CSI driver plugin interface to support custom node topology segments that can be propagated to Kubernetes resources.

What is this PR about? / Why do we need it?

  • Added GetNodeSegments() method to the EbsCsiPlugin interface, allowing plugins to provide additional topology segments
  • Introduced SetNodeSegments() and GetNodeSegments() utility functions to manage plugin-provided segments globally
  • Updated controller and node services to merge plugin segments with existing topology information (zone, OS, outpost ARN)
  • Modified all relevant tests to account for the new segment merging behavior using lo.Assign()

Dependencies:

  • Added github.com/samber/lo for convenient map merging operations

How was this change tested?

  • make test

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. labels Jan 24, 2026
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 24, 2026
@engedaam engedaam changed the title Expand plugin interface GetSegments() feat: Expand plugin interface GetSegments() Jan 24, 2026
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 24, 2026
@engedaam engedaam force-pushed the getsegment-interface branch from 3f043e5 to 0959e9b Compare January 24, 2026 02:13
@github-actions
Copy link

github-actions bot commented Jan 24, 2026

Code Coverage Diff

File Old Coverage New Coverage Delta
github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/node.go 89.5% 88.3% -1.2
github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/util/util.go 56.3% 51.3% -5.1

@engedaam engedaam force-pushed the getsegment-interface branch from 0959e9b to e393546 Compare January 26, 2026 19:40
@engedaam engedaam force-pushed the getsegment-interface branch from e393546 to 1a3db16 Compare January 26, 2026 20:01
@engedaam
Copy link
Contributor Author

/retest

@k8s-ci-robot
Copy link
Contributor

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Details

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 26, 2026
@engedaam engedaam force-pushed the getsegment-interface branch from 6765ac5 to 926b09a Compare January 26, 2026 22:30
@engedaam
Copy link
Contributor Author

/retest

Copy link
Member

@torredil torredil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested changes, also make verify is failing and needs to be fixed before merge.

AccessibleTopology: []*csi.Topology{
{
Segments: map[string]string{WellKnownZoneTopologyKey: expZone},
Segments: lo.Assign(map[string]string{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetNodeSegments() returns an empty map here since nothing calls SetNodeSegments with actual data, so this test passes whether or not the merge works. Can we add a test that sets some segments and checks they show up in the response?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is expected. Once a consumer has loaded the test with a plugin the segments will be updated to the correct value. This is how the remaining of the tests have been configured for the plugin

@torredil
Copy link
Member

/assign @ConnorJC3

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from connorjc3. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@engedaam engedaam force-pushed the getsegment-interface branch 5 times, most recently from edca3bb to 2b957c9 Compare January 29, 2026 19:32
@engedaam engedaam force-pushed the getsegment-interface branch from 2b957c9 to 48384f8 Compare January 29, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants