Skip to content
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.

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

Add support for priorityClassName (fixes #1034) #1432

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

olim7t
Copy link
Contributor

@olim7t olim7t commented Oct 17, 2024

What this PR does:

Which issue(s) this PR fixes:
Fixes #1034

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

@olim7t olim7t requested a review from a team as a code owner October 17, 2024 01:01
Copy link

sonarcloud bot commented Oct 17, 2024

Copy link
Contributor

@burmanm burmanm left a comment

Choose a reason for hiding this comment

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

suggestion: This was not part of the original ticket, so these are not mandatory changes, but if you feel like these would worth doing doing later (but not part of this PR), we could create a ticket for it:

That is, we don't actually check if the PriorityClass exists. Right now that could cause the Pod to have a priorityClassName that points to nothing and this probably prevents the pod from being scheduled correctly. I think this check could be implemented in cass-operator also, I have no preference to that right now.

Second is actually creating PriorityClasses in k8ssandra-operator, but I would have to think about this feature more. Most users should probably set something like preemptionPolicy to Never to prevent new Cassandra pods from killing existing pods. Right now, that would be up to the user to understand what they're doing (like the entire feature, which I think we could consider "experienced users only").

If we use this feature in downstream products, then tracking PodStatus' nominatedNodeName might make sense also. And our PodDisruptionBudget policy is probably not very compatible with this feature either. The scheduling is quite complex on some of these and the documentation is a bit hand waving in some cases "we might or might not do this and maybe something else".

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.

priorityClassName support
2 participants