Add API command remove management server #10325

nicoschmdt · 2025-02-04T15:39:54Z

Description

This PR adds a command to mark a Management Server as removed on the database only if its status is marked as Down. To execute the command, it is obligatory to inform the id of the targeted MS.

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

How Has This Been Tested?

In a local lab, I added an MS through the database.

With State="Down" I executed through CMK remove managementserver id=<uuid> and verified that the MS wasn't being listed on the UI and was marked as Removed on the database.
With State="Up" I executed the same command and verified an error was returned informing it wasn't possible to remove an MS which state was marked as "Up".
With State="Down" I executed through a URL http://<IP>:8080/client/api?command=removeManagementServer&response=json&id=<UUID> and verified the MS was marked as removed and wasn't being listed on the UI.
With State="Up" I executed the same URL and received an error informing it wasn't possible to remove an MS which state was marked as "Up".
Trying to execute the command with a non-existent id returns an error warning the request format may be invalid, the entity may not exist, or the parameter annotation may be wrong.
Trying to execute the command without an id, both in CMK and with a URL, returns an error informing the id parameter is obligatory and it is missing.

api/src/main/java/org/apache/cloudstack/api/command/admin/management/RemoveMgmtCmd.java

server/src/main/java/com/cloud/server/ManagementServerImpl.java

bernardodemarco · 2025-02-04T18:33:16Z

@nicoschmdt, apparently the ManagementServerImpl.java file has conflicts. Could you resolve them, please?

github-actions · 2025-02-05T02:34:16Z

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

bernardodemarco · 2025-02-05T11:41:32Z

@blueorangutan package

blueorangutan · 2025-02-05T11:42:04Z

@bernardodemarco 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 · 2025-02-05T12:51:06Z

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

codecov · 2025-02-05T13:04:00Z

Codecov Report

Attention: Patch coverage is 5.26316% with 36 lines in your changes missing coverage. Please review.

Project coverage is 16.71%. Comparing base (7632814) to head (fb3e7de).
Report is 34 commits behind head on main.

Files with missing lines	Patch %	Lines
...nd/admin/management/RemoveManagementServerCmd.java	0.00%	18 Missing ⚠️
...in/java/com/cloud/server/ManagementServerImpl.java	0.00%	15 Missing ⚠️
...com/cloud/api/query/vo/ManagementServerJoinVO.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #10325      +/-   ##
============================================
+ Coverage     16.57%   16.71%   +0.13%     
- Complexity    13868    14144     +276     
============================================
  Files          5719     5731      +12     
  Lines        507178   517729   +10551     
  Branches      61571    66058    +4487     
============================================
+ Hits          84085    86536    +2451     
- Misses       413674   421532    +7858     
- Partials       9419     9661     +242

Flag	Coverage Δ
uitests	`4.26% <ø> (+0.29%)`	⬆️
unittests	`17.61% <5.26%> (+0.15%)`	⬆️

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.

github-actions · 2025-02-07T12:25:54Z

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

api/src/main/java/org/apache/cloudstack/api/command/admin/management/RemoveMgmtCmd.java

server/src/main/java/com/cloud/server/ManagementServerImpl.java

bernardodemarco

clgtm

server/src/main/java/com/cloud/server/ManagementServerImpl.java

JoaoJandre · 2025-02-14T11:25:42Z

@blueorangutan package

blueorangutan · 2025-02-14T11:26:04Z

@JoaoJandre 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 · 2025-02-14T12:13:34Z

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

nicoschmdt · 2025-02-17T18:18:21Z

@JoaoJandre could we run package again?

winterhazel · 2025-02-17T18:24:32Z

@nicoschmdt you do not need any permission to run package

nicoschmdt · 2025-02-17T18:58:17Z

@blueorangutan package

blueorangutan · 2025-02-17T19:00:04Z

@nicoschmdt 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 · 2025-02-17T20:06:39Z

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

.../main/java/org/apache/cloudstack/api/command/admin/management/RemoveManagementServerCmd.java

server/src/main/java/com/cloud/server/ManagementServerImpl.java

github-actions · 2025-05-28T13:35:04Z

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

Co-authored-by: Bernardo De Marco Gonçalves <[email protected]>

Co-authored-by: Fabricio Duarte <[email protected]>

…gement/RemoveManagementServerCmd.java Co-authored-by: Fabricio Duarte <[email protected]>

sureshanaparti · 2025-06-05T09:32:17Z

@blueorangutan package

DaanHoogland · 2025-06-09T08:51:19Z

@blueorangutan package

blueorangutan · 2025-06-09T08:52:04Z

@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 · 2025-06-09T10:10:55Z

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

DaanHoogland · 2025-06-11T19:04:35Z

@blueorangutan test

blueorangutan · 2025-06-11T19:06:03Z

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

blueorangutan · 2025-06-12T11:57:32Z

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

Test	Result	Time (s)	Test File

DaanHoogland · 2025-06-13T14:39:45Z

@winterhazel , is this alright by you now?

winterhazel · 2025-06-16T01:02:03Z

@winterhazel , is this alright by you now?

@DaanHoogland yup, its good for me.

winterhazel

Did some basic testing by attempting to remove an Up and a Down MS. The latter was removed, and the former was not (expected).

DaanHoogland · 2025-06-16T07:22:39Z

closing to re-kick gha

boring-cyborg bot added component:api component:management-server labels Feb 4, 2025

bernardodemarco reviewed Feb 4, 2025

View reviewed changes

bernardodemarco added this to the 4.21.0 milestone Feb 4, 2025

github-actions bot added the status:has-conflicts label Feb 5, 2025

nicoschmdt force-pushed the add-remove-ms-api-command branch from 093efee to f5e79d9 Compare February 5, 2025 11:14

github-actions bot removed the status:has-conflicts label Feb 5, 2025

github-actions bot added the status:has-conflicts label Feb 7, 2025

JoaoJandre reviewed Feb 7, 2025

View reviewed changes

nicoschmdt force-pushed the add-remove-ms-api-command branch from 329a111 to 84fe078 Compare February 10, 2025 15:31

github-actions bot removed the status:has-conflicts label Feb 10, 2025

bernardodemarco approved these changes Feb 11, 2025

View reviewed changes

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

JoaoJandre approved these changes Feb 14, 2025

View reviewed changes

winterhazel reviewed Feb 20, 2025

View reviewed changes

github-actions bot added the status:has-conflicts label Feb 24, 2025

nicoschmdt and others added 7 commits May 29, 2025 14:53

Add API command remove management server

1145c3a

Apply suggestions from code review

de5b6f5

Co-authored-by: Bernardo De Marco Gonçalves <[email protected]>

Apply sugestions from code review

380d621

Update log message with current management server state

16c4aba

Co-authored-by: Bernardo De Marco Gonçalves <[email protected]>

Apply suggestions from code review

b656688

Co-authored-by: Fabricio Duarte <[email protected]>

Update api/src/main/java/org/apache/cloudstack/api/command/admin/mana…

4d7472c

…gement/RemoveManagementServerCmd.java Co-authored-by: Fabricio Duarte <[email protected]>

Remove unused imports

fb3e7de

nicoschmdt force-pushed the add-remove-ms-api-command branch from dcdecf7 to fb3e7de Compare May 29, 2025 17:53

github-actions bot removed the status:has-conflicts label May 29, 2025

sureshanaparti added this to Apache CloudStack 4.21.0 Jun 5, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.21.0 Jun 5, 2025

sureshanaparti added the status:needs-testing label Jun 5, 2025

sureshanaparti requested review from weizhouapache and shwstppr June 5, 2025 09:32

winterhazel approved these changes Jun 16, 2025

View reviewed changes

DaanHoogland closed this Jun 16, 2025

github-project-automation bot moved this from In Progress to Done in Apache CloudStack 4.21.0 Jun 16, 2025

DaanHoogland reopened this Jun 16, 2025

DaanHoogland removed the status:needs-testing label Jun 16, 2025

Add API command remove management server #10325

Are you sure you want to change the base?

Add API command remove management server #10325

Uh oh!

Conversation

nicoschmdt commented Feb 4, 2025

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

How Has This Been Tested?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bernardodemarco commented Feb 4, 2025

Uh oh!

github-actions bot commented Feb 5, 2025

Uh oh!

bernardodemarco commented Feb 5, 2025

Uh oh!

blueorangutan commented Feb 5, 2025

Uh oh!

blueorangutan commented Feb 5, 2025

Uh oh!

codecov bot commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Feb 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bernardodemarco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoaoJandre commented Feb 14, 2025

Uh oh!

blueorangutan commented Feb 14, 2025

Uh oh!

blueorangutan commented Feb 14, 2025

Uh oh!

nicoschmdt commented Feb 17, 2025

Uh oh!

winterhazel commented Feb 17, 2025

Uh oh!

nicoschmdt commented Feb 17, 2025

Uh oh!

blueorangutan commented Feb 17, 2025

Uh oh!

blueorangutan commented Feb 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 28, 2025

Uh oh!

sureshanaparti commented Jun 5, 2025

Uh oh!

DaanHoogland commented Jun 9, 2025

Uh oh!

blueorangutan commented Jun 9, 2025

Uh oh!

blueorangutan commented Jun 9, 2025

Uh oh!

DaanHoogland commented Jun 11, 2025

Uh oh!

blueorangutan commented Jun 11, 2025

Uh oh!

blueorangutan commented Jun 12, 2025

Uh oh!

codecov bot commented Feb 5, 2025 •

edited

Loading