Skip to content

Add weighted address strategy for multi-key listeners (#2394)#2405

Merged
gabordozsa merged 4 commits intoskupperproject:mainfrom
gabordozsa:gabor_weighted_address_2394
Mar 24, 2026
Merged

Add weighted address strategy for multi-key listeners (#2394)#2405
gabordozsa merged 4 commits intoskupperproject:mainfrom
gabordozsa:gabor_weighted_address_2394

Conversation

@gabordozsa
Copy link
Copy Markdown
Collaborator

Add weighted address strategy for multi-key listeners

Fixes #2394

@gabordozsa gabordozsa added this to the 2.2.0 milestone Mar 13, 2026
@gabordozsa gabordozsa force-pushed the gabor_weighted_address_2394 branch 3 times, most recently from 1f3766e to e0d2c99 Compare March 16, 2026 14:59
@gabordozsa gabordozsa force-pushed the gabor_weighted_address_2394 branch from e0d2c99 to 4a3197f Compare March 18, 2026 12:02
@gabordozsa gabordozsa marked this pull request as ready for review March 18, 2026 12:03
Comment thread config/crd/bases/skupper_multikeylistener_crd.yaml Outdated
Comment thread config/crd/bases/skupper_multikeylistener_crd.yaml Outdated
Comment thread config/crd/bases/skupper_multikeylistener_crd.yaml Outdated
Comment thread config/crd/bases/skupper_multikeylistener_crd.yaml Outdated
Comment thread pkg/apis/skupper/v2alpha1/multikeylistener_types.go Outdated
@gabordozsa gabordozsa requested a review from pwright March 18, 2026 18:30
Copy link
Copy Markdown
Member

@fgiorgetti fgiorgetti left a comment

Choose a reason for hiding this comment

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

It is working great on both kubernetes with system sites. Nice job @gabordozsa.

One thing that we need to consider, is whether the strategy field should be immutable or not. If we allow it to change, we should cleanup the status.strategy field and update it with the new strategy.

Comment thread internal/nonkube/common/site_state_validator.go
Comment thread internal/nonkube/common/site_state_validator.go Outdated
Comment thread pkg/apis/skupper/v2alpha1/multikeylistener_types.go
@gabordozsa
Copy link
Copy Markdown
Collaborator Author

One thing that we need to consider, is whether the strategy field should be immutable or not. If we allow it to change, we should cleanup the status.strategy field and update it with the new strategy.

@fgiorgetti I think the strategy field should be immutable for now. The router does not allow updating the strategy so, the control plane should delete and re-create the multi-key listener if strategy changes. That would terminate existing TCP connections. If strategy update is required, we may add it in a follow up change including the router support for it.

@fgiorgetti
Copy link
Copy Markdown
Member

fgiorgetti commented Mar 20, 2026

One thing that we need to consider, is whether the strategy field should be immutable or not. If we allow it to change, we should cleanup the status.strategy field and update it with the new strategy.

@fgiorgetti I think the strategy field should be immutable for now. The router does not allow updating the strategy so, the control plane should delete and re-create the multi-key listener if strategy changes. That would terminate existing TCP connections. If strategy update is required, we may add it in a follow up change including the router support for it.

Agreed. The CRD validation could be enhanced to guarantee that the field is immutable.

@pwright
Copy link
Copy Markdown
Member

pwright commented Mar 20, 2026

Help me understand immutable in this context.
If i create a CR named 'A' with strategy 'priority', I can not change the strategy without first deleting 'A' resource. Can i then recreate 'A' with strategy weighted?

@fgiorgetti
Copy link
Copy Markdown
Member

Help me understand immutable in this context. If i create a CR named 'A' with strategy 'priority', I can not change the strategy without first deleting 'A' resource. Can i then recreate 'A' with strategy weighted?

Correct.

Copy link
Copy Markdown
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.

lgtm

@gabordozsa
Copy link
Copy Markdown
Collaborator Author

Agreed. The CRD validation could be enhanced to guarantee that the field is immutable.

@fgiorgetti I added the extra CRD validation following your guidance. It covers kube platform. System platform validation will be covered in a follow up change after dynamic reload work is completed in issue #2411.

@gabordozsa gabordozsa requested a review from fgiorgetti March 23, 2026 19:26
Copy link
Copy Markdown
Member

@fgiorgetti fgiorgetti left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@gabordozsa gabordozsa merged commit b93ec9c into skupperproject:main Mar 24, 2026
1 check 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.

Add wrr strategy to MultiKeyListener

3 participants