Skip to content

Add multikeylistener#2372

Merged
fgiorgetti merged 14 commits intoskupperproject:mainfrom
c-kruse:add-multikeylistener
Mar 3, 2026
Merged

Add multikeylistener#2372
fgiorgetti merged 14 commits intoskupperproject:mainfrom
c-kruse:add-multikeylistener

Conversation

@c-kruse
Copy link
Contributor

@c-kruse c-kruse commented Feb 4, 2026

Implements #2301

Extends skupper with a new MultiKeyListener CRD. Incompatible with skupper-router builds without listenerAddress support.

@c-kruse c-kruse added the do-not-merge Work In Progress label Feb 4, 2026
@c-kruse c-kruse force-pushed the add-multikeylistener branch 2 times, most recently from a279432 to 64680a5 Compare February 6, 2026 19:24
@c-kruse c-kruse force-pushed the add-multikeylistener branch 6 times, most recently from 2afdd0e to 971f20b Compare February 18, 2026 17:31
Introduce the MultiKeyListener custom resource type with support for
priority-based routing across multiple routing keys. This commit adds:

- CRD schema definition (skupper_multikeylistener_crd.yaml)
- API types with Priority strategy (multikeylistener_types.go)
- Generated clientset, informers, and listers

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Add ListenerAddress entity and multiAddressStrategy field to support
priority-based routing.

- Add ListenerAddress type for mapping priority values to routing keys
- Add ListenerAddressMap to BridgeConfig alongside TcpListeners/TcpConnectors
- Extend TcpEndpoint with MultiAddressStrategy field
- Update AMQP management to query/create/delete listenerAddress entities
- Update config marshaling/unmarshaling for listenerAddress entities
- Add ListenerAddressDifference for config synchronization

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Add core MultiKeyListener support to the site bindings package

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Add the watcher infrastructure for MultiKeyListener resources

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Add MultiKeyListener handling to the kube site ExtendedBindings:

- Track multiKeyListenerHosts for service exposure management
- Implement updateBridgeConfigForMultiKeyListener with port mapping
- Add multiKeyListenerUpdated for exposing services
- Add multiKeyListenerDeleted for cleaning up exposed services
- Implement UpdateMultiKeyListener/GetMultiKeyListener methods
- Update bindings_test.go to include ListenerAddresses in expected configs

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Signed-off-by: Christian Kruse <christian@c-kruse.com>
Add status reconciliation that updates MultiKeyListener READY condition
based on matching connectors in the network. The MultiKeyListener becomes
READY when any routing key in its strategy has a reachable connector.

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Signed-off-by: Christian Kruse <christian@c-kruse.com>
Signed-off-by: Christian Kruse <christian@c-kruse.com>
Signed-off-by: Christian Kruse <christian@c-kruse.com>
- Add MultiKeyListener to inputs
- Wire up MultiKeyListener to SiteState for router config and status
  updates
- Extend system apply and system delete commands for MultiKeyListener
  resources.

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Adds a sample for operator bundles

Signed-off-by: Christian Kruse <christian@c-kruse.com>
multiAddressStrategy=priority instead of priorityFailover
listenerAddress.listener instead of listenerRef

Signed-off-by: Christian Kruse <christian@c-kruse.com>
Updates the multikeylistener service port logic to prefix port names.

Fixes bug where Listener and MultiKeyListener with the same name got
assigned the same router Port: e.g. two tcpListeners with same port.

Fixes bug where Service ports (unique by name) conflicted between a
Listener and MKL with the same name.

Signed-off-by: Christian Kruse <christian@c-kruse.com>
@c-kruse c-kruse force-pushed the add-multikeylistener branch from 971f20b to eba39cc Compare February 20, 2026 20:19
@c-kruse c-kruse marked this pull request as ready for review February 20, 2026 20:19
@c-kruse c-kruse requested a review from ajssmith February 20, 2026 20:21
if port, err := a.mapping.GetPortForKey(listener.Name); err == nil {
site.UpdateBridgeConfigForListenerWithHostAndPort(siteId, listener, "", port, config)
} else {
port, err := a.mapping.GetPortForKey(listener.Name)
Copy link
Member

@fgiorgetti fgiorgetti Feb 27, 2026

Choose a reason for hiding this comment

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

Minor issue and most unlikely to happen, but if you have a MultiKeyListener named backend and for any reason you define a Listener named multiaddress-backend, the returned port will be the same returned for the backend MKL.
Adding a prefix to listener port keys, like listener- would require an update to all existing k8s service port names.
Not a big deal, but something to keep in mind.

Copy link
Contributor Author

@c-kruse c-kruse Feb 27, 2026

Choose a reason for hiding this comment

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

Hi! Yeah, that is true. I added the prefix for the very likely collision between a listener and mkl with the same name.

I briefly explored the idea of a more ideal solution but never quite nailed it down: maybe something like just using the listener/mkl name for port name then resolving conflicts by appending a short hash of the resource uid or something.

Copy link
Member

@pwright pwright left a comment

Choose a reason for hiding this comment

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

just some comments, small typo

jsonPath: .status.status
name: Status
type: string
- description: Any human reandable message relevant to the multikeylistener
Copy link
Member

Choose a reason for hiding this comment

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

typo

Suggested change
- description: Any human reandable message relevant to the multikeylistener
- description: Any human-readable message relevant to the multikeylistener

type: string
message:
description: |-
message is a human readable message indicating details about the transition.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
message is a human readable message indicating details about the transition.
message is a human-readable message indicating details about the transition.

type: object
type: object
x-kubernetes-validations:
- message: exactly one of the fields in [priority] must be
Copy link
Member

Choose a reason for hiding this comment

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

Dunno if this is where you specify the strategy?

Exactly one routing strategy (e.g., priority) must be set

@fgiorgetti
Copy link
Member

@c-kruse @ajssmith This PR still has the do-not-merge label.
Is there anything other than @pwright's comments preventing it from being merged?

@ajssmith
Copy link
Member

ajssmith commented Mar 3, 2026

I am not aware of anything preventing its merge, it is passing ci now. We can address additional issues in subsequent pr's.

@c-kruse
Copy link
Contributor Author

c-kruse commented Mar 3, 2026

@fgiorgetti no

I just forgot to remove the label when I marked it as ready. Would be good to see e2e passing with the latest router main image

@fgiorgetti
Copy link
Member

@fgiorgetti no

I just forgot to remove the label when I marked it as ready. Would be good to see e2e passing with the latest router main image

CI is already passing now.
I will remove the label and merge it now.

@fgiorgetti fgiorgetti removed the do-not-merge Work In Progress label Mar 3, 2026
@fgiorgetti fgiorgetti merged commit 00a13b5 into skupperproject:main Mar 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants