import smaller root disk sizes by DaanHoogland · Pull Request #8488 · apache/cloudstack

DaanHoogland · 2024-01-10T08:08:06Z

Description

This PR tries to deal with on of the leftovers from #8465 as reported in #8439

when root size is 0 it is set to 1 to allow for creation of a disk.

still remaining and to be addressed separately:

manual adding an IP address on import fails. this PR adds some logging to try and explain the issue

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?

DaanHoogland · 2024-01-10T08:08:21Z

@blueorangutan package

blueorangutan · 2024-01-10T08:10: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.

codecov · 2024-01-10T08:17:24Z

Codecov Report

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

Comparison is base (59e78cb) 30.79% compared to head (1a37043) 30.81%.

Files	Patch %	Lines
.../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java	33.33%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8488      +/-   ##
============================================
+ Coverage     30.79%   30.81%   +0.02%     
- Complexity    33990    34002      +12     
============================================
  Files          5341     5341              
  Lines        374961   374962       +1     
  Branches      54543    54543              
============================================
+ Hits         115452   115544      +92     
+ Misses       244241   244147      -94     
- Partials      15268    15271       +3

Flag	Coverage Δ
simulator-marvin-tests	`24.72% <0.00%> (+0.03%)`	⬆️
uitests	`4.39% <ø> (ø)`
unit-tests	`16.48% <33.33%> (-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.

shwstppr

Code LGTM

blueorangutan · 2024-01-10T09:22:19Z

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

DaanHoogland · 2024-01-10T09:44:23Z

the rootsize issue seems solved (tested)
the available ips for the network return as an empty list however, so that part needs work;
networkModel.getAvailableIps(network, ipAddresses.getIp4Address()); returns an empty list instead of the list of available IP addresses, because of

2024-01-10 09:40:24,046 DEBUG [c.c.n.NetworkModelImpl] (API-Job-Executor-1:ctx-97ef3f2c job-60 ctx-e9d0a030) (logid:eb59ab23) network defaultGuestNetwork[e77313d2-97d5-4b08-bfcd-2a058f6a4ef0] has no CIDR, returning empty addrress set.

/me still looking
As it is fixing some issue I am adding it to the milestone though

blueorangutan · 2024-01-10T11:29:57Z

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

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

shwstppr

Disk size change looks okay but not sure about IP check

shwstppr · 2024-01-10T11:31:05Z

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

+            }
+            for (String usedIp : usedIps) {
+                // only half witted support for ipv6 here :(
+                if (usedIp.equals(ipAddresses.getIp4Address()) || usedIp.equals(ipAddresses.getIp6Address())) {


Will this allow passing any random IP outside of the network?

good point, maybe add a cidr check as well. The issue was that the network did not have a cidr though. Not sure how to handle that except for just accepting the ip.

@DaanHoogland then would it make sense to just fix/merge the root disk size issue and address the specific case later?

yes, but can you check again? I can split in two PRs if it still needs work

splitting out as dragons be there

blueorangutan · 2024-01-10T14:16:42Z

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

nvazquez

Code LGTM

nvazquez · 2024-01-10T15:11:02Z

@blueorangutan package

blueorangutan · 2024-01-10T15:12:03Z

@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.

shwstppr · 2024-01-11T04:05:02Z

@blueorangutan package

blueorangutan · 2024-01-11T04:16: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.

sureshanaparti

code lgtm

blueorangutan · 2024-01-11T05:20:30Z

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

DaanHoogland · 2024-01-11T08:06:56Z

@blueorangutan package

blueorangutan · 2024-01-11T08:20: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.

blueorangutan · 2024-01-11T09:29:53Z

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

shwstppr · 2024-01-11T11:45:42Z

Tested this. Import works fine but volume size is returned as 0 for imported VM with smaller size volumes.

listUnmanagedInstances output,

{
    "listunmanagedinstancesresponse": {
        "count": 1,
        "unmanagedinstance": [
            {
                "name": "i-2-36-VM",
                "clusterid": "4c948570-9ceb-465a-829a-88eb777ce13f",
                "clustername": "p1-c2",
                "hostid": "9d1e614a-b9b6-4b57-8df5-8dd49a2501f3",
                "hostname": "ref-trl-5272-v-mr8-abhishek-kumar-kvm1",
                "powerstate": "PowerOn",
                "cpunumber": 1,
                "cpucorepersocket": -1,
                "cpuspeed": 250,
                "memory": 512,
                "disk": [
                    {
                        "id": "0",
                        "label": "hda",
                        "capacity": 52428800,
                        "imagepath": "/mnt/ef4f8bb2-c104-30cc-a3a2-087548c11d6f/3447c0bb-1961-4bb0-bcb3-2e06c7d35c9b",
                        "controller": "ide",
                        "position": 0,
                        "datastorename": "/acs/primary/ref-trl-5272-v-Mr8-abhishek-kumar/ref-trl-5272-v-Mr8-abhishek-kumar-kvm-pri1",
                        "datastorehost": "10.1.32.4",
                        "datastorepath": "/acs/primary/ref-trl-5272-v-Mr8-abhishek-kumar/ref-trl-5272-v-Mr8-abhishek-kumar-kvm-pri1",
                        "datastoretype": "file"
                    }
                ],
                "nic": [
                    {
                        "id": "0",
                        "networkname": "vnet0",
                        "macaddress": "02:01:00:cd:00:05",
                        "vlanid": -1,
                        "adaptertype": "e1000"
                    }
                ]
            }
        ]
    }
}

Import called with params,

name=i-2-36-VM&clusterid=4c948570-9ceb-465a-829a-88eb777ce13f&displayname=test&zoneid=b4a732bb-e837-4682-bbb9-b845b04a17dd&importsource=unmanaged&hypervisor=kvm&host=&username=&password=&diskpath=&temppath=&serviceofferingid=cec984be-1c57-468b-b1b1-94ea3cef705e&templateid=9e2ebcc2-d325-456e-8cad-1e1485e32d6a&nicnetworklist[0].nic=0&nicnetworklist[0].network=074dd540-2414-4ce2-9a51-f1888b8839c4

Template used in Macchinina

ListVM output for the imported VM,

{
    "listvirtualmachinesmetricsresponse": {
        "count": 1,
        "virtualmachine": [
            {
                "cputotal": "0.5 Ghz",
                "memorytotal": "0.50 GiB",
                "networkread": "0.01 MiB",
                "networkwrite": "0.00 MiB",
                "diskread": "0.00 MiB",
                "diskwrite": "0.00 MiB",
                "diskiopstotal": 0,
                "id": "62f777a5-9dbb-472f-b70b-a2635bbaa2f0",
                "name": "test",
                "displayname": "test",
                "account": "admin",
                "userid": "c77dbaa7-af98-11ee-847d-1e0058000a5d",
                "username": "admin",
                "domainid": "a2138f1e-af98-11ee-847d-1e0058000a5d",
                "domain": "ROOT",
                "created": "2024-01-11T11:43:01+0000",
                "state": "Running",
                "haenable": false,
                "zoneid": "b4a732bb-e837-4682-bbb9-b845b04a17dd",
                "zonename": "ref-trl-5272-v-Mr8-abhishek-kumar",
                "hostid": "9d1e614a-b9b6-4b57-8df5-8dd49a2501f3",
                "hostname": "ref-trl-5272-v-mr8-abhishek-kumar-kvm1",
                "hostcontrolstate": "Enabled",
                "templateid": "9e2ebcc2-d325-456e-8cad-1e1485e32d6a",
                "templatename": "macc-kvm",
                "templatetype": "USER",
                "templatedisplaytext": "macc-kvm",
                "passwordenabled": false,
                "serviceofferingid": "cec984be-1c57-468b-b1b1-94ea3cef705e",
                "serviceofferingname": "Small Instance",
                "cpunumber": 1,
                "cpuspeed": 500,
                "memory": 512,
                "cpuused": "3.17%",
                "networkkbsread": 7,
                "networkkbswrite": 0,
                "diskkbsread": 0,
                "diskkbswrite": 0,
                "memorykbs": 524288,
                "memoryintfreekbs": -1,
                "memorytargetkbs": 524288,
                "diskioread": 0,
                "diskiowrite": 0,
                "guestosid": "a23b8c54-af98-11ee-847d-1e0058000a5d",
                "rootdeviceid": 0,
                "rootdevicetype": "ROOT",
                "securitygroup": [],
                "nic": [
                    {
                        "id": "947586e0-5c81-4a1f-980c-60ba6f0ae4ff",
                        "networkid": "074dd540-2414-4ce2-9a51-f1888b8839c4",
                        "networkname": "testL2",
                        "isolationuri": "vlan://1760",
                        "broadcasturi": "vlan://1760",
                        "traffictype": "Guest",
                        "type": "L2",
                        "isdefault": true,
                        "macaddress": "02:01:00:cd:00:05",
                        "secondaryip": [],
                        "extradhcpoption": [],
                        "deviceid": "0"
                    }
                ],
                "hypervisor": "KVM",
                "instancename": "i-2-36-VM",
                "details": {
                    "deployvm": "true",
                    "kvm.vnc.password": "lqU9pMsfaaaaaaaaaaaaaa",
                    "rootdisksize": "1",
                    "nicAdapter": "e1000",
                    "rootDiskController": "ide"
                },
                "affinitygroup": [],
                "displayvm": true,
                "isdynamicallyscalable": false,
                "ostypeid": "a23b8c54-af98-11ee-847d-1e0058000a5d",
                "osdisplayname": "Other Linux (64-bit)",
                "pooltype": "NetworkFilesystem",
                "receivedbytes": 0,
                "sentbytes": 0,
                "tags": [],
                "hasannotations": false
            }
        ]
    }
}

List volumes output for the VM volume,

{
    "listvolumesresponse": {
        "count": 1,
        "volume": [
            {
                "sizegb": "0.00 GiB",
                "id": "c888227f-6676-487c-9625-b09e199abdb3",
                "name": "ROOT-39",
                "zoneid": "b4a732bb-e837-4682-bbb9-b845b04a17dd",
                "zonename": "ref-trl-5272-v-Mr8-abhishek-kumar",
                "type": "ROOT",
                "deviceid": 0,
                "virtualmachineid": "62f777a5-9dbb-472f-b70b-a2635bbaa2f0",
                "templateid": "9e2ebcc2-d325-456e-8cad-1e1485e32d6a",
                "templatename": "macc-kvm",
                "templatedisplaytext": "macc-kvm",
                "vmname": "test",
                "vmdisplayname": "test",
                "vmstate": "Running",
                "vmtype": "User",
                "provisioningtype": "thin",
                "size": 0,
                "created": "2024-01-11T11:43:01+0000",
                "state": "Ready",
                "account": "admin",
                "domainid": "a2138f1e-af98-11ee-847d-1e0058000a5d",
                "domain": "ROOT",
                "storagetype": "Shared (ROOT)",
                "hypervisor": "KVM",
                "storage": "ref-trl-5272-v-Mr8-abhishek-kumar-kvm-pri1",
                "attached": "2024-01-11T11:43:01+0000",
                "serviceofferingid": "1",
                "serviceofferingname": "Small Instance",
                "serviceofferingdisplaytext": "Small Instance",
                "isextractable": false,
                "displayvolume": true,
                "path": "/mnt/ef4f8bb2-c104-30cc-a3a2-087548c11d6f/3447c0bb-1961-4bb0-bcb3-2e06c7d35c9b",
                "storageid": "ef4f8bb2-c104-30cc-a3a2-087548c11d6f",
                "quiescevm": false,
                "supportsstoragesnapshot": false,
                "physicalsize": 1052672,
                "virtualsize": 52428800,
                "utilization": "2.0%",
                "clusterid": "4c948570-9ceb-465a-829a-88eb777ce13f",
                "clustername": "p1-c2",
                "podid": "639ef97b-fa03-4de1-8d55-c262234fc033",
                "podname": "Pod1",
                "tags": [],
                "hasannotations": false
            }
        ]
    }
}

DaanHoogland · 2024-01-11T12:47:27Z

ok, @shwstppr so there is another place where rounding is done. Should this block this PR or an RC? Or should we create a left-over ticket and move on for now? (cc @nvazquez )

ad. I have found a second spot where the rounding error occurred and am retesting

blueorangutan · 2024-01-11T14:31:04Z

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

DaanHoogland · 2024-01-11T14:58:49Z

ok, @shwstppr so there is another place where rounding is done. Should this block this PR or an RC? Or should we create a left-over ticket and move on for now? (cc @nvazquez )

ad. I have found a second spot where the rounding error occurred and am retesting

this is now solved is a way, but the size is now rounded to 1Gb. this seems to be a limitation of the feature. I am not sure if there is something to be done about that (@nvazquez ?)

shwstppr · 2024-01-12T05:21:45Z

@DaanHoogland I tested on 4.18.1 and the problem from #8488 (comment) exists there as well. Should we merge without the last change?
I'm testing #8500 it should probably fix the issue. In code we are currently unnecessarily doing conversion twice (probably I added that 😶‍🌫️ )

DaanHoogland · 2024-01-12T07:53:25Z

@DaanHoogland I tested on 4.18.1 and the problem from #8488 (comment) exists there as well. Should we merge without the last change? I'm testing #8500 it should probably fix the issue. In code we are currently unnecessarily doing conversion twice (probably I added that 😶‍🌫️ )

ok, by the look of c01e162 we can abandon this PR.

DaanHoogland · 2024-01-12T08:55:17Z

this is incorporated in #8500 as far as needed

DaanHoogland requested review from nvazquez and shwstppr January 10, 2024 08:08

boring-cyborg bot added the component:networking label Jan 10, 2024

DaanHoogland mentioned this pull request Jan 10, 2024

Some issues with managed/unmanaged VMs on KVM #8439

Closed

shwstppr approved these changes Jan 10, 2024

View reviewed changes

DaanHoogland mentioned this pull request Jan 10, 2024

Fix Vmware to KVM migration issue #8485

Merged

13 tasks

DaanHoogland added this to the 4.19.0.0 milestone Jan 10, 2024

shwstppr self-requested a review January 10, 2024 11:29

shwstppr reviewed Jan 10, 2024

View reviewed changes

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

shwstppr reviewed Jan 10, 2024

View reviewed changes

DaanHoogland changed the title ~~import unmanaged instance issues adressed~~ import smaller root disk sizes Jan 10, 2024

nvazquez approved these changes Jan 10, 2024

View reviewed changes

sureshanaparti approved these changes Jan 11, 2024

View reviewed changes

DaanHoogland marked this pull request as ready for review January 11, 2024 08:04

round up root disk size to the nearest accptable size for import

1a37043

DaanHoogland force-pushed the furtherUnmanagedFixes branch from f045990 to 1a37043 Compare January 11, 2024 13:08

DaanHoogland mentioned this pull request Jan 12, 2024

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

Merged

13 tasks

DaanHoogland closed this Jan 12, 2024

shwstppr mentioned this pull request Jan 12, 2024

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

Merged

13 tasks

Comments

Conversation

DaanHoogland commented Jan 10, 2024 • edited 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!

DaanHoogland commented Jan 10, 2024

Uh oh!

blueorangutan commented Jan 10, 2024

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 10, 2024

Uh oh!

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

Uh oh!

Uh oh!

blueorangutan commented Jan 10, 2024

Uh oh!

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

shwstppr Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 10, 2024

Uh oh!

nvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jan 10, 2024

Uh oh!

blueorangutan commented Jan 10, 2024

Uh oh!

shwstppr commented Jan 11, 2024

Uh oh!

blueorangutan commented Jan 11, 2024

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 11, 2024

Uh oh!

DaanHoogland commented Jan 11, 2024

Uh oh!

blueorangutan commented Jan 11, 2024

Uh oh!

blueorangutan commented Jan 11, 2024

Uh oh!

shwstppr commented Jan 11, 2024

Uh oh!

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

Uh oh!

Uh oh!

blueorangutan commented Jan 11, 2024

DaanHoogland commented Jan 10, 2024 •

edited

Loading

codecov bot commented Jan 10, 2024 •

edited

Loading

DaanHoogland commented Jan 10, 2024 •

edited

Loading

DaanHoogland commented Jan 11, 2024 •

edited

Loading