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 nodepool to the CRD #553

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Add nodepool to the CRD #553

merged 1 commit into from
Nov 10, 2023

Conversation

tomach
Copy link
Contributor

@tomach tomach commented Oct 25, 2023

Summary of changes

Add optional nodepool to the node's CRD and set node affinity and tolerations accordingly when creating a cluster or changing its compute.
https://github.com/crate/cloud/issues/1502

Checklist

  • Relevant changes are reflected in CHANGES.rst
  • Added or changed code is covered by tests
  • Documentation has been updated if necessary
  • Changed code does not contain any breaking changes (or this is a major version change)

Copy link
Contributor

@SStorm SStorm left a comment

Choose a reason for hiding this comment

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

If I understand correctly, if nodepool is not specified, we will always default to dedicated, right?

if not (cpu_request or memory_request):
return False
return cpu_request != cpu_limit or memory_request != memory_limit
return (
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the question is whether we need to check the requests/limits at all anymore? It might be a bit counter-intuitive, because if your requests/limits do not differ, but you still specify shared for the nodepool, it will ignore that and put it to the dedicated one?...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm you're right. I can't think of a case where both checks are needed either.

If I understand correctly, if nodepool is not specified, we will always default to dedicated, right?

yes. if no nodepool is specified, it defaults to the tolerations and affinity of the dedicated one.

@tomach tomach force-pushed the ta/crd-add-nodepool branch 2 times, most recently from 4d06a62 to d53aeb1 Compare November 8, 2023 10:16
@tomach tomach merged commit 3a97c5d into master Nov 10, 2023
5 checks passed
@tomach tomach deleted the ta/crd-add-nodepool branch November 10, 2023 09:48
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.

2 participants