Load Average for KVM by DennisKonrad · Pull Request #3738 · apache/cloudstack · GitHub

Description

This adds the load average for the kvm hypervisor. The average Load already seems to be available for Xen.

In the listHosts the averageload is already present. In listHostMetrics it appears as "cpuloadaverage".

It adds the 1-min load average from /proc/loadavg to the
listHosts
listHostsMetrics
API calls.

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?

Built and tested in kvm cluster.
Tested manually via API calls

* Adding avgload for kvm * Fix coding style issue * Add getter/setter * Fix several small errors * Add override * Uncomment getAverageLoad * Override getAverageLoad() * Checkstyle bug? * Delete trailing spaces * Renaming function * Change interface to match * Rename method in GetHostStatsAnswer * Change method call name * Convert double to long * Remove trailing whitespace * Change names around * Make load visible to return it * Parse string to double * Change Long to Double * Fix getter

Code LGTM

@blueorangutan package

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

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-420

@blueorangutan package

@DennisKonrad
I have tested it , the value it looks fine.
The only problem is, I spent some time to look for the value in the api responses. it would be better to change them to same name

(local) 🐵 > list hosts filter=averageload
(local) 🐵 > list hostsmetrics filter=loadAverage

@weizhouapache I changed both of the keys to cpuloadaverage:

LGTM
manually test it on ubuntu 18.04

@blueorangutan package

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

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-423

I would start a regression suite, but...
Do we need test automation for this?
/cc @weizhouapache @GabrielBrascher @DennisKonrad

@DaanHoogland I don't think that automation tests are strictly necessary in this case. It is a simple change, and we already have code reviews and one manual tests by @weizhouapache.

I am assuming that the centos6 failing to package is not related to this PR.

@DennisKonrad Why do the Host Metrics on your screenshots show a String and not a Double?

We should return this in the API as a Double and not a String.

The screenshot where you do 'list hostmetrics'

Good point @wido. It is set as double or Double on HostStats.java, HostResponse.java, GetHostStatsAnswer.java, and HostStatsEntry.java. Additionally, at CPUStat.java the method getCpuLoadAverage() returns Double.

Therefore, it looks OK on the PR. However, list hostmetrics returns it as String; list hosts looks fine though. I will take a look around the list hostmetrics.

On a 5.0 we can normalize API values and fix issues such as % values returned as Strings instead of double.

@wido Good catch. Will change that to output as double

@GabrielBrascher centos6 is broken on a lot (most if not all) PRs. not to worry about that one.
so a package after @DennisKonrad 's next addition will be enough to carry on.

Remove some unnecessary whitespaces

@wido I think it's better now.

Looking good!

LGTM, left a minor remark. (rename should be avoided, but I personally don't hold on that as the renaming makes it consistent)

Looking good to merge.
Merging it considering that all checks have passed, we have 4 LGTMs, a couple of manual tests, and the packaging results are ok for Ubuntu and CentOS.

* Avgload (#2) * Adding avgload for kvm * Fix coding style issue * Add getter/setter * Fix several small errors * Add override * Uncomment getAverageLoad * Override getAverageLoad() * Checkstyle bug? * Delete trailing spaces * Renaming function * Change interface to match * Rename method in GetHostStatsAnswer * Change method call name * Convert double to long * Remove trailing whitespace * Change names around * Make load visible to return it * Parse string to double * Change Long to Double * Fix getter * Unify naming to cpuloadaverage * Change cpuloadaverage String to Double in listHostsMetrics Remove some unnecessary whitespaces * Add CPU_LOAD_AVERAGE to ApiConstants

DennisKonrad added component:api type:enhancement labels Dec 3, 2019

DennisKonrad requested review from weizhouapache, wido and yadvr December 3, 2019 15:29

wido reviewed Dec 3, 2019

View reviewed changes

Comment thread plugins/metrics/src/main/java/org/apache/cloudstack/response/HostMetricsResponse.java Outdated

DennisKonrad requested a review from wido December 3, 2019 16:31

GabrielBrascher approved these changes Dec 3, 2019

View reviewed changes

GabrielBrascher added this to the 4.14.0.0 milestone Dec 4, 2019

GabrielBrascher assigned DennisKonrad Dec 4, 2019

weizhouapache reviewed Dec 4, 2019

View reviewed changes

Comment thread plugins/metrics/src/main/java/org/apache/cloudstack/response/HostMetricsResponse.java Outdated

Unify naming to cpuloadaverage

083f5be

DennisKonrad requested a review from weizhouapache December 4, 2019 16:44

weizhouapache approved these changes Dec 4, 2019

View reviewed changes

svenvogel added the component:kvm label Dec 4, 2019

GabrielBrascher added the status:ready-for-merge label Dec 4, 2019

Change cpuloadaverage String to Double in listHostsMetrics

fbd0894

Remove some unnecessary whitespaces

yadvr reviewed Dec 6, 2019

View reviewed changes

Comment thread api/src/main/java/org/apache/cloudstack/api/response/HostResponse.java Outdated

yadvr approved these changes Dec 6, 2019

View reviewed changes

Add CPU_LOAD_AVERAGE to ApiConstants

e056676

GabrielBrascher merged commit 2e8c069 into apache:master Dec 17, 2019

DennisKonrad deleted the avgload-pr branch January 20, 2020 08:46

DennisKonrad restored the avgload-pr branch January 28, 2020 09:41

DaanHoogland removed the status:ready-for-merge label Dec 4, 2024

Load Average for KVM by DennisKonrad · Pull Request #3738 · apache/cloudstack · GitHub

Uh oh!

Load Average for KVM - #3738

Load Average for KVM#3738GabrielBrascher merged 4 commits intoapache:masterapache/cloudstack:masterfrom DennisKonrad:avgload-prCopy head branch name to clipboard

Conversation

DennisKonrad commented Dec 3, 2019 • 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!

Uh oh!

GabrielBrascher left a comment

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher commented Dec 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blueorangutan commented Dec 4, 2019

Uh oh!

blueorangutan commented Dec 4, 2019

Uh oh!

GabrielBrascher commented Dec 4, 2019

Uh oh!

Uh oh!

weizhouapache commented Dec 4, 2019

Uh oh!

DennisKonrad commented Dec 4, 2019

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher commented Dec 4, 2019

Uh oh!

blueorangutan commented Dec 4, 2019

Uh oh!

blueorangutan commented Dec 4, 2019

Uh oh!

DaanHoogland commented Dec 5, 2019

Uh oh!

GabrielBrascher commented Dec 5, 2019

Uh oh!

wido commented Dec 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GabrielBrascher commented Dec 5, 2019

Uh oh!

DennisKonrad commented Dec 5, 2019

Uh oh!

DaanHoogland commented Dec 5, 2019

Uh oh!

DennisKonrad commented Dec 5, 2019

Uh oh!

wido commented Dec 6, 2019

Uh oh!

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

GabrielBrascher commented Dec 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Load Average for KVM#3738
GabrielBrascher merged 4 commits into
apache:masterfrom
DennisKonrad:avgload-pr

DennisKonrad commented Dec 3, 2019 •
edited
Loading

GabrielBrascher commented Dec 4, 2019 •
edited
Loading

wido commented Dec 5, 2019 •
edited
Loading