-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Revert "Add the option to filter by host when retrieving of unregistered VMs (#9925)" #10647
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.
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
Conversation
@blueorangutan package |
@@ -82,6 +82,49 @@ public VirtualMachineMO findVm(String vmName) throws Exception { | |||
List<ObjectContent> ocs = getVmProperties(new String[] {"name", instanceNameCustomField}); | |||
return HypervisorHostHelper.findVmFromObjectContent(_context, ocs.toArray(new ObjectContent[0]), vmName, instanceNameCustomField); | |||
} | |||
public VirtualMachineMO findVm2(String vmName) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename the findVm
method to something similar to findVmThroughPropertiesEx
and rename this findVm2
method to findVm
? This way we can redirect all the existing usages of the method to it. Then we could invoke findVmThroughPropertiesEx
to the listUnmanagedInstances execution
@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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #10647 +/- ##
==========================================
Coverage 15.16% 15.16%
- Complexity 11328 11333 +5
==========================================
Files 5415 5412 -3
Lines 474824 474997 +173
Branches 57912 57957 +45
==========================================
+ Hits 72014 72041 +27
- Misses 394757 394899 +142
- Partials 8053 8057 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 12977 |
@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]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12997 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
…red VMs (apache#9925)" This reverts commit aa6c581.
90b38d5
to
b2f24d8
Compare
@blueorangutan package |
UI build: ✔️ |
for (VmwareDatacenter vmwareDc : vmwareDcList) { | ||
VmwareDatacenterResponse vmwareDcResponse = new VmwareDatacenterResponse(); | ||
|
||
vmwareDcResponse.setId(vmwareDc.getUuid()); | ||
vmwareDcResponse.setVcenter(vmwareDc.getVcenterHost()); | ||
vmwareDcResponse.setName(vmwareDc.getVmwareDatacenterName()); | ||
vmwareDcResponse.setZoneId(getZoneId()); | ||
vmwareDcResponse.setObjectName(ApiConstants.VMWARE_DC); | ||
vmwareDcResponse.setObjectName("VMwareDC"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change needed? most of the object names used are in lower case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree @sureshanaparti , but this is a revert I can remove this part but than we need to add the code for https://github.com/apache/cloudstack/pull/10731/files
[SF] Trillian test result (tid-13015)
|
@blueorangutan test ol8 vmware-70u3 |
@vladimirpetrov a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-70u3) has been kicked to run smoke tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM based on manual testing, let's wait for the smoke test results though.
[SF] Trillian Build Failed (tid-13042) |
[SF] Trillian test result (tid-13054)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
@DaanHoogland can you update the title and description to explain which PR is being reverted and why?
@blueorangutan test ol8 vmware-70u3 |
@rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-70u3) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13084)
|
…red VMs (apache#9925)" (apache#10647) This reverts commit aa6c581.
Description
This PR...
Fixes: #10634
#9925 caused a regression where under certain circumstances (with more than 100 VMs) console proxy would not work anymore. This has to do with the APIs being used in vmware, that timeout or do not work at all.
#9782 needs to be reworked as a consequence.
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?