Fix KVM import unmanaged instances on basic zone by nvazquez · Pull Request #8465 · apache/cloudstack

nvazquez · 2024-01-08T17:28:49Z

Description

This PR fixes import unmanaged instances on KVM basic zones, on top of #8433

Fixes: #8439: point 1

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

codecov · 2024-01-08T18:06:02Z

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (514d2c2) 30.76% compared to head (a4eb9b5) 29.89%.
Report is 6 commits behind head on main.

Files	Patch %	Lines
...tack/engine/orchestration/NetworkOrchestrator.java	43.47%	11 Missing and 2 partials ⚠️
.../main/java/org/apache/cloudstack/ServerDaemon.java	0.00%	6 Missing ⚠️
...n/java/com/cloud/network/dao/IPAddressDaoImpl.java	0.00%	5 Missing ⚠️
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java	0.00%	0 Missing and 1 partial ⚠️
.../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8465      +/-   ##
============================================
- Coverage     30.76%   29.89%   -0.87%     
+ Complexity    33938    32810    -1128     
============================================
  Files          5341     5341              
  Lines        374891   374940      +49     
  Branches      54527    54537      +10     
============================================
- Hits         115328   112094    -3234     
- Misses       244316   248020    +3704     
+ Partials      15247    14826     -421

Flag	Coverage Δ
simulator-marvin-tests	`23.41% <0.00%> (-1.24%)`	⬇️
uitests	`4.39% <ø> (ø)`
unit-tests	`16.47% <41.17%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

nvazquez · 2024-01-08T18:15:11Z

@blueorangutan package

...hestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

DaanHoogland · 2024-01-08T18:16:19Z

@blueorangutan package

blueorangutan · 2024-01-08T18:18:03Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

nvazquez · 2024-01-08T18:56:11Z

@blueorangutan package

blueorangutan · 2024-01-08T18:58:05Z

@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Pearl1594

code LGTM

blueorangutan · 2024-01-08T19:25:01Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8237

blueorangutan · 2024-01-08T20:08:16Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8238

shwstppr

Code LGTM

DaanHoogland · 2024-01-09T08:25:38Z

tested in a lab env @nvazquez , But i still see the errors described by @slavkap, for both automatic assigned and manual selected IP addresses.

DaanHoogland · 2024-01-09T08:30:32Z

tested in a lab env @nvazquez , But i still see the errors described by @slavkap, for both automatic assigned and manual selected IP addresses.

note that the rootdisksize issue is also an issue here, but I tested with a larger template to get around that. (see #8433 (comment)

shwstppr · 2024-01-09T12:17:12Z

@blueorangutan package

blueorangutan · 2024-01-09T12:18:03Z

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

nvazquez · 2024-01-09T12:29:21Z

@blueorangutan package

blueorangutan · 2024-01-09T12:30:04Z

@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-01-09T13:23:46Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8252

blueorangutan · 2024-01-09T13:33:12Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8253

DaanHoogland · 2024-01-09T14:15:10Z

point 1a) from #8439 seems fixed now.
point 1 b) remains (if you use a predetermined IP address, the import fails)
as well as the rootsize issue.
personally I think the rootsize issue is more pressing than the IP assignment as one would want ACS to be responsible for that anyway, but I can imagine some providers having a problem with that. Neither seems to be a blocker though.

cc @nvazquez @shwstppr @slavkap

nvazquez · 2024-01-09T14:38:21Z

Thanks @DaanHoogland, about the rootdisksize issue it seems the template size affects the rounding and ends up trying to resize to 0 GB, this doesn't happen with the default template but does it happen with every other user registered template below 1GB?

shwstppr · 2024-01-10T07:50:01Z

@DaanHoogland @nvazquez I'm merging this. Lets handle any remaining issues in separate PR(s)

This PR fixes import unmanaged instances on KVM basic zones, on top of apache#8433 Fixes: apache#8439: point 1

…password (#8492) This PR fixes a regression caused by #8465 on advanced zones, import fails with: 2024-01-10 12:13:33,234 DEBUG [o.a.c.e.o.NetworkOrchestrator] (API-Job-Executor-3:ctx-991bbe9f job-128 ctx-f49517d4) (logid:d7b8e716) Allocating nic for vm 142272e8-9e2e-407b-9d7e-e9a03b81653c in network Network {"id": 204, "name": "Isolated", "uuid": "9679fac5-e3ac-4694-a57b-beb635340f39", "networkofferingid": 10} during import 2024-01-10 12:13:33,239 ERROR [o.a.c.v.UnmanagedVMsManagerImpl] (API-Job-Executor-3:ctx-991bbe9f job-128 ctx-f49517d4) (logid:d7b8e716) Failed to import NICs while importing vm: i-2-31-VM com.cloud.exception.InsufficientVirtualNetworkCapacityException: Unable to acquire Guest IP address for network Network {"id": 204, "name": "Isolated", "uuid": "9679fac5-e3ac-4694-a57b-beb635340f39", "networkofferingid": 10}Scope=interface com.cloud.dc.DataCenter; id=1 at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.importNic(NetworkOrchestrator.java:4582) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importNic(UnmanagedVMsManagerImpl.java:859) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importVirtualMachineInternal(UnmanagedVMsManagerImpl.java:1198) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importUnmanagedInstanceFromHypervisor(UnmanagedVMsManagerImpl.java:1511) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.baseImportInstance(UnmanagedVMsManagerImpl.java:1342) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importUnmanagedInstance(UnmanagedVMsManagerImpl.java:1282) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) Also, addresses the VNC password field set instead of a fixed string

…password (apache#8492) This PR fixes a regression caused by apache#8465 on advanced zones, import fails with: 2024-01-10 12:13:33,234 DEBUG [o.a.c.e.o.NetworkOrchestrator] (API-Job-Executor-3:ctx-991bbe9f job-128 ctx-f49517d4) (logid:d7b8e716) Allocating nic for vm 142272e8-9e2e-407b-9d7e-e9a03b81653c in network Network {"id": 204, "name": "Isolated", "uuid": "9679fac5-e3ac-4694-a57b-beb635340f39", "networkofferingid": 10} during import 2024-01-10 12:13:33,239 ERROR [o.a.c.v.UnmanagedVMsManagerImpl] (API-Job-Executor-3:ctx-991bbe9f job-128 ctx-f49517d4) (logid:d7b8e716) Failed to import NICs while importing vm: i-2-31-VM com.cloud.exception.InsufficientVirtualNetworkCapacityException: Unable to acquire Guest IP address for network Network {"id": 204, "name": "Isolated", "uuid": "9679fac5-e3ac-4694-a57b-beb635340f39", "networkofferingid": 10}Scope=interface com.cloud.dc.DataCenter; id=1 at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.importNic(NetworkOrchestrator.java:4582) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importNic(UnmanagedVMsManagerImpl.java:859) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importVirtualMachineInternal(UnmanagedVMsManagerImpl.java:1198) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importUnmanagedInstanceFromHypervisor(UnmanagedVMsManagerImpl.java:1511) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.baseImportInstance(UnmanagedVMsManagerImpl.java:1342) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importUnmanagedInstance(UnmanagedVMsManagerImpl.java:1282) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) Also, addresses the VNC password field set instead of a fixed string

nvazquez added 2 commits January 8, 2024 14:27

Fix KVM import unmanaged on basic zones

5cf8a5d

Fix networking

030d545

nvazquez requested review from DaanHoogland, Pearl1594, harikrishna-patnala, shwstppr and slavkap January 8, 2024 17:28

boring-cyborg bot added component:networking component:orchestration component:vpc labels Jan 8, 2024

nvazquez changed the title ~~Fix kvm unmanage basic zone~~ Fix KVM import unmanaged instanes on basic zone Jan 8, 2024

Fix host import and add unit tests

cbe0c5d

DaanHoogland reviewed Jan 8, 2024

View reviewed changes

...hestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java Outdated Show resolved Hide resolved

Pearl1594 reviewed Jan 8, 2024

View reviewed changes

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java Show resolved Hide resolved

Refactor and more unit tests

b2ce132

Pearl1594 approved these changes Jan 8, 2024

View reviewed changes

shwstppr approved these changes Jan 9, 2024

View reviewed changes

shwstppr added this to the 4.19.0.0 milestone Jan 9, 2024

DaanHoogland removed the component:vpc label Jan 9, 2024

Merge branch 'main' into fix-kvm-unmanage-basic-zone

a4eb9b5

boring-cyborg bot added the component:vpc label Jan 9, 2024

DaanHoogland changed the title ~~Fix KVM import unmanaged instanes on basic zone~~ Fix KVM import unmanaged instances on basic zone Jan 9, 2024

shwstppr marked this pull request as ready for review January 10, 2024 07:48

shwstppr merged commit b8d3e34 into apache:main Jan 10, 2024

DaanHoogland mentioned this pull request Jan 10, 2024

import smaller root disk sizes #8488

Closed

13 tasks

DaanHoogland deleted the fix-kvm-unmanage-basic-zone branch January 10, 2024 08:18

nvazquez mentioned this pull request Jan 10, 2024

Fixes for KVM unmanaged instances import on advanced network and VNC password #8492

Merged

13 tasks

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jan 12, 2024

Fix KVM import unmanaged instances on basic zone (apache#8465)

4f92ee6

This PR fixes import unmanaged instances on KVM basic zones, on top of apache#8433 Fixes: apache#8439: point 1

shwstppr mentioned this pull request Jan 12, 2024

vm-import: kvm import and fix volume size when lesser than 1GiB #8500

Merged

13 tasks

Conversation

nvazquez commented Jan 8, 2024 • edited by shwstppr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

codecov bot commented Jan 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nvazquez commented Jan 8, 2024

Uh oh!

Uh oh!

DaanHoogland commented Jan 8, 2024

Uh oh!

blueorangutan commented Jan 8, 2024

Uh oh!

Uh oh!

nvazquez commented Jan 8, 2024

Uh oh!

blueorangutan commented Jan 8, 2024

Uh oh!

Pearl1594 left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 8, 2024

Uh oh!

blueorangutan commented Jan 8, 2024

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jan 9, 2024

Uh oh!

DaanHoogland commented Jan 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shwstppr commented Jan 9, 2024

Uh oh!

blueorangutan commented Jan 9, 2024

Uh oh!

nvazquez commented Jan 9, 2024

Uh oh!

blueorangutan commented Jan 9, 2024

Uh oh!

blueorangutan commented Jan 9, 2024

Uh oh!

blueorangutan commented Jan 9, 2024

Uh oh!

DaanHoogland commented Jan 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nvazquez commented Jan 9, 2024

Uh oh!

shwstppr commented Jan 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nvazquez commented Jan 8, 2024 •

edited by shwstppr

Loading

codecov bot commented Jan 8, 2024 •

edited

Loading

DaanHoogland commented Jan 9, 2024 •

edited

Loading

DaanHoogland commented Jan 9, 2024 •

edited

Loading