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

neonvm-controller: Remove support for DIMM slots #1070

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

Conversation

sharnoff
Copy link
Member

@sharnoff sharnoff commented Sep 9, 2024

This is part 1 of 2 for removing DIMM slots support and completing our transition to using virtio-mem instead (ref #1060).

With this change, neonvm-controller will always assume that VMs are using virtio-mem. The only checks exist at the top of the reconcile functions, making sure that this is indeed the case.

The fields still exist in the CRD (although they're marked as deprecated), so that rollback of this change is still sound.

neonvm-runner also retains support for both memory providers, so that custom neonvm-runner images continue to work for at least one version across the change.

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 1 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca
🛡️ The following IaC misconfigurations have been detected
NAME FILE
info Ensure Administrative Boundaries Between Resources ...ller/deployment.yaml View in code

neonvm/controllers/vm_controller.go Outdated Show resolved Hide resolved
neonvm/controllers/vm_controller.go Outdated Show resolved Hide resolved
tests/e2e/vm-migration/00-assert.yaml Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Oct 12, 2024

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/neondatabase/autoscaling/neonvm-controller/cmd 0.00% (ø)
github.com/neondatabase/autoscaling/neonvm/apis/neonvm/v1 0.56% (ø)
github.com/neondatabase/autoscaling/pkg/neonvm/controllers 13.18% (+1.67%) 👍
github.com/neondatabase/autoscaling/pkg/neonvm/controllers/functests 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/neondatabase/autoscaling/neonvm-controller/cmd/main.go 0.00% (ø) 125 (-5) 0 125 (-5)
github.com/neondatabase/autoscaling/neonvm/apis/neonvm/v1/virtualmachine_types.go 1.96% (+0.04%) 51 (-1) 1 50 (-1) 👍
github.com/neondatabase/autoscaling/neonvm/apis/neonvm/v1/virtualmachine_webhook.go 0.00% (ø) 52 (+1) 0 52 (+1)
github.com/neondatabase/autoscaling/pkg/neonvm/controllers/config.go 0.00% (ø) 0 0 0
github.com/neondatabase/autoscaling/pkg/neonvm/controllers/vm_controller.go 25.88% (+0.66%) 622 (-40) 161 (-6) 461 (-34) 👍
github.com/neondatabase/autoscaling/pkg/neonvm/controllers/vm_qmp_queries.go 0.00% (ø) 191 (-189) 0 191 (-189)
github.com/neondatabase/autoscaling/pkg/neonvm/controllers/vmmigration_controller.go 0.00% (ø) 342 (-4) 0 342 (-4)

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/neondatabase/autoscaling/pkg/neonvm/controllers/functests/vm_controller_test.go
  • github.com/neondatabase/autoscaling/pkg/neonvm/controllers/vm_controller_unit_test.go
  • github.com/neondatabase/autoscaling/pkg/neonvm/controllers/vm_qmp_test.go

HTML Report

Click to open

@sharnoff sharnoff requested a review from Omrigan October 12, 2024 21:45
@sharnoff sharnoff assigned Omrigan and unassigned sharnoff Oct 12, 2024
@sharnoff
Copy link
Member Author

sharnoff commented Nov 4, 2024

todo: Need to update this PR after recent changes.

Also, explicitly set them to use spec.memoryProvider = DIMMSlots, to
contrast with the existing VirtioMem versions.
This is part 1 of 2 for removing DIMM slots support and completing our
transition to using virtio-mem instead (ref #1060).

With this change, neonvm-controller will always assume that VMs are
using virtio-mem. The only checks exist at the top of the reconcile
functions, making sure that this is indeed the case.

The fields still exist in the CRD (although the're marked as
deprecated), so that rollback of this change is still sound.

neonvm-runner also retains support for both memory providers, so that
custom neonvm-runner images continue to work for at least one version
across the change.
@sharnoff sharnoff force-pushed the sharnoff/neonvm-deprecate-dimmslots branch from 7decf05 to 9035287 Compare November 18, 2024 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants