Skip to content

Conversation

@GabrielBrascher
Copy link
Member

Description

PR #4698 was merged at the master (4.16) but it fixes an issue that is also relevant for current LTS versions.

Considering that the port is relatively straight-forward and it can benefit releases 4.14.2.0 and 4.15.1.0, this PR proposes back-porting PR #4698 into 4.14 and later forward it to 4.15.

More details at #4698.

In the execution flow of the API migrateVirtualMachineWithVolume, there is a step that copies the volume's template to the target storage, if it is needed.
In some cases when the volume is a data type, it (the volume) does not have a template; therefore, it is going to generate an NPE when it tries to get the template's ID (Long to long auto conversion).

This PR intends to validate if the volume being copied has a template, before trying to copy the template.

@GabrielBrascher GabrielBrascher added this to the 4.14.2.0 milestone Mar 9, 2021
@GabrielBrascher GabrielBrascher changed the base branch from master to 4.14 March 9, 2021 12:25
Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

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

lgtm.

why did not create pr for 4.14 instead of 4.16?

@GabrielBrascher
Copy link
Member Author

@weizhouapache thanks for the review!

It was the contributor option sending the changes to the master. I have nothing against their decision. I understand and respect their point of view.

Personally, I see the benefit of porting it into 4.14.2 and 4.15.1 and thus proposed this PR.

Here is the discussion about it with the contributor point of view.

Copy link
Contributor

@RodrigoDLopez RodrigoDLopez left a comment

Choose a reason for hiding this comment

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

LGTM

@weizhouapache
Copy link
Member

@weizhouapache thanks for the review!

It was the contributor option sending the changes to the master. I have nothing against their decision. I understand and respect their point of view.

Personally, I see the benefit of porting it into 4.14.2 and 4.15.1 and thus proposed this PR.

Here is the discussion about it with the contributor point of view.

@GabrielBrascher thanks for your explanation.
sounds good to me.

@yadvr
Copy link
Member

yadvr commented Mar 12, 2021

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. [S]

@blueorangutan
Copy link

Packaging result: ✔️ centos7 ✖️ centos8 ✔️ debian. SL-JID 97

@shwstppr
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests [S]

@GabrielBrascher
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔️ centos7 ✔️ debian. SL-JID 300

@shwstppr
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

Trillian test result (tid-319)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45382 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4775-t319-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 78 look OK, 5 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
ContextSuite context=TestIsolatedNetworksPasswdServer>:setup Error 0.00 test_password_server.py
ContextSuite context=TestKubernetesCluster>:teardown Error 75.76 test_kubernetes_clusters.py
ContextSuite context=TestPortForwardingRules>:setup Error 0.00 test_portforwardingrules.py
test_01_migrate_VM_and_root_volume Error 61.08 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 51.16 test_vm_life_cycle.py
test_05_rvpc_multi_tiers Failure 377.07 test_vpc_redundant.py
test_05_rvpc_multi_tiers Error 402.96 test_vpc_redundant.py

@yadvr
Copy link
Member

yadvr commented Apr 5, 2021

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@apache apache deleted a comment from blueorangutan Apr 5, 2021
@blueorangutan
Copy link

Packaging result: ✔️ centos7 ✔️ debian. SL-JID 324

@yadvr
Copy link
Member

yadvr commented Apr 5, 2021

@blueorangutan test

@blueorangutan
Copy link

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

Trillian test result (tid-351)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33876 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4775-t351-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 82 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_01_migrate_VM_and_root_volume Error 76.30 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 53.09 test_vm_life_cycle.py

@yadvr yadvr merged commit cb91a76 into apache:4.14 Apr 6, 2021
nlgordon pushed a commit to ippathways/cloudstack that referenced this pull request Aug 2, 2022
Cherry-pick commit 59fba49 and fix conflict.

Co-authored-by: Daniel Augusto Veronezi Salvador <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants