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

neonvm: Make spec min/max/use resources required #971

Merged
merged 4 commits into from
Jun 19, 2024

Conversation

sharnoff
Copy link
Member

This already was de-facto the case, which made much of the logic around handling "but what if .max is nil?" somewhat tricky to reason about.

Extracted from #970 and fleshed out here.

All our staging & production clusters have VMs meeting this criteria, with the notable exception of a single, already-broken VM in dev-us-east-2-beta (it's a long story).

More here: https://neondb.slack.com/archives/C03TN5G758R/p1718742308251569


Checked each cluster with:

kubectl get neonvm -o json \
  |  jq -r '.items[] | select(.spec.guest | [.cpus.max,.cpus.min,.memorySlots.max,.memorySlots.min] | map(select(. == null)) | (length != 0)) | .metadata.name'

This already was de-facto the case, which made much of the logic around
handling "but what if .max is nil?" somewhat hard to reason about.

Extracted from #970 and fleshed out here.

All our staging & production clusters have VMs meeting this criteria,
with the notable exception of a single, already-broken VM in
dev-us-east-2-beta.

More here: https://neondb.slack.com/archives/C03TN5G758R/p1718742308251569

---

Checked each cluster with:

kubectl get neonvm -o json \
  |  jq -r '.items[] | select(.spec.guest | [.cpus.max,.cpus.min,.memorySlots.max,.memorySlots.min] | map(select(. == null)) | (length != 0)) | .metadata.name'
@sharnoff sharnoff requested a review from Omrigan June 18, 2024 20:58
@sharnoff sharnoff mentioned this pull request Jun 18, 2024
5 tasks
Copy link
Contributor

@Omrigan Omrigan left a comment

Choose a reason for hiding this comment

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

Nice! It's always pleasant to see when the type checker does the work instead of us.

@sharnoff sharnoff changed the title neonvm: Make spec min/max resources required neonvm: Make spec min/max/use resources required Jun 19, 2024
@sharnoff sharnoff enabled auto-merge (squash) June 19, 2024 19:26
@sharnoff sharnoff merged commit 936a238 into main Jun 19, 2024
15 checks passed
@sharnoff sharnoff deleted the sharnoff/neonvm-require-min-max branch June 19, 2024 19:37
sharnoff added a commit that referenced this pull request Jun 21, 2024
Seems like #935 had issues after merging main into it due to #971, but
test failure didn't prevent auto-merge.
sharnoff added a commit that referenced this pull request Jun 21, 2024
Seems like #935 had issues after merging main into it due to #971, but
test failure didn't prevent auto-merge.
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