Global setting to allow/disallow users to force stop a vm by abh1sar · Pull Request #9569 · apache/cloudstack

abh1sar · 2024-08-22T10:08:06Z

Description

Fixes #6629

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

Screenshots (if appropriate):

How Has This Been Tested?

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

codecov · 2024-08-22T10:16:22Z

Codecov Report

Attention: Patch coverage is 15.38462% with 11 lines in your changes missing coverage. Please review.

Project coverage is 15.57%. Comparing base (1e12a80) to head (1ccb94f).
Report is 60 commits behind head on main.

Files with missing lines	Patch %	Lines
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java	16.66%	5 Missing ⚠️
.../cloudstack/api/response/CapabilitiesResponse.java	0.00%	3 Missing ⚠️
...in/java/com/cloud/server/ManagementServerImpl.java	0.00%	2 Missing ⚠️
...k/api/command/user/config/ListCapabilitiesCmd.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9569      +/-   ##
============================================
+ Coverage     15.55%   15.57%   +0.02%     
- Complexity    12011    12056      +45     
============================================
  Files          5500     5506       +6     
  Lines        481889   482919    +1030     
  Branches      62240    61451     -789     
============================================
+ Hits          74966    75231     +265     
- Misses       398637   399376     +739     
- Partials       8286     8312      +26

Flag	Coverage Δ
uitests	`4.17% <ø> (-0.01%)`	⬇️
unittests	`16.35% <15.38%> (+0.02%)`	⬆️

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.

vishesh92

clgtm. didn't test.

abh1sar · 2024-08-22T11:50:02Z

@blueorangutan package

blueorangutan · 2024-08-22T11:52:04Z

@abh1sar 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-08-22T13:00:23Z

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

DaanHoogland · 2024-08-22T13:49:54Z

@blueorangutan test

blueorangutan · 2024-08-22T13:52:03Z

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

blueorangutan · 2024-08-23T04:29:31Z

[SF] Trillian test result (tid-11144)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 49951 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9569-t11144-kvm-ol8.zip
Smoke tests completed. 138 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_isolate_network_FW_PF_default_routes_egress_true	`Failure`	109.38	test_routers_network_ops.py

shwstppr

Functionality looks okay, left some comments

shwstppr · 2024-08-23T06:03:56Z

    @Param(description = "true if users can see all accounts within the same domain, false otherwise")
    private boolean allowUserViewAllDomainAccounts;

+    @SerializedName("allowuserforcestopvm")


Better to define it in ApiConstants

shwstppr · 2024-08-23T06:04:20Z

        response.setAllowUserExpungeRecoverVM((Boolean)capabilities.get("allowUserExpungeRecoverVM"));
        response.setAllowUserExpungeRecoverVolume((Boolean)capabilities.get("allowUserExpungeRecoverVolume"));
        response.setAllowUserViewAllDomainAccounts((Boolean)capabilities.get("allowUserViewAllDomainAccounts"));
+        response.setAllowUserForceStopVM((Boolean)capabilities.get("allowUserForceStopVM"));


Use key from ApiConstants?

shwstppr · 2024-08-23T06:05:16Z

        capabilities.put("allowUserExpungeRecoverVM", allowUserExpungeRecoverVM);
        capabilities.put("allowUserExpungeRecoverVolume", allowUserExpungeRecoverVolume);
        capabilities.put("allowUserViewAllDomainAccounts", allowUserViewAllDomainAccounts);
+        capabilities.put("allowUserForceStopVM", allowUserForceStopVM);


same as previous comment

abh1sar · 2024-08-23T09:45:34Z

@blueorangutan package

blueorangutan · 2024-08-23T09:46:04Z

@abh1sar 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-08-23T10:46:02Z

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

abh1sar · 2024-08-23T11:26:54Z

@blueorangutan package

blueorangutan · 2024-08-23T11:28:04Z

@abh1sar 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-08-23T12:38:18Z

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

DaanHoogland · 2024-08-26T07:53:25Z

@blueorangutan test

blueorangutan · 2024-08-26T07:54:04Z

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

blueorangutan · 2024-08-26T22:45:05Z

[SF] Trillian test result (tid-11175)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 50884 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9569-t11175-kvm-ol8.zip
Smoke tests completed. 139 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

sureshanaparti

minor comment, changes lgtm

Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

bernardodemarco

LGTM, manually tested in a local environment, here are the test descriptions.

Test Descriptions

I created two User accounts, u1 and u2. Then, I set the allow.user.force.stop.vm setting to true for the u1 account and to false for the u2 account.

Tests with `u1` account

UI

I selected the option to stop a running VM and, as expected, the Force input field was rendered on the confirmation form.

CloudMonkey

I executed the stopVirtualMachine API, with the parameter forced set to true, and the virtual machine was stopped successfully.

Tests with `u2` account

UI

I selected the option to stop a running VM and, as expected, the Force input field was not rendered on the confirmation form.

CloudMonkey

I executed the stopVirtualMachine API, with the parameter forced set to true, and received the following response:

(localcloud) 🐱 > stop virtualmachine id="de80cbfc-e736-413d-be78-113e093955de" forced="true"
{
  "account": "u2",
  "accountid": "80cc6bd6-6bc5-43d7-85b9-84557ef7501f",
  "cmd": "org.apache.cloudstack.api.command.user.vm.StopVMCmd",
  "completed": "2024-09-01T13:55:56+0000",
  "created": "2024-09-01T13:55:56+0000",
  "domainid": "6b5335fb-4b6f-11ef-87b4-cec422422af1",
  "domainpath": "ROOT",
  "jobid": "f47b1388-6ed5-4c6b-b6f8-9754fbae3863",
  "jobinstanceid": "de80cbfc-e736-413d-be78-113e093955de",
  "jobinstancetype": "VirtualMachine",
  "jobprocstatus": 0,
  "jobresult": {
    "errorcode": 530,
    "errortext": "Account does not have the permission to force stop the vm."
  },
  "jobresultcode": 530,
  "jobresulttype": "object",
  "jobstatus": 2,
  "userid": "852c7737-b9eb-479c-8613-e366e165b6e2"
}
🙈 Error: async API failed for job f47b1388-6ed5-4c6b-b6f8-9754fbae3863

As can be seen, the operation was not completed successfully, which is the expected behavior.

Co-authored-by: Bernardo De Marco Gonçalves <bernardomg2004@gmail.com>

…ache#9569) Global setting to allow/disallow users to force stop a vm Fixes apache#6629 Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com> Co-authored-by: Rohit Yadav <rohityadav89@gmail.com> Co-authored-by: Bernardo De Marco Gonçalves <bernardomg2004@gmail.com>

abh1sar added 2 commits August 22, 2024 15:31

Global setting to allow/disallow users to force stop a vm

f491881

remove extra space before method call

28039ff

boring-cyborg Bot added component:api component:management-server component:UI labels Aug 22, 2024

vishesh92 approved these changes Aug 22, 2024

View reviewed changes

shwstppr approved these changes Aug 23, 2024

View reviewed changes

abh1sar and others added 2 commits August 23, 2024 14:37

moved allowuserforcestopvm to ApiConstants

7eb84df

Merge branch 'main' into stop-vm

2480254

sureshanaparti reviewed Aug 27, 2024

View reviewed changes

Comment thread server/src/main/java/com/cloud/vm/UserVmManagerImpl.java Outdated

sureshanaparti approved these changes Aug 27, 2024

View reviewed changes

DaanHoogland added the status:needs-testing label Aug 28, 2024

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

9245206

Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

bernardodemarco approved these changes Sep 1, 2024

View reviewed changes

Comment thread server/src/main/java/com/cloud/vm/UserVmManagerImpl.java Outdated

DaanHoogland removed the status:needs-testing label Sep 2, 2024

yadvr added this to the 4.20.0.0 milestone Sep 5, 2024

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

1ccb94f

Co-authored-by: Bernardo De Marco Gonçalves <bernardomg2004@gmail.com>

yadvr merged commit b7a2b04 into apache:main Sep 5, 2024

abh1sar deleted the stop-vm branch April 18, 2026 01:02

Uh oh!

Conversation

abh1sar commented Aug 22, 2024

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 Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vishesh92 left a comment

Choose a reason for hiding this comment

Uh oh!

abh1sar commented Aug 22, 2024

Uh oh!

blueorangutan commented Aug 22, 2024

Uh oh!

blueorangutan commented Aug 22, 2024

Uh oh!

DaanHoogland commented Aug 22, 2024

Uh oh!

blueorangutan commented Aug 22, 2024

Uh oh!

blueorangutan commented Aug 23, 2024

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

shwstppr Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

abh1sar Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

abh1sar Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

abh1sar Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

abh1sar commented Aug 23, 2024

Uh oh!

blueorangutan commented Aug 23, 2024

Uh oh!

blueorangutan commented Aug 23, 2024

Uh oh!

abh1sar commented Aug 23, 2024

Uh oh!

blueorangutan commented Aug 23, 2024

Uh oh!

blueorangutan commented Aug 23, 2024

Uh oh!

DaanHoogland commented Aug 26, 2024

Uh oh!

blueorangutan commented Aug 26, 2024

Uh oh!

blueorangutan commented Aug 26, 2024

Uh oh!

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

bernardodemarco left a comment

Choose a reason for hiding this comment

Test Descriptions

Tests with u1 account

UI

CloudMonkey

codecov Bot commented Aug 22, 2024 •

edited

Loading

Tests with `u1` account

Tests with `u2` account