Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

UI: Fix deploy VM wizard vApp properties#8072

Merged
shwstppr merged 1 commit intoapache:4.18apache/cloudstack:4.18from
shapeblue:fix-ui-deploy-as-is-props-deployapishapeblue/cloudstack:fix-ui-deploy-as-is-props-deployapiCopy head branch name to clipboard
Oct 25, 2023
Merged

UI: Fix deploy VM wizard vApp properties#8072
shwstppr merged 1 commit intoapache:4.18apache/cloudstack:4.18from
shapeblue:fix-ui-deploy-as-is-props-deployapishapeblue/cloudstack:fix-ui-deploy-as-is-props-deployapiCopy head branch name to clipboard

Conversation

@nvazquez
Copy link
Contributor

@nvazquez nvazquez commented Oct 11, 2023

Description

This PR fixes the vApp properties values sent as part of the deployVirtualMachine API through the UI

Fixes: #7998

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

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Tested with vApps containing properties registered as 'Read settings from OVA' templates on Vmware env

How did you try to break this feature and the system with this change?

@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #8072 (def62d4) into 4.18 (864a195) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               4.18    #8072   +/-   ##
=========================================
  Coverage     13.06%   13.06%           
  Complexity     9108     9108           
=========================================
  Files          2720     2720           
  Lines        257537   257537           
  Branches      40156    40156           
=========================================
  Hits          33658    33658           
  Misses       219649   219649           
  Partials       4230     4230

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

DaanHoogland
DaanHoogland approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Oct 11, 2023

tested ok with a vstream template. in the ui:
image

call being made:

http://localhost:5050/client/api/?zoneid=b97af5af-6dc9-408e-8e5a-d3918591ab95&dynamicscalingenabled=true&iothreadsenabled=false&templateid=d38fd9a5-f30e-42d1-b0a8-740d14aa1f4f&startvm=true&serviceofferingid=5e380a1c-708b-4f48-8aad-c93b8943f387&affinitygroupids=&nicnetworklist[0].nic=12&nicnetworklist[0].network=4430bd0a-978a-43ad-8184-73a680ef6ffc&nicnetworklist[1].nic=13&nicnetworklist[1].network=4430bd0a-978a-43ad-8184-73a680ef6ffc&keypairs=&name=v1&displayname=v1&properties[0].key=A_IPADDR&properties[0].value=10.10.10.1&properties[1].key=B_NETMASK&properties[1].value=255.255.255.0&properties[2].key=C_DEFGATEWAY&properties[2].value=10.10.10.11&properties[3].key=H_HOSTNAME&properties[3].value=vstream&properties[4].key=I_DOMAINNAME&properties[4].value=bla&properties[5].key=E_DNS1&properties[5].value=10.10.10.11&properties[6].key=F_DNS2&properties[6].value=10.10.10.22&properties[7].key=F_NTP1&properties[7].value=10.10.10.11&properties[8].key=G_NTP2&properties[8].value=10.10.10.22&properties[9].key=A_NGENIUS_SERVER_IPV4&properties[9].value=10.10.10.11&properties[10].key=dataDisk&properties[10].value=20&command=deployVirtualMachine&response=json

shwstppr
shwstppr approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

Code LGTM

@DaanHoogland DaanHoogland linked an issue Oct 11, 2023 that may be closed by this pull request
[UI] Vmware deploy-as-is instances are not receiving the properties values #7998
Closed
@DaanHoogland
Copy link
Contributor

DaanHoogland commented Oct 11, 2023

@nvazquez , do you need to add to this or can we merge?

@shwstppr shwstppr added this to the 4.18.2.0 milestone Oct 11, 2023
@nvazquez
Copy link
Contributor Author

nvazquez commented Oct 11, 2023

@DaanHoogland I think this is ready but we could double check other vApps sections as well for verification

borisstoyanov
borisstoyanov approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

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

LGTM, just ui changes no need for Marvin tests

harikrishna-patnala
harikrishna-patnala approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@harikrishna-patnala harikrishna-patnala left a comment

Choose a reason for hiding this comment

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

LGTM, tested these changes.

@DaanHoogland DaanHoogland marked this pull request as ready for review October 18, 2023 10:21
@shwstppr
Copy link
Contributor

shwstppr commented Oct 19, 2023

@blueorangutan ui

@blueorangutan
Copy link

blueorangutan commented Oct 19, 2023

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

@shwstppr shwstppr requested a review from kiranchavala October 19, 2023 11:49
@blueorangutan
Copy link

blueorangutan commented Oct 19, 2023

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/8072 (QA-JID-208)

@kiranchavala
Copy link
Contributor

kiranchavala commented Oct 23, 2023

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Oct 23, 2023

@kiranchavala 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
Copy link

blueorangutan commented Oct 23, 2023

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

kiranchavala
kiranchavala reviewed Oct 23, 2023
View reviewed changes
Copy link
Contributor

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

@nvazquez If there are no networks available, the VM deployment form does allow to create a new network

An existing isolated network should be present before launching a vm from a vapp

vapp

vapp2

vapp3

@kiranchavala kiranchavala self-assigned this Oct 23, 2023
@shwstppr
Copy link
Contributor

shwstppr commented Oct 24, 2023

Option to add a new network can be a separate improvement as it has been the same behaviour even before this PR @kiranchavala cc @nvazquez

kiranchavala
kiranchavala approved these changes Oct 25, 2023
View reviewed changes
Copy link
Contributor

@kiranchavala kiranchavala left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, tested the changes manually

Before fix

http://10.0.32.243:8080/client/api/?zoneid=6e980031-2cee-4f44-8dcb-88882d635a9a&dynamicscalingenabled=false&iothreadsenabled=false&templateid=422d4275-36d0-4c7f-8130-7bdc76db3042&startvm=true&serviceofferingid=b3bd601b-8103-4fbd-a3b8-a3f22eb60c5d&affinitygroupids=&nicnetworklist[0].nic=12&nicnetworklist[0].network=36e7ca23-5713-4bbe-aff9-e988c31ce604&nicnetworklist[1].nic=13&nicnetworklist[1].network=43bc19cf-6342-4c22-98dd-15c746105680&keypairs=&command=deployVirtualMachine&response=json

After fix

http://10.0.34.107:8080/client/api/?zoneid=b64006da-03e5-40ac-9381-2bae36357555&dynamicscalingenabled=false&iothreadsenabled=false&templateid=8bdbc792-91ba-49fc-8fd9-2234c610c286&startvm=true&serviceofferingid=3f1f2e13-7946-4e39-a30c-9c754630b789&affinitygroupids=&nicnetworklist[0].nic=12&nicnetworklist[0].network=59261057-2eee-47e5-a3a5-0bcd32ca9c87&nicnetworklist[1].nic=13&nicnetworklist[1].network=59261057-2eee-47e5-a3a5-0bcd32ca9c87&keypairs=&properties[0].key=A_IPADDR&properties[0].value=10.10.10.11&properties[1].key=B_NETMASK&properties[1].value=255.255.255.0&properties[2].key=C_DEFGATEWAY&properties[2].value=10.10.10.1&properties[3].key=H_HOSTNAME&properties[3].value=vstream&properties[4].key=I_DOMAINNAME&properties[4].value=&properties[5].key=E_DNS1&properties[5].value=8.8.8.8&properties[6].key=F_DNS2&properties[6].value=8.8.4.4&properties[7].key=F_NTP1&properties[7].value=0.0.0.0&properties[8].key=G_NTP2&properties[8].value=0.0.0.0&properties[9].key=A_NGENIUS_SERVER_IPV4&properties[9].value=0.0.0.0&properties[10].key=dataDisk&properties[10].value=100&command=deployVirtualMachine&response=json

@shwstppr shwstppr merged commit b09acb8 into apache:4.18 Oct 25, 2023
@shwstppr shwstppr mentioned this pull request Oct 25, 2023
Closed
@DaanHoogland DaanHoogland deleted the fix-ui-deploy-as-is-props-deployapi branch October 25, 2023 07:26
@nvazquez nvazquez mentioned this pull request Mar 4, 2024
Closed
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

@kiranchavala kiranchavala kiranchavala approved these changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@harikrishna-patnala harikrishna-patnala harikrishna-patnala approved these changes

@borisstoyanov borisstoyanov borisstoyanov approved these changes

@Pearl1594 Pearl1594 Awaiting requested review from Pearl1594

@vladimirpetrov vladimirpetrov Awaiting requested review from vladimirpetrov

Assignees

@kiranchavala kiranchavala

Labels

component:UI type:bug

Projects

None yet

Milestone

4.18.2.0

Development

Successfully merging this pull request may close these issues.

[UI] Vmware deploy-as-is instances are not receiving the properties values

7 participants

@nvazquez @DaanHoogland @shwstppr @blueorangutan @kiranchavala @harikrishna-patnala @borisstoyanov
Morty Proxy This is a proxified and sanitized view of the page, visit original site.