feat(virtualmachine): Add eviction strategy, grace period, and OS type#159
Open
jniedergang wants to merge 5 commits intoharvester:masterfrom
Open
feat(virtualmachine): Add eviction strategy, grace period, and OS type#159jniedergang wants to merge 5 commits intoharvester:masterfrom
jniedergang wants to merge 5 commits intoharvester:masterfrom
Conversation
jniedergang
force-pushed
the
upstream-vm-runtime
branch
2 times, most recently
from
81fe719 to
34d1a75
Compare
|
This pull request is now in conflict. Could you fix it @jniedergang? 🙏 |
jniedergang
force-pushed
the
upstream-vm-runtime
branch
from
34d1a75 to
7496769
Compare
|
This pull request is now in conflict. Could you fix it @jniedergang? 🙏 |
jniedergang
force-pushed
the
upstream-vm-runtime
branch
from
7496769 to
8a371e2
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the harvester_virtualmachine Terraform resource to expose additional KubeVirt runtime options (eviction strategy, termination grace period, and OS type annotation) instead of relying on hardcoded defaults, improving configurability and import/read behavior.
Changes:
- Added
eviction_strategy,termination_grace_period_seconds, andos_typefields to the VM schema and constants. - Implemented constructor support to write these fields into the KubeVirt VM spec/annotations.
- Extended importer/state getter plus unit + acceptance tests to cover the new runtime options.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/importer/resource_virtualmachine_importer.go | Import/read support for eviction strategy, termination grace period, and OS type. |
| pkg/importer/resource_virtualmachine_importer_test.go | Unit test coverage for new importer accessors and defaults. |
| pkg/constants/constants_virtualmachine.go | Adds new Terraform field keys and the Harvester OS annotation key constant. |
| internal/provider/virtualmachine/schema_virtualmachine.go | Exposes new runtime options in the Terraform resource schema. |
| internal/provider/virtualmachine/resource_virtualmachine_constructor.go | Writes the new runtime options into the VM spec/annotations during create/update. |
| internal/tests/resource_virtualmachine_test.go | Adds acceptance test for the new runtime options and updates the test config builder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/provider/virtualmachine/resource_virtualmachine_constructor.go
Outdated
Show resolved
Hide resolved
jniedergang
force-pushed
the
upstream-vm-runtime
branch
from
691a070 to
aad83ea
Compare
jniedergang
force-pushed
the
upstream-vm-runtime
branch
3 times, most recently
from
ac268a7 to
732cd80
Compare
|
This pull request is now in conflict. Could you fix it @jniedergang? 🙏 |
added 4 commits
March 6, 2026 17:28
Make eviction strategy configurable (was hardcoded to LiveMigrateIfPossible). Add termination_grace_period_seconds for graceful shutdown control. Add os_type annotation for KVM guest optimizations. Signed-off-by: Terraform Provider Developer <terraform@harvester.local>
…nds and update docs Add ValidateFunc: validation.IntAtLeast(0) to reject negative values for termination_grace_period_seconds. Add missing field documentation for eviction_strategy, termination_grace_period_seconds, and os_type. Signed-off-by: Terraform Provider Developer <terraform@harvester.local>
- Extract hardcoded defaults into shared constants (DefaultEvictionStrategy, DefaultTerminationGracePeriodSeconds) - Use constants in schema, importer, and tests to avoid drift - Remove Required:true from os_type processor to not override server-provided values when field is unset - Fix test builder to use *int for terminationGracePeriodSeconds, allowing testing of value 0 Signed-off-by: Terraform Provider Developer <terraform@harvester.local>
Fix staticcheck QF1012 lint findings in test builder. Signed-off-by: Terraform Provider Developer <terraform@harvester.local>
jniedergang
force-pushed
the
upstream-vm-runtime
branch
from
381cef6 to
759ba5d
Compare
Prevents InternalValidate error on schedule_backup data source. Signed-off-by: Terraform Provider Developer <terraform@harvester.local>
Author
|
Ran functional tests on Harvester v1.7.1:
Updated test plan in PR description accordingly. |
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
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.
Summary
termination_grace_period_secondsfor graceful shutdown controlos_typeannotation for KVM guest optimizationsrelated to harvester/harvester#10035
Test plan
go test ./pkg/importer/ -run TestVMRuntimeImporteviction_strategy=None,termination_grace_period_seconds=60— verified viakubectl get vmos_typecomputed field reads correctly (empty when no annotation set)terraform planshows 0 changesterraform destroy— VM cleaned up