Skip to content

Conversation

@shwstppr
Copy link
Contributor

@shwstppr shwstppr commented Nov 4, 2025

Description

Fixes #9878

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
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

(local) 🐱 > update virtualmachine -h
updateVirtualMachine: Updates properties of a virtual machine. The VM has to be stopped and restarted for the new properties to take effect. UpdateVirtualMachine does not first check whether the VM is stopped. Therefore, stop the VM manually before issuing this call.
Required params: id, 
API Params               Type     Description
==========               ====     ===========
cleanupdetails           boolean  optional boolean field, which indicates i
                                  f details should be cleaned up or not (i
                                  f set to true, details removed for this 
                                  resource, details field ignored; if fals
                                  e or not set, no action)
cleanupextraconfig       boolean  Optional boolean field, which indicates i
                                  f extraconfig for the instance should be
                                   cleaned up or not (If set to true, extr
                                  aconfig removed for this instance, extra
                                  config field ignored; if false or not se
                                  t, no action)
customid                 string   an optional field, in case you want to se
                                  t a custom id to the resource. Allowed t
                                  o Root Admins only
...

(localcloud) 🐱 > list virtualmachines id=d6c566f9-bec2-4daf-b1fb-2da25683229e  filter=id,name,details
{
  "count": 1,
  "virtualmachine": [
    {
      "details": {
        "cpuOvercommitRatio": "2.0",
        "extraconfig-1": "alpha=123\nbeta=456"
      },
      "id": "d6c566f9-bec2-4daf-b1fb-2da25683229e",
      "name": "VM-d6c566f9-bec2-4daf-b1fb-2da25683229e"
    }
  ]
}
(localcloud) 🐱 > update virtualmachine id=d6c566f9-bec2-4daf-b1fb-2da25683229e cleanupextraconfig=true
{
  "virtualmachine": {
    "account": "admin",
    "affinitygroup": [],
    "arch": "x86_64",
    "cpunumber": 1,
    "cpuspeed": 500,
    "cpuused": "14.36%",
    "created": "2025-12-01T06:26:23+0000",
    "deleteprotection": false,
    "details": {
      "cpuOvercommitRatio": "2.0"
    },
    "diskioread": 0,
    "diskiowrite": 4,
    "diskkbsread": 0,
    "diskkbswrite": 24,
    "displayname": "VM-d6c566f9-bec2-4daf-b1fb-2da25683229e",
    "displayvm": true,
    "domain": "ROOT",
    "domainid": "82f87e9e-ce76-11f0-8a20-1e00060003cb",
    "domainpath": "ROOT",
    "guestosid": "83199135-ce76-11f0-8a20-1e00060003cb",
    "haenable": false,
    "hasannotations": false,
    "hostcontrolstate": "Enabled",
    "hostid": "5249cdbd-9ca9-4c00-a734-0537d89d0dbc",
    "hostname": "pr11974-t14893-kvm-ol8-kvm1",
    "hypervisor": "KVM",
    "id": "d6c566f9-bec2-4daf-b1fb-2da25683229e",
    "instancename": "i-2-3-VM",
    "isdynamicallyscalable": false,
    "lastupdated": "2025-12-01T06:26:37+0000",
    "memory": 512,
    "memoryintfreekbs": -1,
    "memorykbs": 524288,
    "memorytargetkbs": 524288,
    "name": "VM-d6c566f9-bec2-4daf-b1fb-2da25683229e",
    "networkkbsread": 0,
    "networkkbswrite": 0,
    "nic": [
      {
        "broadcasturi": "vlan://1262",
        "deviceid": "0",
        "extradhcpoption": [],
        "id": "4c576998-3258-4f52-9df1-766315d8f14e",
        "isdefault": true,
        "isolationuri": "vlan://1262",
        "macaddress": "02:01:00:cc:00:01",
        "networkid": "882de96d-a74f-4131-814d-9d8e09119125",
        "networkname": "test",
        "secondaryip": [],
        "traffictype": "Guest",
        "type": "L2"
      }
    ],
    "osdisplayname": "CentOS 5.5 (64-bit)",
    "ostypeid": "83199135-ce76-11f0-8a20-1e00060003cb",
    "passwordenabled": false,
    "pooltype": "NetworkFilesystem",
    "receivedbytes": 0,
    "rootdeviceid": 0,
    "rootdevicetype": "ROOT",
    "securitygroup": [],
    "sentbytes": 0,
    "serviceofferingid": "c049fc6b-1ff2-4ce5-a881-aaa26b9625e1",
    "serviceofferingname": "Small Instance",
    "state": "Running",
    "tags": [],
    "templatedisplaytext": "CentOS 5.5(64-bit) no GUI (KVM)",
    "templateformat": "QCOW2",
    "templateid": "82fe7d45-ce76-11f0-8a20-1e00060003cb",
    "templatename": "CentOS 5.5(64-bit) no GUI (KVM)",
    "templatetype": "BUILTIN",
    "userid": "bea6206e-ce76-11f0-8a20-1e00060003cb",
    "username": "admin",
    "zoneid": "15d72fa5-8b47-4d90-b1c2-9e2c0d0b78e5",
    "zonename": "pr11974-t14893-kvm-ol8"
  }
}
(localcloud) 🐱 > list virtualmachines id=d6c566f9-bec2-4daf-b1fb-2da25683229e  filter=id,name,details
{
  "count": 1,
  "virtualmachine": [
    {
      "details": {
        "cpuOvercommitRatio": "2.0"
      },
      "id": "d6c566f9-bec2-4daf-b1fb-2da25683229e",
      "name": "VM-d6c566f9-bec2-4daf-b1fb-2da25683229e"
    }
  ]
}
clear-vm-extraconfig.mp4

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

Fixes apache#9878

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.86%. Comparing base (04b58ac) to head (c3b6012).
⚠️ Report is 82 commits behind head on main.

Files with missing lines Patch % Lines
...he/cloudstack/api/command/user/vm/UpdateVMCmd.java 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #11974      +/-   ##
============================================
+ Coverage     17.84%   17.86%   +0.01%     
- Complexity    15983    16008      +25     
============================================
  Files          5929     5930       +1     
  Lines        531084   531457     +373     
  Branches      64914    64973      +59     
============================================
+ Hits          94795    94923     +128     
- Misses       425675   425908     +233     
- Partials      10614    10626      +12     
Flag Coverage Δ
uitests 3.58% <ø> (-0.01%) ⬇️
unittests 18.95% <89.65%> (+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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr shwstppr marked this pull request as ready for review December 1, 2025 06:46
@shwstppr
Copy link
Contributor Author

shwstppr commented Dec 1, 2025

@blueorangutan package

@blueorangutan
Copy link

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

@shwstppr shwstppr requested a review from abh1sar December 1, 2025 07:29
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

Comment on lines +43 to +48
sb.and("vmId", sb.entity().getResourceId(), SearchCriteria.Op.EQ);
sb.and("prefix", sb.entity().getName(), SearchCriteria.Op.LIKE);
sb.done();
SearchCriteria<VMInstanceDetailVO> sc = sb.create();
sc.setParameters("vmId", vmId);
sc.setParameters("prefix", prefix + "%");
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit gives me to think that we might want to include these in a DbConstants, similar to the ApiConstants. No critisism on this PR as I know this is the general practice atm.

@blueorangutan
Copy link

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

Copy link
Collaborator

@abh1sar abh1sar left a comment

Choose a reason for hiding this comment

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

A minor comment. Otherwise the code LGTM.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

shwstppr commented Dec 9, 2025

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@rajujith
Copy link

@shwstppr Since this is for the 4.22.1 release, could you retarget the PR to the 4.22 branch?

@shwstppr
Copy link
Contributor Author

@rajujith this adds a new API request param, so I think it is better to go in 4.23. I feel this was wrongly added to 4.22.1

@shwstppr shwstppr modified the milestones: 4.22.1, 4.23.0 Jan 13, 2026
@github-actions
Copy link

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

@RosiKyu RosiKyu assigned RosiKyu and unassigned RosiKyu Jan 26, 2026
@vishesh92 vishesh92 requested review from Copilot and removed request for weizhouapache January 28, 2026 07:13
@vishesh92 vishesh92 self-assigned this Jan 28, 2026
@vishesh92
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Enables updateVirtualMachine to remove (“clean up”) stored VM extraconfig entries, with UI support and unit tests.

Changes:

  • Added cleanupextraconfig API parameter to updateVirtualMachine.
  • Implemented server-side extraconfig cleanup via a new DAO helper that removes VM instance details by prefix.
  • Updated the UI to send cleanupextraconfig=true when extraconfig is cleared, and added unit tests for the new behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ui/src/views/compute/EditVM.vue Sends cleanupextraconfig when the user clears extraconfig in the Edit VM form.
api/src/main/java/org/apache/cloudstack/api/ApiConstants.java Adds the CLEAN_UP_EXTRA_CONFIG API constant.
api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java Adds the cleanupextraconfig request parameter and accessor.
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java Adds updateVmExtraConfig logic and wires cleanupextraconfig into updateVirtualMachine.
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDao.java Extends DAO contract with removeDetailsWithPrefix.
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDaoImpl.java Implements prefix-based detail deletion.
server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java Adds unit tests for updateVmExtraConfig and config toggling helper.
engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDetailsDaoImplTest.java Adds unit tests for removeDetailsWithPrefix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 426 to 427
if (values.extraconfig && values.extraconfig.length > 0) {
params.extraconfig = encodeURIComponent(values.extraconfig)
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

In the UI, whitespace-only values for extraconfig are treated as “present” (length > 0), so the request sends extraconfig instead of cleanupextraconfig. On the server side StringUtils.isNotBlank treats whitespace as blank, so this results in no update and the existing extraconfig is not cleaned. Consider trimming before the length check (or using a blank check) so clearing via whitespace triggers cleanup as expected.

Suggested change
if (values.extraconfig && values.extraconfig.length > 0) {
params.extraconfig = encodeURIComponent(values.extraconfig)
const extraConfig = typeof values.extraconfig === 'string' ? values.extraconfig.trim() : values.extraconfig
if (extraConfig && extraConfig.length > 0) {
params.extraconfig = encodeURIComponent(extraConfig)

Copilot uses AI. Check for mistakes.
@blueorangutan
Copy link

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

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

feature: cleanup VM extraconfig in updateVirtualMachine API

8 participants