server: fix resource count updated unmanage vm by shwstppr · Pull Request #8169 · apache/cloudstack

shwstppr · 2023-11-01T09:06:08Z

Description

This PR ensures that the resource counts are not decremented twice for a running VM while unmanaging.
Currently, the code does the same operation for a non-stopped VM twice. This could lead to resource count discrepancy for an account or domain.

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?

Before fix:

Create an account
Deploy a VM for that account
Observe the account limits and count
Unmanage the VM
Again observe the account limits and count. There will be some negative values for VM, CPU and memory as the resource counts have been decremented twice

(localcloud) 🐱 > list virtualmachines account=ACSuser
{
  "count": 1,
  "virtualmachine": [
    {
      "account": "ACSUser",
      "affinitygroup": [],
      "cpunumber": 1,
      "cpuspeed": 500,
      "created": "2024-06-12T12:48:59+0000",
      "details": {
        "cpuOvercommitRatio": "2.0",
        "dataDiskController": "osdefault",
        "memoryOvercommitRatio": "1.0",
        "rootDiskController": "ide"
      },
      "displayname": "uservm",
      "displayvm": true,
      "domain": "ROOT",
      "domainid": "8ce79cd7-1d86-11ef-a886-1e0095000407",
      "guestosid": "8cf5785f-1d86-11ef-a886-1e0095000407",
      "haenable": false,
      "hasannotations": false,
      "hostcontrolstate": "Enabled",
      "hostid": "13b19f23-b01f-49b3-98bb-f708cc668b47",
      "hostname": "10.0.35.149",
      "hypervisor": "VMware",
      "id": "bab406c5-f235-4cd2-b365-10338d613f7a",
      "instancename": "i-4-10-VM",
      "isdynamicallyscalable": false,
      "lastupdated": "2024-06-12T12:49:18+0000",
      "memory": 512,
      "name": "uservm",
      "nic": [
        {
          "broadcasturi": "vlan://2047",
          "deviceid": "0",
          "extradhcpoption": [],
          "id": "30a97242-158e-41ae-ac4b-eefbc73d09a8",
          "isdefault": true,
          "isolationuri": "vlan://2047",
          "macaddress": "02:01:00:ce:00:01",
          "networkid": "fb6d8567-74d6-43cc-a4bb-539c165c5a63",
          "networkname": "usernet",
          "secondaryip": [],
          "traffictype": "Guest",
          "type": "L2"
        }
      ],
      "osdisplayname": "CentOS 5.3 (64-bit)",
      "ostypeid": "8cf5785f-1d86-11ef-a886-1e0095000407",
      "passwordenabled": false,
      "pooltype": "NetworkFilesystem",
      "receivedbytes": 0,
      "rootdeviceid": 0,
      "rootdevicetype": "ROOT",
      "securitygroup": [],
      "sentbytes": 0,
      "serviceofferingid": "7d856b41-9a5e-40c4-b96d-72217a82fa5f",
      "serviceofferingname": "Small Instance",
      "state": "Running",
      "tags": [],
      "templatedisplaytext": "CentOS 5.3(64-bit) no GUI (vSphere)",
      "templateformat": "OVA",
      "templateid": "8cf00de6-1d86-11ef-a886-1e0095000407",
      "templatename": "CentOS 5.3(64-bit) no GUI (vSphere)",
      "templatetype": "BUILTIN",
      "userid": "cd6f78c9-ed6f-44ae-8733-570148c28540",
      "username": "user",
      "zoneid": "40e1cd6a-9353-4325-bd43-d0ffa691f10e",
      "zonename": "pr8601-t10311-vmware-67u3"
    }
  ]
}
(localcloud) 🐱 > list accounts id=39223f34-f465-438f-8d43-04ff0670514d 
{
  "account": [
    {
      "accounttype": 0,
      "cpuavailable": "39",
      "cpulimit": "40",
      "cputotal": 1,
      "created": "2024-05-29T06:48:57+0000",
      "domain": "ROOT",
      "domainid": "8ce79cd7-1d86-11ef-a886-1e0095000407",
      "domainpath": "ROOT",
      "groups": [],
      "id": "39223f34-f465-438f-8d43-04ff0670514d",
      "ipavailable": "16",
      "iplimit": "20",
      "iptotal": 0,
      "isdefault": false,
      "memoryavailable": "40448",
      "memorylimit": "40960",
      "memorytotal": 512,
      "name": "ACSUser",
      "networkavailable": "19",
      "networklimit": "20",
      "networktotal": 1,
      "primarystorageavailable": "198",
      "primarystoragelimit": "200",
      "primarystoragetotal": 2,
      "projectavailable": "Unlimited",
      "projectlimit": "Unlimited",
      "projecttotal": 0,
      "roleid": "a51b7f95-1d86-11ef-a886-1e0095000407",
      "rolename": "User",
      "roletype": "User",
      "secondarystorageavailable": "400.0",
      "secondarystoragelimit": "400",
      "secondarystoragetotal": 0,
      "snapshotavailable": "20",
      "snapshotlimit": "20",
      "snapshottotal": 0,
      "state": "enabled",
      "templateavailable": "20",
      "templatelimit": "20",
      "templatetotal": 0,
      "user": [
        {
          "account": "ACSUser",
          "accountid": "39223f34-f465-438f-8d43-04ff0670514d",
          "accounttype": 0,
          "created": "2024-05-29T06:49:01+0000",
          "domain": "ROOT",
          "domainid": "8ce79cd7-1d86-11ef-a886-1e0095000407",
          "email": "sblab@shapeblue.com",
          "firstname": "ACloudStack",
          "id": "cd6f78c9-ed6f-44ae-8733-570148c28540",
          "is2faenabled": false,
          "is2famandated": false,
          "iscallerchilddomain": false,
          "isdefault": false,
          "lastname": "User",
          "roleid": "a51b7f95-1d86-11ef-a886-1e0095000407",
          "rolename": "User",
          "roletype": "User",
          "state": "enabled",
          "timezone": "Etc/UTC",
          "username": "user",
          "usersource": "native"
        }
      ],
      "vmavailable": "19",
      "vmlimit": "20",
      "vmrunning": 1,
      "vmstopped": 0,
      "vmtotal": 1,
      "volumeavailable": "19",
      "volumelimit": "20",
      "volumetotal": 1,
      "vpcavailable": "20",
      "vpclimit": "20",
      "vpctotal": 0
    }
  ],
  "count": 1
}
(localcloud) 🐱 > unmanage virtualmachine id=bab406c5-f235-4cd2-b365-10338d613f7a 
{
  "unmanagevirtualmachineresponse": {
    "details": "VM unmanaged successfully",
    "success": true
  }
}
(localcloud) 🐱 > list accounts id=39223f34-f465-438f-8d43-04ff0670514d 
{
  "account": [
    {
      "accounttype": 0,
      "cpuavailable": "41",
      "cpulimit": "40",
      "cputotal": -1,
      "created": "2024-05-29T06:48:57+0000",
      "domain": "ROOT",
      "domainid": "8ce79cd7-1d86-11ef-a886-1e0095000407",
      "domainpath": "ROOT",
      "groups": [],
      "id": "39223f34-f465-438f-8d43-04ff0670514d",
      "ipavailable": "16",
      "iplimit": "20",
      "iptotal": 0,
      "isdefault": false,
      "memoryavailable": "41472",
      "memorylimit": "40960",
      "memorytotal": -512,
      "name": "ACSUser",
      "networkavailable": "19",
      "networklimit": "20",
      "networktotal": 1,
      "primarystorageavailable": "200",
      "primarystoragelimit": "200",
      "primarystoragetotal": 0,
      "projectavailable": "Unlimited",
      "projectlimit": "Unlimited",
      "projecttotal": 0,
      "roleid": "a51b7f95-1d86-11ef-a886-1e0095000407",
      "rolename": "User",
      "roletype": "User",
      "secondarystorageavailable": "400.0",
      "secondarystoragelimit": "400",
      "secondarystoragetotal": 0,
      "snapshotavailable": "20",
      "snapshotlimit": "20",
      "snapshottotal": 0,
      "state": "enabled",
      "templateavailable": "20",
      "templatelimit": "20",
      "templatetotal": 0,
      "user": [
        {
          "account": "ACSUser",
          "accountid": "39223f34-f465-438f-8d43-04ff0670514d",
          "accounttype": 0,
          "created": "2024-05-29T06:49:01+0000",
          "domain": "ROOT",
          "domainid": "8ce79cd7-1d86-11ef-a886-1e0095000407",
          "email": "sblab@shapeblue.com",
          "firstname": "ACloudStack",
          "id": "cd6f78c9-ed6f-44ae-8733-570148c28540",
          "is2faenabled": false,
          "is2famandated": false,
          "iscallerchilddomain": false,
          "isdefault": false,
          "lastname": "User",
          "roleid": "a51b7f95-1d86-11ef-a886-1e0095000407",
          "rolename": "User",
          "roletype": "User",
          "state": "enabled",
          "timezone": "Etc/UTC",
          "username": "user",
          "usersource": "native"
        }
      ],
      "vmavailable": "21",
      "vmlimit": "20",
      "vmrunning": 0,
      "vmstopped": 0,
      "vmtotal": -1,
      "volumeavailable": "20",
      "volumelimit": "20",
      "volumetotal": 0,
      "vpcavailable": "20",
      "vpclimit": "20",
      "vpctotal": 0
    }
  ],
  "count": 1
}

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

codecov · 2023-11-01T10:07:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.21%. Comparing base (1577218) to head (f4776cc).
Report is 74 commits behind head on 4.19.

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #8169      +/-   ##
============================================
+ Coverage     14.96%   15.21%   +0.24%     
- Complexity    11002    11534     +532     
============================================
  Files          5373     5389      +16     
  Lines        469315   491097   +21782     
  Branches      59847    70632   +10785     
============================================
+ Hits          70233    74697    +4464     
- Misses       391309   408170   +16861     
- Partials       7773     8230     +457

Flag	Coverage Δ
uitests	`4.34% <ø> (+0.04%)`	⬆️
unittests	`15.91% <ø> (+0.23%)`	⬆️

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.

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

DaanHoogland · 2024-04-19T09:22:27Z

@shwstppr @vishesh92 , in view of other work on this topic, should we keep this open?

Do not decrement resource count for a running VM twice while unmanaging. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

github-actions · 2024-06-12T07:46:18Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

shwstppr · 2024-06-12T07:47:48Z

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

            UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_STOP, vm.getAccountId(), vm.getDataCenterId(),
                    vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
                    vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
-            resourceCountDecrement(vm.getAccountId(), vm.isDisplayVm(), cpu, ram);


Same method is called later at line 8502

shwstppr · 2024-06-12T07:48:07Z

@blueorangutan package

blueorangutan · 2024-06-12T07:50:06Z

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

DaanHoogland

clgtm

blueorangutan · 2024-06-12T09:31:54Z

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

shwstppr · 2024-06-12T12:56:20Z

Added steps to reproduce in the PR description. cc @DaanHoogland

yadvr · 2024-06-25T10:54:35Z

@blueorangutan package

blueorangutan · 2024-06-25T10:56:07Z

@rohityadavcloud 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-06-25T11:58:31Z

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

vishesh92

clgtm. Tested.
Before patch. resource count got decremented by 2. After patch, it got decremented by 1.

Do not decrement resource count for a running VM twice while unmanaging a VM

DaanHoogland reviewed Nov 1, 2023

View reviewed changes

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java Outdated Show resolved Hide resolved

server: fix resource count updated unmanage vm

f4776cc

Do not decrement resource count for a running VM twice while unmanaging. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr force-pushed the fix-unmanagevm-resourcecount branch from 02d36fa to f4776cc Compare June 12, 2024 07:46

boring-cyborg bot added component:agent component:api component:build component:integration labels Jun 12, 2024

github-actions bot added the status:has-conflicts label Jun 12, 2024

shwstppr changed the base branch from 4.18 to 4.19 June 12, 2024 07:46

shwstppr closed this Jun 12, 2024

shwstppr reopened this Jun 12, 2024

shwstppr commented Jun 12, 2024

View reviewed changes

shwstppr marked this pull request as ready for review June 12, 2024 07:50

DaanHoogland approved these changes Jun 12, 2024

View reviewed changes

github-actions bot removed the status:has-conflicts label Jun 12, 2024

shwstppr added the status:needs-testing label Jun 12, 2024

yadvr approved these changes Jun 25, 2024

View reviewed changes

yadvr added this to the 4.19.1.0 milestone Jun 25, 2024

yadvr closed this Jun 25, 2024

yadvr reopened this Jun 25, 2024

sureshanaparti assigned sureshanaparti and vishesh92 Jun 26, 2024

vishesh92 approved these changes Jun 26, 2024

View reviewed changes

vishesh92 removed the status:needs-testing label Jun 26, 2024

vishesh92 merged commit 7a8066d into apache:4.19 Jun 26, 2024

vishesh92 deleted the fix-unmanagevm-resourcecount branch June 26, 2024 09:04

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

server: fix resource count updated unmanage vm (apache#8169)

7586d7e

Do not decrement resource count for a running VM twice while unmanaging a VM

Conversation

shwstppr commented Nov 1, 2023 • 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!

codecov bot commented Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

DaanHoogland commented Apr 19, 2024

Uh oh!

github-actions bot commented Jun 12, 2024

Uh oh!

shwstppr Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr commented Jun 12, 2024

Uh oh!

blueorangutan commented Jun 12, 2024

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jun 12, 2024

Uh oh!

shwstppr commented Jun 12, 2024

Uh oh!

yadvr commented Jun 25, 2024

Uh oh!

blueorangutan commented Jun 25, 2024

Uh oh!

blueorangutan commented Jun 25, 2024

Uh oh!

vishesh92 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

shwstppr commented Nov 1, 2023 •

edited

Loading

codecov bot commented Nov 1, 2023 •

edited

Loading