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

design-proposal: instancetype.kubevirt.io - Support custom resource sizing #318

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lyarwood
Copy link
Member

@lyarwood lyarwood commented Aug 20, 2024

What this PR does / why we need it:

Instance types and preferences provide a way to define common VirtualMachine workload resource sizing and preferences.

The common-instancetypes project within KubeVirt currently provides a set of standardized instance types and preferences that can be deployed by virt-operator alongside all of the other core KubeVirt components.

While the resources provided by the common-instancetypes project are a great starting point they are generalized and could never possibly cover all workload resourcing and preference use cases.

As such this design proposal seeks to make it as easy as possible for both cluster admins and users to create their own customized versions of these commonly deployed resources specific to their workloads. Additionally the instance type API itself will be made more flexible by making vCPU and memory resource sizing optional within an instance type, allowing users to provide their own sizing within their VirtualMachine while still retaining the remaining benefits of using an instance type.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Release note:

NONE

@kubevirt-bot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 20, 2024
@kubevirt-bot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign cwilkers for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Aug 20, 2024
@lyarwood lyarwood changed the title design-proposal: instancetype - Support custom resource sizing design-proposal: instancetype.kubevirt.io - Support custom resource sizing Aug 20, 2024
@lyarwood lyarwood force-pushed the instancetype-custom-resource-sizing branch from d972e47 to 3e0463f Compare September 3, 2024 13:24
Copy link
Contributor

@jean-edouard jean-edouard left a comment

Choose a reason for hiding this comment

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

Lots of pedantic formatting comments, feel free to ignore.
My only real question is around the need for a new virtctl command.
Thanks!


## Allow for the easy creation of customization common-instancetype resources

### Introduce `virtctl customize`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? Can't users just kubectl edit to make changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

The use case with virtctl customize is that we want to start with an object in the cluster, make some small changes, automate kind conversion/metadata removal and then output the newly crafted definition.

Does kubectl edit actually support taking an existing object and only outputting the result locally? I thought it always attempted to mutate the object in the cluster?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah so the idea is for users to be able to make a copy of a system-wide instancetype into their namespace so they can modify it?
Then I would suggest something like virtctl instancetype clone with a mandatory namespace argument. Then users can kubectl edit the resulting object. Would that make sense?
I just don't like the fact that virtctl customize isn't self explanatory.
Sorry for the extremely long delay btw, I somehow didn't get notified of your answer...

Instance types and preferences provide a way to define common
VirtualMachine workload resource sizing and preferences.

The common-instancetypes project within KubeVirt currently provides a
set of standardized instance types and preferences that can be deployed
by `virt-operator` alongside all of the other core KubeVirt components.

While the resources provided by the common-instancetypes project are a
great starting point they are generalized and could never possibly cover
all workload resourcing and preference use cases.

As such this design proposal seeks to make it as easy as possible for
both cluster admins and users to create their own customized versions of
these commonly deployed resources specific to their workloads.
Additionally the instance type API itself will be made more flexible by
making vCPU and memory resource sizing optional within an instance type,
allowing users to provide their own sizing within their VirtualMachine
while still retaining the remaining benefits of using an instance type.

Signed-off-by: Lee Yarwood <[email protected]>

Apply suggestions from code review

Co-authored-by: Jed Lejosne <[email protected]>
Signed-off-by: Lee Yarwood <[email protected]>
@lyarwood lyarwood force-pushed the instancetype-custom-resource-sizing branch from 1abbece to 44ea064 Compare September 4, 2024 13:18
@lyarwood lyarwood marked this pull request as ready for review September 4, 2024 13:19
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 4, 2024
@lyarwood
Copy link
Member Author

lyarwood commented Sep 4, 2024

Lots of pedantic formatting comments, feel free to ignore. My only real question is around the need for a new virtctl command. Thanks!

Thanks @jean-edouard , really appreciate the review!

@xpivarc
Copy link
Member

xpivarc commented Nov 18, 2024

/sig compute

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. sig/compute size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants