| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 1.64474% with 299 lines in your changes missing coverage. Please review. @@ Coverage Diff @@
## main #11061 +/- ##
============================================
- Coverage 17.38% 17.37% -0.01%
+ Complexity 15282 15281 -1
============================================
Files 5891 5892 +1
Lines 526356 526631 +275
Branches 64270 64307 +37
============================================
- Hits 91526 91523 -3
- Misses 424488 424764 +276
- Partials 10342 10344 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
|
@harikrishna-patnala 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. |
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13855 |
Sorry, something went wrong.
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Sorry, something went wrong.
|
[SF] Trillian test result (tid-13587)
|
Sorry, something went wrong.
|
@weizhouapache @DaanHoogland @rohityadavcloud can this be merged? |
Sorry, something went wrong.
It needs 2 approvals and manual test |
Sorry, something went wrong.
|
Just FYI: this code is now used on ~500 hosts across multiple Cloudstack platforms in our production. So it at least doesn't crash :) |
Sorry, something went wrong.
ok @phsm can you fix the conflicts @phsm ? |
Sorry, something went wrong.
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
The conflicts have been fixed. Should be alright now. |
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14734 |
Sorry, something went wrong.
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Sorry, something went wrong.
|
[SF] Trillian test result (tid-14118)
|
Sorry, something went wrong.
@tanganellilore , 4.22 has a release candidate out, so I don’t think so. cc @harikrishna-patnala . |
Sorry, something went wrong.
|
So @DaanHoogland you think that is not feasable have on it? |
Sorry, something went wrong.
we can merge it when @harikrishna-patnala call the freeze over we can merge it for 4.22.1. Or if the current RC doesn’t make it for the next one. |
Sorry, something went wrong.
Hello, sorry for the late reply: had to allocate some time to build the newer Cloudstack 4.22-RC and test the Unmanage behvior. I wanted to test what actually happens during unmanage. Which means, even the smbios info says its Cloudstack: <sysinfo type='smbios'>
<system>
<entry name='manufacturer'>Apache Software Foundation</entry>
<entry name='product'>CloudStack KVM Hypervisor</entry>
<entry name='serial'>15d24df0-db8d-44c4-819c-b58897e8efa0</entry>
<entry name='uuid'>15d24df0-db8d-44c4-819c-b58897e8efa0</entry>
</system>
</sysinfo>Thus, I am unsure whether the metadata should or should not stay in the unmanaged instance. So far the Unmanage functionality looks like "completely forget the VM and leave it be in the libvirt". |
Sorry, something went wrong.
yes, so do you want to give it the extra attention to remove the metadata @phsm ? |
Sorry, something went wrong.
|
It was not clear if the CS maintainers wanted me to actually clean the <metadata> after the VM instance is unmanaged. I think it is better not to touch the <sysinfo type='smbios'> part of the XML tree (this part is not something that this PR adds), as this part is actually visible from the inside of the VM. |
Sorry, something went wrong.
@DaanHoogland @phsm |
Sorry, something went wrong.
ok, in this cas we are ready to merge? |
Sorry, something went wrong.
I guess this feature is designed to export VM metadata from Apache CloudStack to a monitoring system. However, we must address a data integrity concern: when a VM is unmanaged in CloudStack, its CloudStack-specific metadata becomes obsolete. To ensure the monitoring environment receives accurate data, this feature should automatically revoke exported metadata for unmanaged VMs. |
Sorry, something went wrong.
|
Depends how the "Unmanage" VM action is interpreted by CS authors, and what the users expect it to do: It is either
In your example the user can account for this, and clean up the metadata (and may be something else) by some post-unmanage scripts. Or maybe the user doesn't mind having the information of the VM past in its metadata. |
Sorry, something went wrong.
@phsm If we adopt this approach, we must at a minimum document this behavior clearly in the release notes to set user expectations. |
Sorry, something went wrong.
|
My personal opinion is that the Cloudstack shall not meddle with domainXML during unmanaging. However, I will respect any decision from the maintainers. If the decision is to not edit the domainXML when unmanaging, then the PR is ready to merge. If the decision is to edit domainXML, then I can work on it. |
Sorry, something went wrong.
Personnally I think we can merge and work on an optional clearing the metadata later. I do not think a VM is no longer created by ACS once it is unmanaged so removing that would even be a bit lying ;) no biggy anyway but that’s up to you @pavanaravapalli … merging |
Sorry, something went wrong.
@DaanHoogland |
Sorry, something went wrong.
would you create a PR for the next version of the release notes, @pavanaravapalli ? |
Sorry, something went wrong.
Thanks! I'd be happy to create the PR. To ensure I get it right, would you be open to pairing on it? I'm ready to work with your guidance since this is my first time with ACS release notes. |
Sorry, something went wrong.
open a new PR here: https://github.com/apache/cloudstack-documentation/compare I will review as will some others I presume #famouslastwords |
Sorry, something went wrong.
@DaanHoogland |
Sorry, something went wrong.
* main: (1223 commits) Standardize and auto add license headers for SQL files with pre-commit (apache#12071) pre-commit use colored text in the CI for `pass / fail / skipped` (apache#11977) ui(locales): remove duplicates and fix typos (apache#11872) pre-commit: auto add table of contents with `doctoc` (apache#11679) chore: rename workflow `linter.yml` to `pre-commit.yml` (apache#11647) engine-schema: upgrade path for 4.23.0 (apache#12048) Fixes:apache#7837: Add isolationMethods and vlan to TrafficTypeResponse (apache#8151) Svgs (apache#12051) Update GUI Kubernetes logo (apache#11895) Keeping consistency with other error messages. (apache#11649) enhancement: add instance info as Libvirt metadata (apache#11061) Add empty Provider value in Network/VPC Offering form (apache#11982) merge fix Updating pom.xml version numbers for release 4.23.0.0-SNAPSHOT Updating pom.xml version numbers for release 4.22.1.0-SNAPSHOT UI: fix typo Upload SSL certificate (apache#11869) api/test: fix storage pool update with only id (apache#11897) Updating pom.xml version numbers for release 4.22.0.0 Handle null mountTimeout in RestoreBackupCommand (apache#11944) Fix the config 'powerflex.connect.on.demand' description (apache#11926) ...
| Back | FazBrowse Home | New Git URL |
Description
This PR adds <metadata> section to the domain XML of the virtual machine.
It is useful when the additional instance info is needed from within the compute host,
for example to use it with a monitoring exporter that would label the virtual machine with the additional information.
The method that generates the metadata object is defined in the HypervisorGuruBase class as protected, so it can be used in all its children classes. Currently I've only used it in the KVMGuru because I do not have any other hypervisor kind to test this on.
Additionally, some dead code that looks to be half-finished, was removed, namely
LibvirtVMDef.java: public class MetadataDef and the references to it. Kindly check this part, perhaps I accidentally remove something that was obscurely used by something.
Fixes: #6695
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
The metadata has the following format:
How Has This Been Tested?
I specifically put an emphasis on compatibility with the previous versions, so having a mgmt server with this feature/agent without this feature and vice versa will not crash the VM startup.
These are the tests that I preformed:
Both the Mgmt servers and the Nodes contain this patch:
Mgmt server contains the patch, the node does not contain the patch:
Mgmt server does not contain the patch, the host contains the patch:
How did you try to break this feature and the system with this change?
I specifically checked for the empty values in my tests: service offering without tags, no resource tags, no project etc as those look to be the most dangerous parts.
Plus, the code checks for null value everywhere, and uses "unknown" default value when null is occurred.