Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apis/v1alpha3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions apis/v1alpha4/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions apis/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,16 @@ type VirtualMachineCloneSpec struct {
// virtual machine is cloned.
// +optional
NumCoresPerSocket int32 `json:"numCoresPerSocket,omitempty"`
// CPUReservationMhz is the amount of CPU in MHz that is guaranteed available to the virtual machine.
// Defaults to the eponymous property value in the template from which the
// virtual machine is cloned.
// +optional
CPUReservationMhz int64 `json:"cpuReservationMhz,omitempty"`
// CPUShares are a relative priority to other virtual machines used in case of resource contention.
// Defaults to the eponymous property value in the template from which the
// virtual machine is cloned.
// +optional
CPUShares int32 `json:"cpuShares,omitempty"`
Comment on lines +170 to +179
Copy link
Member

@chrischdi chrischdi Sep 9, 2025

Choose a reason for hiding this comment

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

Does it make sense to have something like:

resources:
  reservation:
    cpu: ...
    memory: ...
  shares:
    ...

Or maybe this should be modelled in the k8s wordings which has limits and requests? (might not match the things this PR currently sets).

This is e.g. done by the vm-operator APIs. However, vm-operator does not use shares.

Could someone research what the benefits are of setting shares? And should we also consider CPU Limit to be set?

Copy link
Author

@tommasopozzetti tommasopozzetti Sep 9, 2025

Choose a reason for hiding this comment

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

@chrischdi thanks for the review!

In terms of shares vs reservations, shares are a relative measure of prioritization while reservations are an absolute one. A VM with 2Ghz set for reservation will be guaranteed that even under host contention, it will always have 2Ghz of cpu power available to it. The vSphere admission controller will prevent a VM to power on if the total sum of reservations for VMs on a given host exceeds the total available cpu power of that host (no overprovisioning).
Shares on the other hand are just meaningful relative to shares of other VMs on that host. The host can be over provisioned and if it comes under cpu contention, VMs will be prioritized for cpu time relative to each other depending on their shares. So if a host has 3 VMs, one with 6000 shares, one with 3000 and one with 1000, if the host comes under cpu contention, the first will get 60% of cpu time, the second 30% and the third 10%. Each VM normally gets shares assigned by default proportional to its number of vCPUs but it is very useful to be able to tune that at will.

In terms of using cpu/memory limits, I have never had to implement these but they essentially would artificially cause the same effects as if the underlying host was under resource contention even if it is not, when the VM reaches said limit. More detailed info here. I'd be happy to add to this PR the limit as well as an optional configurable if desired.

Finally, in terms of the syntax, I'm open to suggestions! I personally feel like using the same syntax as standard Kubernetes containers might be misleading since the practical implementation of using reservations, shares and limits on VMs is very different than memory and cpu requests and limits for k8s pods.
I was going for a more flat mapping similar to the other properties that matches with the VM options and would look like

cpuReservationMhz: xxx
cpuShares: xxx
cpuLimitMhz: xxx
memoryReservationMB: xxx
memoryShares: xxx
memoryLimitMB: xxx
reserveAllMemory: false

but, if preferred, we could also go for something nested like

resourceManagement:
  cpu:
    reservationMhz:
    shares:
    limitMhz:
  memory:
    reservationMB:
    shares:
    limitMB:
    reserveAll:

or similar

// MemoryMiB is the size of a virtual machine's memory, in MiB.
// Defaults to the eponymous property value in the template from which the
// virtual machine is cloned.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,20 @@ spec:
Defaults to LinkedClone, but fails gracefully to FullClone if the source
of the clone operation has no snapshots.
type: string
cpuReservationMhz:
description: |-
CPUReservationMhz is the amount of CPU in MHz that is guaranteed available to the virtual machine.
Defaults to the eponymous property value in the template from which the
virtual machine is cloned.
format: int64
type: integer
cpuShares:
description: |-
CPUShares are a relative priority to other virtual machines used in case of resource contention.
Defaults to the eponymous property value in the template from which the
virtual machine is cloned.
format: int32
type: integer
customVMXKeys:
additionalProperties:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,20 @@ spec:
Defaults to LinkedClone, but fails gracefully to FullClone if the source
of the clone operation has no snapshots.
type: string
cpuReservationMhz:
description: |-
CPUReservationMhz is the amount of CPU in MHz that is guaranteed available to the virtual machine.
Defaults to the eponymous property value in the template from which the
virtual machine is cloned.
format: int64
type: integer
cpuShares:
description: |-
CPUShares are a relative priority to other virtual machines used in case of resource contention.
Defaults to the eponymous property value in the template from which the
virtual machine is cloned.
format: int32
type: integer
customVMXKeys:
additionalProperties:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,20 @@ spec:
Defaults to LinkedClone, but fails gracefully to FullClone if the source
of the clone operation has no snapshots.
type: string
cpuReservationMhz:
description: |-
CPUReservationMhz is the amount of CPU in MHz that is guaranteed available to the virtual machine.
Defaults to the eponymous property value in the template from which the
virtual machine is cloned.
format: int64
type: integer
cpuShares:
description: |-
CPUShares are a relative priority to other virtual machines used in case of resource contention.
Defaults to the eponymous property value in the template from which the
virtual machine is cloned.
format: int32
type: integer
customVMXKeys:
additionalProperties:
type: string
Expand Down
14 changes: 14 additions & 0 deletions pkg/services/govmomi/vcenter/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,20 @@ func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []by
Snapshot: snapshotRef,
}

// Set CPU reservation and shares if specified
cpuAllocation := types.ResourceAllocationInfo{}
if vmCtx.VSphereVM.Spec.CPUReservationMhz > 0 {
cpuAllocation.Reservation = ptr.To(vmCtx.VSphereVM.Spec.CPUReservationMhz)
}
if vmCtx.VSphereVM.Spec.CPUShares > 0 {
cpuShares := types.SharesInfo{
Shares: vmCtx.VSphereVM.Spec.CPUShares,
Level: types.SharesLevelCustom,
}
cpuAllocation.Shares = ptr.To(cpuShares)
}
spec.Config.CpuAllocation = ptr.To(cpuAllocation)

// For PCI devices, the memory for the VM needs to be reserved
// We can replace this once we have another way of reserving memory option
// exposed via the API types.
Expand Down