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.

Sign up for GitHub

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

Jump to bottom

Keep the custom CPU model #686

Merged
merged 3 commits into from
Dec 25, 2023
Merged

Keep the custom CPU model #686

merged 3 commits into from
Dec 25, 2023

Conversation

liranr23
Copy link
Member

@liranr23 liranr23 commented Dec 21, 2023 •
edited by ahadas
Loading

When importing from oVirt, a VM may be set with a custom CPU model.
This PR will preserve it.

@liranr23 liranr23 added enhancement New feature or request RHV labels Dec 21, 2023
ahadas
ahadas reviewed Dec 24, 2023
View reviewed changes
Copy link
Member

@ahadas ahadas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

it looks good overall, but how about separating out the changes for the custom CPU (from those for the cluster CPU) to another PR?

operator/config/crd/bases/forklift.konveyor.io_plans.yaml Outdated Show resolved Hide resolved
pkg/apis/forklift/v1beta1/plan.go Outdated Show resolved Hide resolved
validation/policies/io/konveyor/forklift/ovirt/cpu_custom_model.rego Outdated Show resolved Hide resolved
validation/policies/io/konveyor/forklift/ovirt/cpu_custom_model.rego Outdated Show resolved Hide resolved
pkg/controller/plan/adapter/ovirt/builder.go Show resolved Hide resolved
pkg/controller/plan/adapter/ovirt/builder.go Show resolved Hide resolved
@liranr23
Add CPU model to the inventory
0a4fd64 
Get the custom CPU model of the VM.

Signed-off-by: Liran Rotenberg <[email protected]>
@codecov Codecov
Copy link

codecov bot commented Dec 24, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3e7c0a2) 0.00% compared to head (571fd18) 0.00%.
Report is 1 commits behind head on main.

❗ Current head 571fd18 differs from pull request most recent head 2b8ebe2. Consider uploading reports for the commit 2b8ebe2 to get more accurate results

Additional details and impacted files 
@@     Coverage Diff     @@
##   main   #686   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@liranr23
Set the CPU in destination VM
0751026 
This patch adds the CPU configuration from oVirt to OCP. This includes
the additional flags pointed out in the source environment.

Signed-off-by: Liran Rotenberg <[email protected]>
@liranr23
Add a custom CPU warning
2b8ebe2 
The validation will now check if the VM is set with custom CPU model and
show it to the user.

Signed-off-by: Liran Rotenberg <[email protected]>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
7.0% Duplication on New Code

See analysis details on SonarCloud

@ahadas
Copy link
Member

ahadas commented Dec 25, 2023

When importing from oVirt, a VM may set with custom CPU model. This PR will preserve it. In other case, the user may wish to migrate and preserve the cluster CPU model. Therefore, in the plan spec it will be possible now to set PerserveClusterCPUModel to true and forklift will set the VMs with the source cluster CPU model.

@liranr23 please update the description of the PR before merging

@liranr23 liranr23 marked this pull request as ready for review December 25, 2023 09:53
@liranr23 liranr23 merged commit 4ea0907 into kubev2v:main Dec 25, 2023
10 checks passed
@liranr23 liranr23 deleted the cpu_model branch December 25, 2023 10:07
@liranr23 liranr23 mentioned this pull request Jun 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahadas ahadas ahadas approved these changes

@bennyz bennyz Awaiting requested review from bennyz

@bkhizgiy bkhizgiy Awaiting requested review from bkhizgiy

Assignees
No one assigned
Labels
enhancement New feature or request RHV
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@liranr23 @ahadas