Adding memoryallocatedpercentage & memoryallocatedbytes to HostsResponse & HostsForMigrationResponse by davidjumani · Pull Request #4478 · apache/cloudstack

davidjumani · 2020-11-18T10:27:08Z

Description

Adds memoryallocatedpercentage & memoryallocatedbytes to HostsResponse as well as HostsForMigrationResponse and deprecates memoryallocated

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)

How Has This Been Tested?

(localhost) 🐱 > list hosts filter=cpuallocated,memoryallocated,memoryallocatedbytes,memoryallocatedpercentage, type=Routing
{
"count": 2,
"host": [
{
"cpuallocated": "6.25%",
"memoryallocated": 2415919104,
"memoryallocatedbytes": 2415919104,
"memoryallocatedpercentage": "28.12%"
},
{
"cpuallocated": "1.56%",
"memoryallocated": 536870912,
"memoryallocatedbytes": 536870912,
"memoryallocatedpercentage": "6.25%"
}
]
}
(localhost) 🐱 > find hostsformigration virtualmachineid=e3c2c89a-26b3-427e-bc4a-34caeedf466a filter=cpuallocated,memoryallocated,memoryallocatedbytes,memoryallocatedpercentage, type=Routing
{
"count": 2,
"host": [
{
"cpuallocated": "1.56%",
"memoryallocated": "6.25%",
"memoryallocatedbytes": 536870912,
"memoryallocatedpercentage": "6.25%"
}
]
}

davidjumani · 2020-11-18T10:47:38Z

@blueorangutan package

davidjumani · 2020-11-18T11:45:22Z

@blueorangutan package

blueorangutan · 2020-11-18T11:45:52Z

@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

davidjumani · 2020-11-18T11:54:19Z

@weizhouapache Is it fine that the memory is numeric but CPU is in percentage ? Or would it be better to have new memoryallocaedpercentage & cpuallocaedpercentage fields ?

blueorangutan · 2020-11-18T12:40:56Z

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2400

DaanHoogland · 2020-11-18T12:59:59Z

@blueorangutan test

blueorangutan · 2020-11-18T13:00:52Z

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

weizhouapache · 2020-11-18T15:29:38Z

@weizhouapache Is it fine that the memory is numeric but CPU is in percentage ? Or would it be better to have new memoryallocaedpercentage & cpuallocaedpercentage fields ?

@davidjumani it would be better. but it might break backwards compatibility (hope few users will be impacted).

blueorangutan · 2020-11-18T22:03:58Z

Trillian test result (tid-3198)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30749 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4478-t3198-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

yadvr · 2020-11-18T23:11:51Z

 hostResponse.setMemWithOverprovisioning(decimalFormat.format(memWithOverprovisioning));
- hostResponse.setMemoryAllocated(decimalFormat.format((float) mem / memWithOverprovisioning * 100.0f) +"%");
+ hostResponse.setMemoryAllocated(mem);



Will this cause regression?

It will but we can introduce two new fields memoryallocaedpercentage & cpuallocaedpercentage to compensate.
This will ensure that the response for both HostResponse and HostForMigrationResponse will be the same. Sounds good @rhtyd ?

DaanHoogland · 2020-11-19T08:44:29Z

€0,02: we can add responce parameters that are alligned for both -precentages and -values and leave these alone alltogether. @weizhouapache @rhtyd @davidjumani ?

weizhouapache · 2020-11-19T08:51:30Z

€0,02: we can add responce parameters that are alligned for both -precentages and -values and leave these alone alltogether. @weizhouapache @rhtyd @davidjumani ?

@DaanHoogland good to me. and mark the cpuallocated and memoryallocated as deprecated ?

DaanHoogland · 2020-11-19T08:54:16Z

€0,02: we can add responce parameters that are alligned for both -precentages and -values and leave these alone alltogether. @weizhouapache @rhtyd @davidjumani ?
@DaanHoogland good to me. and mark the cpuallocated and memoryallocated as deprecated ?

yes

davidjumani · 2020-11-19T12:31:54Z

€0,02: we can add responce parameters that are alligned for both -precentages and -values and leave these alone alltogether. @weizhouapache @rhtyd @davidjumani ?
@DaanHoogland good to me. and mark the cpuallocated and memoryallocated as deprecated ?
yes

Sounds good! I'll make the changes

…nse & HostsForMigrationResponse

davidjumani · 2020-11-20T05:59:13Z

@DaanHoogland @weizhouapache Added memoryallocatedpercentage & memoryallocatedbytes and deprecated memoryallocated

DaanHoogland · 2020-11-20T08:17:42Z

yes 👍 don't bother for now but I'm creating a ticket do implement the same for cpu #4488

DaanHoogland · 2020-11-20T08:18:00Z

@blueorangutan package

blueorangutan · 2020-11-20T08:18:59Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-11-20T08:59:22Z

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2413

DaanHoogland · 2020-11-20T10:26:15Z

I don't think we need smoke tests for this. restarted two of the travis runs

davidjumani force-pushed the fix-host-resp branch from c88d297 to 1c938d9Compare November 18, 2020 10:30

davidjumani requested a review from DaanHoogland November 18, 2020 10:47

davidjumani added Severity:Minor type:bug labels Nov 18, 2020

davidjumani added this to the 4.14.1.0 milestone Nov 18, 2020

davidjumani requested a review from weizhouapache November 18, 2020 10:48

yadvr reviewed Nov 18, 2020
View reviewed changes

davidjumani changed the title ~~hostformigrationresponse: Setting memory allocated to long~~Adding memoryallocatedpercentage & memoryallocatedbytes to HostsResponse & HostsForMigrationResponseNov 20, 2020

Adding memoryallocatedpercentage & memoryallocatedbytes to HostsRespo…
8226351
…nse & HostsForMigrationResponse

davidjumani force-pushed the fix-host-resp branch from 1c938d9 to 8226351Compare November 20, 2020 05:56

DaanHoogland mentioned this pull request Nov 20, 2020
add cpu percentage and value #4488
Closed

yadvr approved these changes Nov 20, 2020
View reviewed changes

DaanHoogland approved these changes Nov 20, 2020
View reviewed changes

DaanHoogland mentioned this pull request Nov 20, 2020
Different format in HostResponse and HostForMigrationResponse #4460
Closed

DaanHoogland merged commit d79d242 into apache:4.14Nov 20, 2020

DaanHoogland deleted the fix-host-resp branch November 20, 2020 11:27

Uh oh!

Conversation

davidjumani commented Nov 18, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

How Has This Been Tested?

Uh oh!

davidjumani commented Nov 18, 2020

Uh oh!

davidjumani commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

davidjumani commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

DaanHoogland commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

weizhouapache commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

yadvrNov 18, 2020

Choose a reason for hiding this comment

Uh oh!

davidjumaniNov 19, 2020

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Nov 19, 2020

Uh oh!

weizhouapache commented Nov 19, 2020

Uh oh!

DaanHoogland commented Nov 19, 2020

Uh oh!

davidjumani commented Nov 19, 2020

Uh oh!

davidjumani commented Nov 20, 2020

Uh oh!

DaanHoogland commented Nov 20, 2020

Uh oh!

DaanHoogland commented Nov 20, 2020

Uh oh!

blueorangutan commented Nov 20, 2020

Uh oh!

blueorangutan commented Nov 20, 2020

Uh oh!

DaanHoogland commented Nov 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

davidjumani commented Nov 18, 2020•
edited
Loading