Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #6621 - Update host memory stats #6622

Merged
merged 1 commit into from Aug 12, 2022
Merged

Fixes #6621 - Update host memory stats #6622

merged 1 commit into from Aug 12, 2022

Conversation

Rubueno
Copy link
Contributor

@Rubueno Rubueno commented Aug 9, 2022 •
edited

Description

This PR ensures that when getMemStat() is called in KVM environments, the hosts actual used memory is returned, and not a static value that was retrieved when the cloudstack-agent was started.

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)

Feature/Enhancement Scale

Feature/Enhancement Scale

  • Major
  • Minor

Screenshots (if appropriate):

How Has This Been Tested?

We have applied this patch into our own environment, built the packages and installed them on the managers and hosts. We then migrated VMs between hosts and saw the host memory usage being updated, instead of remaining static until the cloudstack-agent was restarted.

@Rubueno Rubueno mentioned this pull request Aug 9, 2022
Closed
@acs-robot
Copy link
Collaborator

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan
Copy link

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6622 (SL-JID-2118)

@rohityadavcloud rohityadavcloud linked an issue Aug 9, 2022 that may be closed by this pull request
Host memory stats are not updated #6621
Closed
@rohityadavcloud
Copy link
Member

rohityadavcloud commented Aug 9, 2022 •
edited

@Rubueno can you change the base branch of this PR to 4.17 and rebase your PR branch on 4.17?

@Rubueno Rubueno force-pushed the memoryfix branch 2 times, most recently from 0df5c2d to 59ca742 Compare August 9, 2022 18:55
@acs-robot
Copy link
Collaborator

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@Rubueno
Copy link
Contributor Author

Rubueno commented Aug 9, 2022

component:networking was added by mistake

@acs-robot
Copy link
Collaborator

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan
Copy link

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@acs-robot
Copy link
Collaborator

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@Rubueno Rubueno changed the base branch from main to 4.17 August 9, 2022 18:59
@blueorangutan
Copy link

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@Rubueno
Copy link
Contributor Author

Rubueno commented Aug 9, 2022

@Rubueno can you change the base branch of this PR to 4.17 and rebase your PR branch on 4.17?

Done as requested. Please let me know if there's something missing or done incorrect.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6622 (SL-JID-2119)

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6622 (SL-JID-2120)

wido
wido approved these changes Aug 9, 2022
View reviewed changes
Copy link
Contributor

@wido wido left a comment

Choose a reason for hiding this comment

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

Change is looking good. I know this change is already running in production and fixes the problems reported.

@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2022 •
edited

Codecov Report

Merging #6622 (f12cdc6) into 4.17 (6842583) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               4.17    #6622      +/-   ##
============================================
- Coverage      5.86%    5.86%   -0.01%     
  Complexity     3918     3918              
============================================
  Files          2451     2451              
  Lines        242238   242239       +1     
  Branches      37902    37902              
============================================
  Hits          14207    14207              
- Misses       226461   226462       +1     
  Partials       1570     1570
Impacted Files Coverage Δ
...ervisor/kvm/resource/LibvirtComputingResource.java 16.00% <0.00%> (-0.01%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rohityadavcloud a 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: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3954

Fixes apache#6621 - Update host memory stats
f12cdc6 
Each time getMemStat() is called, a static value is returned. This value
should instead be refreshed to return the actual memory used.
@Rubueno
Copy link
Contributor Author

Rubueno commented Aug 9, 2022

Rebased onto a missing commit. Please restart required workflows.

@sonarcloud
Copy link

sonarcloud bot commented Aug 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rohityadavcloud a 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: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3957

@shwstppr
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

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

@shwstppr shwstppr added this to the 4.17.1.0 milestone Aug 10, 2022
@blueorangutan
Copy link

Trillian test result (tid-4670)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39656 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6622-t4670-kvm-centos7.zip
Smoke tests completed. 100 look OK, 1 have errors
Only failed tests results shown below:

Test Result Time (s) Test File
test_08_upgrade_kubernetes_ha_cluster Failure 572.22 test_kubernetes_clusters.py

@shwstppr shwstppr merged commit 696b93f into apache:4.17 Aug 12, 2022
@Rubueno Rubueno deleted the memoryfix branch August 12, 2022 11:50
neogismm pushed a commit to neogismm/cloudstack that referenced this pull request Sep 5, 2022
@Rubueno @neogismm
kvm: update host memory stats (apache#6622)
c40ff5a 
Fixes apache#6621

Each time getMemStat() is called, a static value is returned. This value
should instead be refreshed to return the actual memory used.

Co-authored-by: Ruben Bosch <ruben.bosch@cldin.eu>
shwstppr pushed a commit to shapeblue/cloudstack that referenced this pull request Mar 13, 2023
@mlsorensen
Backport host memory refresh from upstream apache#6622 (apache#229)
0b7619b 
Co-authored-by: Marcus Sorensen <mls@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shwstppr shwstppr shwstppr approved these changes

@wido wido wido approved these changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@rohityadavcloud rohityadavcloud Awaiting requested review from rohityadavcloud

Assignees
No one assigned
Labels
component:kvm
Projects
None yet
Milestone
4.17.1.0
Development

Successfully merging this pull request may close these issues.

Host memory stats are not updated
8 participants
@Rubueno @acs-robot @blueorangutan @rohityadavcloud @codecov-commenter @shwstppr @wido @DaanHoogland