Fix deploy as is VM start after template deletion by harikrishna-patnala · Pull Request #8115 · apache/cloudstack

harikrishna-patnala · 2023-10-18T09:37:36Z

Description

This PR fixes the issue #7970

Fix is to keep the template deploy as is details even after the template is deleted. VM start need to know whether the vapp property is password or not which are saved in template_deploy_as_is_details table.

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

Bug Severity

How Has This Been Tested?

Registered a VMware template with "Read VM settings from OVA" enabled
Created a VM with it
Force delete the template
Stop and start the VM => succeeded

harikrishna-patnala · 2023-10-18T09:43:15Z

@blueorangutan package

blueorangutan · 2023-10-18T09:44:03Z

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

weizhouapache

code lgtm

I am fine with this change, although this will lead to some records in the database.
I think this is safest way to do, unless we can 100% figure out what details needs to be kept after template is removed.

We probably need to look into the template details as well

 // Remove template details
templateDetailsDao.removeDetails(template.getId());

DaanHoogland · 2023-10-18T11:41:14Z

I think the best way is to copy template details to the ivm_instance_details on deploy

weizhouapache · 2023-10-18T11:59:05Z

I think the best way is to copy template details to the ivm_instance_details on deploy

@DaanHoogland
it may require some code changes (e.g. read property in vm deploy-as-is details instead of template deploy-as-is details )

DaanHoogland · 2023-10-18T14:25:27Z

I think the best way is to copy template details to the ivm_instance_details on deploy
@DaanHoogland it may require some code changes (e.g. read property in vm deploy-as-is details instead of template deploy-as-is details )

yes, but that would solve the DB remnance. I am not demanding we change this, just suggesting what could be a better design choice.

harikrishna-patnala · 2023-10-19T04:30:08Z

template_deploy_as_is_details looks like this

The corresponding values in user_vm_deploy_as_is_details looks like this

So I dont think we can directly copy the template_deploy_as_is_details to user_vm_deploy_as_is_details as it is.
If we have to do that, we need a logic to differentiate the properties entries of templates and their values in the same table "user_vm_deploy_as_is_details", which I'm not sure if it is possible. May be @nvazquez can help here.

Currently we are parsing everything from user_vm_deploy_as_is_details table and passing them to the resource.
https://github.com/shapeblue/cloudstack/blob/4a0636374931e4e6b562def76acb9f8673a7f22a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java#L317-L324

To simplify all these, I thought it will be better to keep the template_deploy_as_is_details even after deletion.
Or else like I mentioned in the issue ticket, we need another column in user_vm_deploy_as_is details table which is "is_password", because thats the only field we are looking in template details table.

cc @DaanHoogland @weizhouapache

harikrishna-patnala · 2023-10-20T06:22:41Z

@blueorangutan package

blueorangutan · 2023-10-20T06:24:03Z

@harikrishna-patnala 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 · 2023-10-20T07:23:31Z

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

codecov · 2023-10-20T07:32:10Z

Codecov Report

Merging #8115 (482a80b) into 4.18 (29c7b31) will increase coverage by 0.04%.
Report is 91 commits behind head on 4.18.
The diff coverage is 18.80%.

@@ Coverage Diff @@## 4.18 #8115 +/- ##
============================================
+ Coverage 13.02% 13.06% +0.04% - Complexity 9032 9108 +76 
============================================
Files 2720 2720 Lines 257080 257547 +467 Branches 40088 40152 +64 ============================================
+ Hits 33476 33655 +179 - Misses 219400 219664 +264 - Partials 4204 4228 +24

Files	Coverage Δ
...hestration/service/VolumeOrchestrationService.java	`100.00% <ø> (ø)`
.../main/java/com/cloud/network/IpAddressManager.java	`100.00% <100.00%> (ø)`
...ava/com/cloud/network/as/AutoScaleVmProfileVO.java	`80.20% <100.00%> (+11.66%)`	⬆️
...java/com/cloud/upgrade/DatabaseUpgradeChecker.java	`40.89% <100.00%> (+0.64%)`	⬆️
...va/com/cloud/upgrade/DatabaseVersionHierarchy.java	`85.10% <100.00%> (+1.01%)`	⬆️
.../storage/vmsnapshot/StorageVMSnapshotStrategy.java	`25.39% <ø> (+0.95%)`	⬆️
.../api/command/admin/ratelimit/ResetApiLimitCmd.java	`0.00% <ø> (ø)`
...oud/hypervisor/kvm/resource/LibvirtConnection.java	`0.00% <ø> (ø)`
.../hypervisor/kvm/storage/ScaleIOStorageAdaptor.java	`10.44% <100.00%> (ø)`
...ava/com/cloud/api/commands/StopNetScalerVMCmd.java	`0.00% <ø> (ø)`
... and 62 more

... and 8 files with indirect coverage changes

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

DaanHoogland

some style critique but clgtm

DaanHoogland · 2023-10-20T14:40:20Z

@blueorangutan test

blueorangutan · 2023-10-20T14:42:03Z

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

blueorangutan · 2023-10-21T02:03:27Z

[SF] Trillian test result (tid-8040)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39447 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8115-t8040-kvm-centos7.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

shwstppr

Code LGTM

shwstppr · 2023-10-26T05:16:23Z

@harikrishna-patnala are you making any more changes for review comments?

harikrishna-patnala · 2023-10-26T06:15:30Z

@blueorangutan package

blueorangutan · 2023-10-26T06:16:03Z

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

yadvr · 2023-10-27T06:03:00Z

@blueorangutan package

blueorangutan · 2023-10-27T06:04:05Z

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

DaanHoogland · 2023-10-30T07:17:24Z

@blueorangutan test rocky8 xenserver-71

blueorangutan · 2023-10-30T07:18:03Z

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

DaanHoogland · 2023-10-30T07:21:46Z

@blueorangutan test alma9 vmware-70u3

blueorangutan · 2023-10-30T07:22:03Z

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

blueorangutan · 2023-10-30T18:55:41Z

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

Test	Result	Time (s)	Test File
test_03_ping_in_ssvm_success	`Error`	1.13	test_diagnostics.py
test_04_ping_in_ssvm_failure	`Error`	1.11	test_diagnostics.py
test_08_arping_in_ssvm	`Error`	1.13	test_diagnostics.py
test_09_arping_in_cpvm	`Error`	1.11	test_diagnostics.py
test_11_traceroute_in_ssvm	`Error`	1.11	test_diagnostics.py
test_15_retrieve_ssvm_default_files	`Error`	1.12	test_diagnostics.py
test_16_retrieve_ssvm_single_file	`Error`	1.12	test_diagnostics.py
test_01_add_primary_storage_disabled_host	`Error`	53.29	test_primary_storage.py
test_01_non_strict_host_anti_affinity	`Failure`	142.91	test_nonstrict_affinity_group.py
test_02_non_strict_host_affinity	`Error`	103.00	test_nonstrict_affinity_group.py
test_02_upgrade_kubernetes_cluster	`Failure`	579.93	test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	268.03	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	57.76	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	47.29	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	42.16	test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster	`Failure`	55.16	test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster	`Error`	55.17	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	128.26	test_kubernetes_clusters.py
test_01_sys_vm_start	`Failure`	0.09	test_secondary_storage.py
test_01_deploy_vm_on_specific_host	`Error`	0.09	test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster	`Error`	1.32	test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod	`Error`	2.33	test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster	`Error`	0.12	test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod	`Error`	2.32	test_vm_deployment_planner.py
test_08_migrate_vm	`Error`	0.07	test_vm_life_cycle.py

blueorangutan · 2023-11-08T14:34:40Z

[SF] Trillian Build Failed (tid-8277)

rajujith

LGTM. Instance starts fine with this fix although the template (vAPP) is deleted, it was failing earlier. Also observed is that the data is retained in the table template_deploy_as_is_details even after the template deletion as required by this fix. As mentioned already here this has side effects.

shwstppr · 2023-11-10T05:12:26Z

@blueorangutan package

blueorangutan · 2023-11-10T05:14:04Z

@shwstppr 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 · 2023-11-10T05:20:25Z

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

shwstppr · 2023-11-11T15:33:46Z

@blueorangutan package

blueorangutan · 2023-11-11T15:34:03Z

@shwstppr 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 · 2023-11-11T16:36:10Z

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

DaanHoogland · 2023-11-13T08:36:45Z

@blueorangutan test alma9 vmware-70u3

blueorangutan · 2023-11-13T08:44:03Z

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

weizhouapache

code lgtm

blueorangutan · 2023-11-13T22:04:45Z

[SF] Trillian test result (tid-8308)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server a9
Total time taken: 46304 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8115-t8308-vmware-70u3.zip
Smoke tests completed. 103 look OK, 6 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_add_primary_storage_disabled_host	`Error`	35.79	test_primary_storage.py
test_01_non_strict_host_anti_affinity	`Failure`	149.25	test_nonstrict_affinity_group.py
test_02_non_strict_host_affinity	`Error`	103.05	test_nonstrict_affinity_group.py
test_02_upgrade_kubernetes_cluster	`Failure`	582.53	test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	291.50	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	92.41	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	57.53	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	57.53	test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster	`Error`	3.48	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	82.69	test_kubernetes_clusters.py
test_01_sys_vm_start	`Failure`	0.09	test_secondary_storage.py
test_01_deploy_vm_on_specific_host	`Error`	0.08	test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster	`Error`	0.11	test_vm_deployment_planner.py
test_03_live_migrate_VM_with_two_data_disks	`Error`	65.96	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	0.06	test_vm_life_cycle.py

harikrishna-patnala · 2023-11-14T04:39:58Z

can this PR be merged, I see the above failures on other PRs also

DaanHoogland · 2023-11-14T08:31:37Z

can this PR be merged, I see the above failures on other PRs also

yes, we shouldn't ignore them, but I am going through them and will add any cleanup or added robustness in #8196

* 4.18: Fix deploy as is VM start after template deletion (#8115)

* 4.18: Fix deploy as is VM start after template deletion (apache#8115)

boring-cyborgBot added the component:kubernetes label Oct 18, 2023

Fix deploy as is VM start after template is force deleted
4a78277

harikrishna-patnala force-pushed the FixVMstartAfterTemplateDeletion branch from ed972a7 to 4a78277Compare October 18, 2023 09:38

harikrishna-patnala added this to the 4.18.2.0 milestone Oct 18, 2023

weizhouapache approved these changes Oct 18, 2023
View reviewed changes

shwstppr reviewed Oct 19, 2023
View reviewed changes

Comment threadserver/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java

Added VMs existance check
ebf01da

harikrishna-patnala force-pushed the FixVMstartAfterTemplateDeletion branch from 7e0ba1e to ebf01daCompare October 20, 2023 06:22

DaanHoogland approved these changes Oct 20, 2023
View reviewed changes

Comment threadserver/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java Outdated
Comment threadserver/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java Outdated

shwstppr approved these changes Oct 26, 2023
View reviewed changes

shwstppr requested a review from weizhouapache October 26, 2023 05:16

Added a method and java doc
482a80b

shwstppr assigned rajujith Nov 8, 2023

DaanHoogland linked an issue Nov 9, 2023 that may be closed by this pull request
After force deleting a deploy-as-is template, the VMs using it cannot start #7970
Closed

DaanHoogland added component:vmware Severity:Major and removed component:kubernetes labels Nov 9, 2023

rajujith approved these changes Nov 10, 2023
View reviewed changes

weizhouapache approved these changes Nov 13, 2023
View reviewed changes

DaanHoogland merged commit b7835d0 into apache:4.18Nov 14, 2023

DaanHoogland deleted the FixVMstartAfterTemplateDeletion branch November 14, 2023 08:32

DaanHoogland added a commit that referenced this pull request Nov 14, 2023
Merge release branch 4.18 to main
c7100c3
* 4.18: Fix deploy as is VM start after template deletion (#8115)

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Nov 16, 2023
Fix deploy as is VM start after template deletion (apache#8115)
2fe7575

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Nov 16, 2023
Merge release branch 4.18 to main
a517f5e
* 4.18: Fix deploy as is VM start after template deletion (apache#8115)

JoaoJandre mentioned this pull request Jan 8, 2024
After force deleting a deploy-as-is template, the VMs using it cannot start #7970
Closed

Uh oh!

Conversation

harikrishna-patnala commented Oct 18, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Bug Severity

How Has This Been Tested?

Uh oh!

harikrishna-patnala commented Oct 18, 2023

Uh oh!

blueorangutan commented Oct 18, 2023

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Oct 18, 2023

Uh oh!

weizhouapache commented Oct 18, 2023

Uh oh!

DaanHoogland commented Oct 18, 2023

Uh oh!

harikrishna-patnala commented Oct 19, 2023

Uh oh!

Uh oh!

harikrishna-patnala commented Oct 20, 2023

Uh oh!

blueorangutan commented Oct 20, 2023

Uh oh!

blueorangutan commented Oct 20, 2023

Uh oh!

codecovBot commented Oct 20, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DaanHoogland commented Oct 20, 2023

Uh oh!

blueorangutan commented Oct 20, 2023

Uh oh!

blueorangutan commented Oct 21, 2023

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

shwstppr commented Oct 26, 2023

Uh oh!

harikrishna-patnala commented Oct 26, 2023

Uh oh!

blueorangutan commented Oct 26, 2023

Uh oh!

yadvr commented Oct 27, 2023

Uh oh!

blueorangutan commented Oct 27, 2023

Uh oh!

DaanHoogland commented Oct 30, 2023

Uh oh!

blueorangutan commented Oct 30, 2023

Uh oh!

DaanHoogland commented Oct 30, 2023

Uh oh!

blueorangutan commented Oct 30, 2023

Uh oh!

blueorangutan commented Oct 30, 2023

Uh oh!

blueorangutan commented Nov 8, 2023

Uh oh!

rajujith left a comment

Choose a reason for hiding this comment

Uh oh!

shwstppr commented Nov 10, 2023

Uh oh!

blueorangutan commented Nov 10, 2023

Uh oh!

blueorangutan commented Nov 10, 2023

Uh oh!

harikrishna-patnala commented Oct 18, 2023•
edited
Loading

codecovBot commented Oct 20, 2023•
edited
Loading