Skip to content

feat!: Add new ListenerClass.stickyNodePorts field #1105

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat!: Add new ListenerClass.stickyNodePorts field #1105

wants to merge 3 commits into from
+36 −1

Conversation

sbernauer
Copy link
Member

@sbernauer sbernauer commented Oct 7, 2025 •
edited
Loading

Description

Part of stackabletech/issues#770
Implementation is in stackabletech/listener-operator#340

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Author

  • Changes are OpenShift compatible
  • CRD changes approved
  • CRD documentation for all fields, following the style guide.
  • Integration tests passed (for non trivial changes)
  • Changes need to be "offline" compatible

Reviewer

  • Code contains useful comments
  • Code contains useful logging statements
  • (Integration-)Test cases added
  • Documentation added or updated. Follows the style guide.
  • Changelog updated
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

  • Feature Tracker has been updated
  • Proper release label has been added

@sbernauer sbernauer self-assigned this Oct 7, 2025
@sbernauer sbernauer moved this to Development: Waiting for Review in Stackable Engineering Oct 7, 2025
This was referenced Oct 7, 2025
Open
Open
@NickLarsenNZ
Copy link
Member

NickLarsenNZ commented Oct 8, 2025 •
edited
Loading

What (k8s resources changes) does it result in? Some changes to generated Services or just the generated Listener?

@sbernauer
Copy link
Member Author

What (k8s resources changes) does it result in?

Actually the csi::v1::Volume, which in turn influences the created PVC (I guess). Feel free to have a look at rust/operator-binary/src/csi_server/controller.rs in stackabletech/listener-operator#340

@sbernauer
Copy link
Member Author

Please vote on this comment

Techassi
Techassi reviewed Oct 9, 2025
View reviewed changes
Techassi
Techassi reviewed Oct 9, 2025
View reviewed changes
crates/stackable-operator/src/crd/listener/class/mod.rs
Comment on lines +76 to +78
/// However, this only works on setups with long-living nodes. If your nodes are rotated on
/// a regular basis, the Pods previously running on a removed node will be stuck in Pending
/// until you delete the PVC with the stickiness.
Copy link
Member

Choose a reason for hiding this comment

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

question: Could we somehow detect this and automatically remove the stickiness? Or is that something we don't want?

Copy link
Member Author

@sbernauer sbernauer Oct 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ideally we have auto-detection in listener-op, which detects what ListenerClasses it should create, based on detecting GKE/AKS/IONOS etc. You might recall this from the CRD maintainer discussions.
But that's a follow-up

@NickLarsenNZ
Copy link
Member

NickLarsenNZ commented Oct 10, 2025 •
edited
Loading

What (k8s resources changes) does it result in?

Actually the csi::v1::Volume, which in turn influences the created PVC (I guess). Feel free to have a look at rust/operator-binary/src/csi_server/controller.rs in stackabletech/listener-operator#340

I'm just now wondering if Stickyness is the correct naming.

I haven't had time to dig deep, and this is in voting now, so I feel a bit rushed.

Is this actual "stickyness" which is typically used to mean directing traffic for the same session to the same instance. This seems more like it's about "Permanency" "Node Pinning" or "Strict Node Affinity" (I see sticky has been used in the PR you linked, so this isn't a new thing, but I do want to discuss whether the naming is appropriate).

I feel like the description lacks context and instead links a bunch of things. Are you able to update the description with the missing context?

I left a related comment on the PR about the field name and avoiding "sticky"/"stickiness": https://github.com/stackabletech/listener-operator/pull/340/files#r2418860493

@sbernauer
Copy link
Member Author

I'm just now wondering if Stickyness is the correct naming.

Good question, I'm no native speaker.
This only affect NodePorts!

The Pod "sticks" to one single node.
=> a.) stickyNodePorts

The Pod is "pinned" to a particular node.
=> b.) pinningNodePorts
=> c.) nodePinningNodePorts
=> d.) ???

Both work for me, I let the native speakers decide 😅 (a.) sounds a bit more natural to me)

@sbernauer
Copy link
Member Author

I feel like the description lacks context and instead links a bunch of things. Are you able to update the description with the missing context?

The parent issue stackabletech/issues#770 should hopefully contain all the motivation and details on making the stickiness/pinning configurable. I would prefer to keep one well-maintained issue instead of cluttering things on issues, decisions and PRs. Pls feel free to ask if there are any missing things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Techassi Techassi Techassi left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@sbernauer sbernauer

Labels
None yet
Projects
Stackable Engineering
Status: Development: Waiting for Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sbernauer @NickLarsenNZ @Techassi