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.

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

fix: mark spot as unavailable on LowPriorityQuota #93

Merged
merged 15 commits into from
Jan 23, 2024
Merged

fix: mark spot as unavailable on LowPriorityQuota #93

merged 15 commits into from
Jan 23, 2024

Conversation

Bryce-Soghigian
Copy link
Collaborator

Fixes #91

Description
If spot capacity hits the spot regional cores count, karpenter should fall back onto on-demand. This PR adds that functionality

How was this change tested?

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note

feat: fail-over to on-demand if spot capacity is unavailable

Bryce-Soghigian
Bryce-Soghigian commented Jan 17, 2024
View reviewed changes
Copy link
Collaborator Author

@Bryce-Soghigian Bryce-Soghigian left a comment

Choose a reason for hiding this comment

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

/test

@Bryce-Soghigian Bryce-Soghigian self-assigned this Jan 17, 2024
@Bryce-Soghigian Bryce-Soghigian changed the title [WIP/POC] feat: fail-over to on-demand if spot capacity is unavailable feat: fail-over to on-demand if spot capacity is unavailable Jan 21, 2024
@Bryce-Soghigian Bryce-Soghigian marked this pull request as ready for review January 22, 2024 23:35
tallaxes
tallaxes requested changes Jan 23, 2024
View reviewed changes
Copy link
Collaborator

@tallaxes tallaxes left a comment

Choose a reason for hiding this comment

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

Like the direction, left some comments.

May also want to rename this to something like "fix: mark spot as unavailable on LowPriorityQuota" (what actually happens after depends on other configuration)

pkg/cache/unavailableofferings.go Outdated Show resolved Hide resolved
pkg/providers/instance/instance.go Outdated Show resolved Hide resolved
pkg/providers/instance/instance.go Outdated Show resolved Hide resolved
pkg/providers/instancetype/instancetypes.go Outdated Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Outdated Show resolved Hide resolved
@tallaxes tallaxes added area/spot Issues or PRs related to spot area/unavailable-offerings Issues or PRs related to unavailable offerings cache area/error-handling Issues or PRs related to handling of errors labels Jan 23, 2024
@Bryce-Soghigian Bryce-Soghigian changed the title feat: fail-over to on-demand if spot capacity is unavailable fix: mark spot as unavailable on LowPriorityQuota Jan 23, 2024
@Bryce-Soghigian @tallaxes
Update pkg/providers/instance/instance.go
1e69d6a 
Co-authored-by: Alex Leites <18728999+tallaxes@users.noreply.github.com>
@Bryce-Soghigian Microsoft Teams for GitHub
Copy link
Collaborator Author

implemented all that was mentioned

tallaxes
tallaxes previously approved these changes Jan 23, 2024
View reviewed changes
Copy link
Collaborator

@tallaxes tallaxes left a comment

Choose a reason for hiding this comment

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

LGTM👍

@Bryce-Soghigian Bryce-Soghigian merged commit dccb36e into main Jan 23, 2024
7 checks passed
@Bryce-Soghigian Bryce-Soghigian deleted the bsoghigian/spot-quota-failover branch January 23, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tallaxes tallaxes tallaxes approved these changes

Assignees

@Bryce-Soghigian Bryce-Soghigian

Labels
area/error-handling Issues or PRs related to handling of errors area/spot Issues or PRs related to spot area/unavailable-offerings Issues or PRs related to unavailable offerings cache
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

LowPriorityCoresQuota error should mark spot as unavailable
2 participants
@Bryce-Soghigian @tallaxes