Fix KVM cloudstack-agent start if there is vm not managed by cloudstack on the host#8049
Conversation
|
@blueorangutan package |
|
@harikrishna-patnala a [SF] 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. |
f7e7661 to
788e08b
Compare
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7252 |
|
@blueorangutan package |
|
@harikrishna-patnala a [SF] 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. |
Codecov Report
@@ Coverage Diff @@
## 4.18 #8049 +/- ##
=========================================
Coverage 13.07% 13.07%
- Complexity 9109 9110 +1
=========================================
Files 2720 2720
Lines 257530 257535 +5
Branches 40154 40156 +2
=========================================
+ Hits 33660 33668 +8
+ Misses 219639 219633 -6
- Partials 4231 4234 +3
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7253 |
| currentVmBalloonStatsPeriod, vmId, dm.getName())); | ||
| } catch (final LibvirtException e) { | ||
| s_logger.warn("Failed to set up memory balloon stats period." + e.getMessage()); | ||
| } catch (final Exception e) { |
There was a problem hiding this comment.
Is this needed or can we add a specific Exception to the catch clause?
There was a problem hiding this comment.
@DaanHoogland we are trying to read many other parameters in that parser. so there are chances of other errors such as NPEs, cast errors. While discussing with Wei several other found in IDE, so thought a common exception could catch it and warn the message.

There was a problem hiding this comment.
ok, I hate it and I would like to see a message per exception, but no -1 for that
There was a problem hiding this comment.
:) So you are giving +1 on this right ;)
|
Should we consider VMs not managed by ACS in Agent start up? Would not it be easier to filter out those VMs? |
@GutoVeronezi |
I see, thanks @weizhouapache. |
|
@blueorangutan package |
|
@weizhouapache a [SF] 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. |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7329 |
|
@DaanHoogland do you want me to improve this code to handle specific exceptions ? |
I would prefer that but am not going to -1 this PR because of it. |
|
@DaanHoogland @harikrishna-patnala |
ok, I would like to see that justification near the catch as to not encourage this kind of coding too much. |
|
@blueorangutan package |
|
@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. |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7545 |
|
@blueorangutan LLtest basicZone |
|
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@blueorangutan LLtest securityGroups |
|
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@blueorangutan test rocky8 kvm-rocky8 |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests |
|
[LL]Trillian test result (tid-6872)
|
|
[SF] Trillian test result (tid-8138)
|
|
[LL]Trillian test result (tid-6873)
|
|
smoke tests for security groups and basic zone are expected to have such a low pass rate. |
rajujith
left a comment
There was a problem hiding this comment.
LGTM. Observed a similar issue parsing domain XML for a VM created outside Cloudstack with 4.18.1, and confirmed this PR fixes the issue.
2023-11-03 06:43:58,568 ERROR [cloud.agent.AgentShell] (main:null) (logid:) Unable to start agent:
javax.naming.ConfigurationException: Error while creating Agent with class [com.cloud.hypervisor.kvm.resource.LibvirtComputingResource]. [Root exception is java.lang.IllegalArgumentException: No enum constant com.cloud.hypervisor.kvm.resource.LibvirtVMDef.ChannelDef.ChannelType.SPICEVMC]
at com.cloud.agent.AgentShell.launchAgentFromClassInfo(AgentShell.java:434)
at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:415)
at com.cloud.agent.AgentShell.start(AgentShell.java:511)
at com.cloud.agent.AgentShell.main(AgentShell.java:541)
Caused by: java.lang.IllegalArgumentException: No enum constant com.cloud.hypervisor.kvm.resource.LibvirtVMDef.ChannelDef.ChannelType.SPICEVMC
at java.base/java.lang.Enum.valueOf(Enum.java:240)
at com.cloud.hypervisor.kvm.resource.LibvirtVMDef$ChannelDef$ChannelType.valueOf(LibvirtVMDef.java:1768)
at com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser.parseDomainXML(LibvirtDomainXMLParser.java:273)
at com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.setupMemoryBalloonStatsPeriod(LibvirtComputingResource.java:1301)
at com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.configure(LibvirtComputingResource.java:1244)
at com.cloud.agent.Agent.<init>(Agent.java:190)
at com.cloud.agent.AgentShell.launchNewAgent(AgentShell.java:452)
at com.cloud.agent.AgentShell.launchAgentFromClassInfo(AgentShell.java:431)
... 3 more
… period for KVM VMs (apache#8049)" This reverts commit f0482e5.
…on stats period for KVM VMs (apache#8049)"" This reverts commit bb2ebbf.
… period for KVM VMs (apache#8049)" This reverts commit f0482e5.
Description
This PR fixes the issue #8040
we are now handling any exception while setting up the memory balloon stats period
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?