Fix XenServer nested extra configuration by gp-santos · Pull Request #8698 · apache/cloudstack

gp-santos · 2024-02-23T13:38:04Z

Description

This PR fixes a problem when trying to boot up a XenServer VM with a template through an ISO.

Steps to reproduce:

Enable enable.additional.vm.configuration and add HVM-boot-params:order to allow.additional.vm.configuration.list.xen.
Create a XenServer VM with a template.
Attach an ISO to it.
Call updateVirtualMachine with parameter extraconfig=HVM-boot-params:order%3Ddcn
Start up the VM.

It will not finish, with the following log message:

INFO  [c.c.h.x.r.XenServer650Resource] (DirectAgent-71:[ctx-6c7bfe3d]) (logid:faabbafa) Appending user extra configuration settings to VM
ERROR [o.a.c.h.x.ExtraConfigurationUtility] (DirectAgent-71:[ctx-6c7bfe3d]) (logid:faabbafa) Exception caught while setting VM configuration. exception: null

After fixing the exception message, we get:

ERROR [o.a.c.h.x.ExtraConfigurationUtility] (DirectAgent-25:ctx-0d31d123) (logid:aa40d07d) Exception caught while setting VM configuration: [You tried to add a key-value pair to a map, but that key is already there.]

This PR fixes the exception message and changes the Xen API method called from addTo to set with get, letting Java substitute any duplicates in the HashMap.

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)
build/CI

Feature/Enhancement Scale or Bug Severity

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

I repeated the steps to reproduce the error and verified the VM with template started up sucessfully, booting the ISO.

DaanHoogland

CLGTM, but don't you think the replacement code should be called inside the VirtualMachine.add<>() methods, @gpordeus ?

codecov · 2024-02-23T14:21:23Z

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 30.98%. Comparing base (8f6721e) to head (df0cd8e).
Report is 23 commits behind head on main.

Files	Patch %	Lines
...ypervisor/xenserver/ExtraConfigurationUtility.java	0.00%	10 Missing ⚠️
...ervisor/xenserver/resource/CitrixResourceBase.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8698      +/-   ##
============================================
+ Coverage     30.90%   30.98%   +0.08%     
- Complexity    33382    33501     +119     
============================================
  Files          5355     5355              
  Lines        375727   375784      +57     
  Branches      54915    54914       -1     
============================================
+ Hits         116109   116444     +335     
+ Misses       244219   243903     -316     
- Partials      15399    15437      +38

Flag	Coverage Δ
simulator-marvin-tests	`24.85% <0.00%> (+0.09%)`	⬆️
uitests	`4.36% <ø> (+<0.01%)`	⬆️
unit-tests	`16.58% <0.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Co-authored-by: dahn <daan.hoogland@gmail.com>

gp-santos · 2024-02-23T16:09:22Z

@DaanHoogland Thanks for the review.

CLGTM, but don't you think the replacement code should be called inside the VirtualMachine.add<>() methods, @gpordeus ?

Do you mean the original methods (VM.addToVCPUsParams(), VM.addToHVMBootParams(), etc)? It would be better, but those are part of the xenapi :/

DaanHoogland · 2024-02-23T16:23:52Z

@DaanHoogland Thanks for the review.

CLGTM, but don't you think the replacement code should be called inside the VirtualMachine.add<>() methods, @gpordeus ?

Do you mean the original methods (VM.addToVCPUsParams(), VM.addToHVMBootParams(), etc)? It would be better, but those are part of the xenapi :/

right, I thought these were in a wrapper :( ok, don't bother

DaanHoogland · 2024-02-23T16:24:45Z

@blueorangutan package

blueorangutan · 2024-02-23T16:26:03Z

@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.

blueorangutan · 2024-02-23T16:38:24Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 8761

vishesh92 · 2024-02-26T04:41:57Z

@gpordeus Unit tests are failing during build. Can you check & fix those?

gp-santos · 2024-02-26T20:09:11Z

@gpordeus Unit tests are failing during build. Can you check & fix those?

@DaanHoogland @vishesh92
It seems the test failed because dl.openvm.eu was down at the time. Could you run them again?

[INFO] Installing /home/runner/work/cloudstack/cloudstack/framework/quota/pom.xml to /home/runner/.m2/repository/org/apache/cloudstack/cloud-framework-quota/4.20.0.0-SNAPSHOT/cloud-framework-quota-4.20.0.0-SNAPSHOT.pom
16:29:16.334 [main] ERROR org.apache.cloudstack.direct.download.HttpDirectTemplateDownloader - Cannot reach URL: http://dl.openvm.eu/cloudstack/macchinina/x86_64/macchinina-kvm.qcow2.bz2 due to: dl.openvm.eu
java.net.UnknownHostException: dl.openvm.eu
	at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:229) ~[?:?]
	at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392) ~[?:?]
	at java.net.Socket.connect(Socket.java:609) ~[?:?]
	at java.net.Socket.connect(Socket.java:558) ~[?:?]
	at java.net.Socket.<init>(Socket.java:454) ~[?:?]
	at java.net.Socket.<init>(Socket.java:306) ~[?:?]
	at org.apache.commons.httpclient.protocol.DefaultProtocolSocketFactory.createSocket(DefaultProtocolSocketFactory.java:80) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.protocol.ControllerThreadSocketFactory$1.doit(ControllerThreadSocketFactory.java:91) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.protocol.ControllerThreadSocketFactory$SocketTask.run(ControllerThreadSocketFactory.java:158) ~[commons-httpclient-3.1.jar:?]
	at java.lang.Thread.run(Thread.java:829) ~[?:?]
Error:  Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 10.965 s <<< FAILURE! - in org.apache.cloudstack.direct.download.MetalinkDirectTemplateDownloaderTest
Error:  testCheckUrlMetalink(org.apache.cloudstack.direct.download.MetalinkDirectTemplateDownloaderTest)  Time elapsed: 10.964 s  <<< FAILURE!
java.lang.AssertionError
	at org.apache.cloudstack.direct.download.MetalinkDirectTemplateDownloaderTest.testCheckUrlMetalink(MetalinkDirectTemplateDownloaderTest.java:34)

weizhouapache · 2024-02-26T20:14:41Z

@gpordeus Unit tests are failing during build. Can you check & fix those?

@DaanHoogland @vishesh92
It seems the test failed because dl.openvm.eu was down at the time. Could you run them again?

[INFO] Installing /home/runner/work/cloudstack/cloudstack/framework/quota/pom.xml to /home/runner/.m2/repository/org/apache/cloudstack/cloud-framework-quota/4.20.0.0-SNAPSHOT/cloud-framework-quota-4.20.0.0-SNAPSHOT.pom
16:29:16.334 [main] ERROR org.apache.cloudstack.direct.download.HttpDirectTemplateDownloader - Cannot reach URL: http://dl.openvm.eu/cloudstack/macchinina/x86_64/macchinina-kvm.qcow2.bz2 due to: dl.openvm.eu
java.net.UnknownHostException: dl.openvm.eu
	at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:229) ~[?:?]
	at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392) ~[?:?]
	at java.net.Socket.connect(Socket.java:609) ~[?:?]
	at java.net.Socket.connect(Socket.java:558) ~[?:?]
	at java.net.Socket.<init>(Socket.java:454) ~[?:?]
	at java.net.Socket.<init>(Socket.java:306) ~[?:?]
	at org.apache.commons.httpclient.protocol.DefaultProtocolSocketFactory.createSocket(DefaultProtocolSocketFactory.java:80) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.protocol.ControllerThreadSocketFactory$1.doit(ControllerThreadSocketFactory.java:91) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.protocol.ControllerThreadSocketFactory$SocketTask.run(ControllerThreadSocketFactory.java:158) ~[commons-httpclient-3.1.jar:?]
	at java.lang.Thread.run(Thread.java:829) ~[?:?]
Error:  Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 10.965 s <<< FAILURE! - in org.apache.cloudstack.direct.download.MetalinkDirectTemplateDownloaderTest
Error:  testCheckUrlMetalink(org.apache.cloudstack.direct.download.MetalinkDirectTemplateDownloaderTest)  Time elapsed: 10.964 s  <<< FAILURE!
java.lang.AssertionError
	at org.apache.cloudstack.direct.download.MetalinkDirectTemplateDownloaderTest.testCheckUrlMetalink(MetalinkDirectTemplateDownloaderTest.java:34)

The server is not down.
But the domain name can not be resolved by some DNS servers (e.g. 8.8.8.8)
Please try 1.1.1.1

@DaanHoogland @NuxRo
We probably can move templates to download.cloudstack.org

DaanHoogland · 2024-02-26T20:15:20Z

@blueorangutan package

blueorangutan · 2024-02-26T20:16:04Z

@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.

blueorangutan · 2024-02-26T20:43:41Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 8795

…or/xenserver/resource/CitrixResourceBase.java

DaanHoogland · 2024-02-28T07:41:01Z

@blueorangutan package

blueorangutan · 2024-02-28T07:42:03Z

@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.

blueorangutan · 2024-02-28T08:54:19Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8804

DaanHoogland · 2024-02-28T09:24:39Z

@blueorangutan test alma9 xcpng82

blueorangutan · 2024-02-28T09:26:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + xcpng82) has been kicked to run smoke tests

blueorangutan · 2024-02-29T05:02:57Z

[SF] Trillian test result (tid-9338)
Environment: xcpng82 (x2), Advanced Networking with Mgmt server a9
Total time taken: 68936 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8698-t9338-xcpng82.zip
Smoke tests completed. 125 look OK, 4 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_condensed_drs_algorithm	`Failure`	170.44	test_cluster_drs.py
test_02_balanced_drs_algorithm	`Failure`	180.24	test_cluster_drs.py
test_01_non_strict_host_anti_affinity	`Error`	207.99	test_nonstrict_affinity_group.py
test_02_non_strict_host_affinity	`Error`	114.43	test_nonstrict_affinity_group.py
test_deploy_vm_with_registered_userdata	`Error`	829.04	test_register_userdata.py
test_deploy_vm_with_registered_userdata_with_override_policy_allow	`Error`	859.04	test_register_userdata.py
test_deploy_vm_with_registered_userdata_with_override_policy_append	`Error`	826.68	test_register_userdata.py
test_deploy_vm_with_registered_userdata_with_params	`Error`	840.29	test_register_userdata.py
test_01_deploy_vm_on_specific_host	`Error`	0.09	test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster	`Error`	0.09	test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod	`Error`	0.11	test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster	`Error`	0.15	test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod	`Error`	0.10	test_vm_deployment_planner.py

DaanHoogland · 2024-03-04T08:48:46Z

alma9 xcpng82

@gpordeus the test run with these env specs fails a bit, is that expected? what did you use?

gp-santos · 2024-03-12T16:33:55Z

@gpordeus the test run with these env specs fails a bit, is that expected? what did you use?

I tested it manually on a local ubuntu lab with xcp-ng-8.2 host. I didn't run the integration tests, but I believe these are related to #8586, since the same tests are acting up (cluster_drs, nonstrict_affinity_group, register_userdata and vm_deployment_planner).

DaanHoogland · 2024-03-15T14:32:43Z

@gpordeus the test run with these env specs fails a bit, is that expected? what did you use?

I tested it manually on a local ubuntu lab with xcp-ng-8.2 host. I didn't run the integration tests, but I believe these are related to #8586, since the same tests are acting up (cluster_drs, nonstrict_affinity_group, register_userdata and vm_deployment_planner).

@weizhouapache , can you review?
@gpordeus , can you run the integration tests to see?

weizhouapache

code lgtm

cc @DaanHoogland

weizhouapache · 2024-03-15T14:40:48Z

@blueorangutan test rocky8 xenserver-71

blueorangutan · 2024-03-15T14:42:03Z

@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + xenserver-71) has been kicked to run smoke tests

blueorangutan · 2024-03-16T06:31:12Z

[SF] Trillian test result (tid-9505)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server r8
Total time taken: 44292 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8698-t9505-xenserver-71.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland

clgtm

DaanHoogland · 2024-03-21T12:14:30Z

@vishesh92 is this alright to you?

vishesh92

clgtm. didn't test.

fix xen nested extraconfig and related exception message

817ebb6

boring-cyborg Bot added the component:XenServer label Feb 23, 2024

DaanHoogland reviewed Feb 23, 2024

View reviewed changes

Apply suggestions from code review

3f5b434

Co-authored-by: dahn <daan.hoogland@gmail.com>

DaanHoogland closed this Feb 26, 2024

DaanHoogland reopened this Feb 26, 2024

vishesh92 reviewed Feb 26, 2024

View reviewed changes

Comment thread ...sors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java Outdated

Update plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervis…

df0cd8e

…or/xenserver/resource/CitrixResourceBase.java

weizhouapache approved these changes Mar 15, 2024

View reviewed changes

DaanHoogland approved these changes Mar 21, 2024

View reviewed changes

vishesh92 approved these changes Mar 21, 2024

View reviewed changes

DaanHoogland merged commit adc05f8 into apache:main Mar 25, 2024

gp-santos deleted the main-fix-xenserver-nested-extraconfigs branch April 5, 2024 15:52

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Apr 9, 2024

Fix XenServer nested extra configuration (apache#8698)

1e3b92e

DaanHoogland added this to the 4.20.0.0 milestone May 7, 2024

Conversation

gp-santos commented Feb 23, 2024

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Feb 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gp-santos commented Feb 23, 2024

Uh oh!

DaanHoogland commented Feb 23, 2024

Uh oh!

DaanHoogland commented Feb 23, 2024

Uh oh!

blueorangutan commented Feb 23, 2024

Uh oh!

blueorangutan commented Feb 23, 2024

Uh oh!

vishesh92 commented Feb 26, 2024

Uh oh!

gp-santos commented Feb 26, 2024

Uh oh!

weizhouapache commented Feb 26, 2024

Uh oh!

DaanHoogland commented Feb 26, 2024

Uh oh!

blueorangutan commented Feb 26, 2024

Uh oh!

blueorangutan commented Feb 26, 2024

Uh oh!

Uh oh!

DaanHoogland commented Feb 28, 2024

Uh oh!

blueorangutan commented Feb 28, 2024

Uh oh!

blueorangutan commented Feb 28, 2024

Uh oh!

DaanHoogland commented Feb 28, 2024

Uh oh!

blueorangutan commented Feb 28, 2024

Uh oh!

blueorangutan commented Feb 29, 2024

Uh oh!

DaanHoogland commented Mar 4, 2024

Uh oh!

gp-santos commented Mar 12, 2024

Uh oh!

DaanHoogland commented Mar 15, 2024

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Mar 15, 2024

Uh oh!

blueorangutan commented Mar 15, 2024

Uh oh!

blueorangutan commented Mar 16, 2024

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Mar 21, 2024

Uh oh!

vishesh92 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

codecov Bot commented Feb 23, 2024 •

edited

Loading