refactor: split offerings handling into a sub-module#1166
Merged
Conversation
comtalyst
added
the
size/M
comtalyst
force-pushed
the
comtalyst/vm-instance-renames
branch
from
f8d89d7 to
03cec38
Compare
comtalyst
force-pushed
the
comtalyst/offerings-split
branch
from
b2364ca to
c0bc25f
Compare
comtalyst
force-pushed
the
comtalyst/vm-instance-renames
branch
from
03cec38 to
6080df2
Compare
comtalyst
force-pushed
the
comtalyst/offerings-split
branch
from
c0bc25f to
4169676
Compare
matthchr
reviewed
Oct 4, 2025
matthchr
previously approved these changes
Oct 8, 2025
Member
matthchr
left a comment
matthchr
left a comment
There was a problem hiding this comment.
Approved.
My main issue here remaining is the fact that the error shape from machine API cannot be used like the CRP error shape, we have to go get the error off the entity itself.
I think that should be fixed (or else at least I want to understand better why it has to be that way), but that is more a machine API behavior problem and less an issue with this Karpenter code.
LGTM.
matthchr
previously approved these changes
Oct 20, 2025
matthchr
approved these changes
Oct 21, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #
Description
The processes of instance provisioning and SKU/Priority/zone selection/handling should be clearly distinguishable from one another. However, current code does not seem to reflect that well enough. This concern is also previously noted in #791.
Naturally, it has been an obstacle to efficient improvements on each, especially for upcoming Machine API integration.
This PR attempts to separate the latter into its own sub-module, with the logic intact.
How was this change tested?
Does this change impact docs?
Release Note