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

temporary PR to help investigate ZONE_NORMAL #959

Closed
wants to merge 4 commits into from
Closed
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: 1 addition & 1 deletion neonvm/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ To adjust the kernel config:

```
cd hack/kernel
docker build --build-arg KERNEL_VERSION=6.1.92 --platform linux/x86_64 --target build-deps -t kernel-build-deps -f Dockerfile.kernel-builder .
docker build --build-arg KERNEL_VERSION=6.1.63 --platform linux/x86_64 --target build-deps -t kernel-build-deps -f Dockerfile.kernel-builder .
docker run --rm -v $PWD:/host --name kernel-build -it kernel-build-deps bash
# inside that bash shell, do the menuconfig, then copy-out the config to /host
```
Expand Down
47 changes: 47 additions & 0 deletions neonvm/apis/neonvm/v1/virtualmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/json"
"errors"
"fmt"
"slices"

"github.com/samber/lo"

Expand Down Expand Up @@ -172,6 +173,8 @@ type Guest struct {
// +optional
MemorySlots MemorySlots `json:"memorySlots"`
// +optional
MemoryProvider *MemoryProvider `json:"memoryProvider,omitempty"`
// +optional
RootDisk RootDisk `json:"rootDisk"`
// Docker image Entrypoint array replacement.
// +optional
Expand All @@ -194,6 +197,24 @@ type Guest struct {
Settings *GuestSettings `json:"settings,omitempty"`
}

const virtioMemBlockSizeBytes = 8 * 1024 * 1024 // 8 MiB

// ValidateForMemoryProvider returns an error iff the guest memory settings are invalid for the
// MemoryProvider.
//
// This is used in two places. First, to validate VirtualMachine object creation. Second, to handle
// the defaulting behavior for VirtualMachines that would be switching from DIMMSlots to VirtioMem
// on restart. We place more restrictions on VirtioMem because we use 8MiB block sizes, so changing
// to a new default can only happen if the memory slot size is a multiple of 8MiB.
func (g Guest) ValidateForMemoryProvider(p MemoryProvider) error {
if p == MemoryProviderVirtioMem {
if g.MemorySlotSize.Value()%virtioMemBlockSizeBytes != 0 {
return fmt.Errorf("memorySlotSize invalid for memoryProvider VirtioMem: must be a multiple of 8Mi")
}
}
return nil
}

type GuestSettings struct {
// Individual lines to add to a sysctl.conf file. See sysctl.conf(5) for more
// +optional
Expand Down Expand Up @@ -361,6 +382,29 @@ type MemorySlots struct {
Use *int32 `json:"use,omitempty"`
}

// +kubebuilder:validation:Enum=DIMMSlots;VirtioMem
type MemoryProvider string

const (
MemoryProviderDIMMSlots MemoryProvider = "DIMMSlots"
MemoryProviderVirtioMem MemoryProvider = "VirtioMem"
)

// FlagFunc is a parsing function to be used with flag.Func
func (p *MemoryProvider) FlagFunc(value string) error {
possibleValues := []string{
string(MemoryProviderDIMMSlots),
string(MemoryProviderVirtioMem),
}

if !slices.Contains(possibleValues, value) {
return fmt.Errorf("Unknown MemoryProvider %q, must be one of %v", value, possibleValues)
}

*p = MemoryProvider(value)
return nil
}

type RootDisk struct {
Image string `json:"image"`
// +optional
Expand Down Expand Up @@ -493,6 +537,8 @@ type VirtualMachineStatus struct {
// +optional
MemorySize *resource.Quantity `json:"memorySize,omitempty"`
// +optional
MemoryProvider *MemoryProvider `json:"memoryProvider,omitempty"`
// +optional
SSHSecretName string `json:"sshSecretName,omitempty"`
}

Expand Down Expand Up @@ -558,6 +604,7 @@ func (vm *VirtualMachine) Cleanup() {
vm.Status.Node = ""
vm.Status.CPUs = nil
vm.Status.MemorySize = nil
vm.Status.MemoryProvider = nil
}

func (vm *VirtualMachine) HasRestarted() bool {
Expand Down
21 changes: 21 additions & 0 deletions neonvm/apis/neonvm/v1/virtualmachine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ func (r *VirtualMachine) ValidateCreate() error {
}
}

// validate .spec.guest.memorySlotSize w.r.t. .spec.guest.memoryProvider
if r.Spec.Guest.MemoryProvider != nil {
if err := r.Spec.Guest.ValidateForMemoryProvider(*r.Spec.Guest.MemoryProvider); err != nil {
return fmt.Errorf(".spec.guest: %w", err)
}
}

// validate .spec.guest.memorySlots.use and .spec.guest.memorySlots.max
if r.Spec.Guest.MemorySlots.Use != nil {
if r.Spec.Guest.MemorySlots.Max == nil {
Expand Down Expand Up @@ -160,6 +167,7 @@ func (r *VirtualMachine) ValidateUpdate(old runtime.Object) error {
{".spec.guest.cpus.max", func(v *VirtualMachine) any { return v.Spec.Guest.CPUs.Max }},
{".spec.guest.memorySlots.min", func(v *VirtualMachine) any { return v.Spec.Guest.MemorySlots.Min }},
{".spec.guest.memorySlots.max", func(v *VirtualMachine) any { return v.Spec.Guest.MemorySlots.Max }},
// nb: we don't check memoryProvider here, we have some specific logic for that.
{".spec.guest.ports", func(v *VirtualMachine) any { return v.Spec.Guest.Ports }},
{".spec.guest.rootDisk", func(v *VirtualMachine) any { return v.Spec.Guest.RootDisk }},
{".spec.guest.command", func(v *VirtualMachine) any { return v.Spec.Guest.Command }},
Expand Down Expand Up @@ -187,6 +195,19 @@ func (r *VirtualMachine) ValidateUpdate(old runtime.Object) error {
}
}

// Only allow changing .spec.guest.memoryProvider if it was previously nil, and the new version
// is equal to .status.memoryProvider.
if !reflect.DeepEqual(before.Spec.Guest.MemoryProvider, r.Spec.Guest.MemoryProvider) {
if before.Spec.Guest.MemoryProvider != nil {
return errors.New(".spec.guest.memoryProvider cannot be changed once it is non-nil")
}
if !reflect.DeepEqual(r.Spec.Guest.MemoryProvider, r.Status.MemoryProvider) {
return errors.New(".spec.guest.memoryProvider cannot be set to any value other than .status.memoryProvider")
}
}

// {".spec.guest.memoryProvider", func(v *VirtualMachine) any { return v.Spec.Guest.MemoryProvider }},

// validate swap changes by comparing the SwapInfo for each.
//
// If there's an error with the old object, but NOT an error with the new one, we'll allow the
Expand Down
10 changes: 10 additions & 0 deletions neonvm/apis/neonvm/v1/zz_generated.deepcopy.go

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

1 change: 1 addition & 0 deletions neonvm/config/controller/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ spec:
# * cache.direct=on - use O_DIRECT (don't abuse host's page cache!)
# * cache.no-flush=on - ignores disk flush operations (not needed; our disks are ephemeral)
- "--qemu-disk-cache-settings=cache.writeback=on,cache.direct=on,cache.no-flush=on"
- "--default-memory-provider=DIMMSlots"
- "--failure-pending-period=1m"
- "--failing-refresh-interval=15s"
env:
Expand Down
10 changes: 10 additions & 0 deletions neonvm/config/crd/bases/vm.neon.tech_virtualmachines.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2412,6 +2412,11 @@ spec:
type: array
kernelImage:
type: string
memoryProvider:
enum:
- DIMMSlots
- VirtioMem
type: string
memorySlotSize:
anyOf:
- type: integer
Expand Down Expand Up @@ -2764,6 +2769,11 @@ spec:
type: string
extraNetMask:
type: string
memoryProvider:
enum:
- DIMMSlots
- VirtioMem
type: string
memorySize:
anyOf:
- type: integer
Expand Down
10 changes: 9 additions & 1 deletion neonvm/controllers/config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package controllers

import "time"
import (
"time"

vmv1 "github.com/neondatabase/autoscaling/neonvm/apis/neonvm/v1"
)

// ReconcilerConfig stores shared configuration for VirtualMachineReconciler and
// VirtualMachineMigrationReconciler.
Expand All @@ -25,6 +29,10 @@ type ReconcilerConfig struct {
// used in setting up the VM disks via QEMU's `-drive` flag.
QEMUDiskCacheSettings string

// DefaultMemoryProvider is the memory provider (dimm slots or virtio-mem) that will be used for
// new VMs (or, when old ones restart) if nothing is explicitly set.
DefaultMemoryProvider vmv1.MemoryProvider

// FailurePendingPeriod is the period for the propagation of
// reconciliation failures to the observability instruments
FailurePendingPeriod time.Duration
Expand Down
Loading
Loading