Default value of force should be false for template delete operation by harikrishna-patnala · Pull Request #7731 · apache/cloudstack

harikrishna-patnala · 2023-07-07T07:09:16Z

Description

Default value of force should be false for the template delete operation

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)

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

harikrishna-patnala · 2023-07-07T07:14:36Z

@blueorangutan package

blueorangutan · 2023-07-07T07:16: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.

blueorangutan · 2023-07-07T08:07:54Z

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

codecov · 2023-07-07T08:14:22Z

Codecov Report

Merging #7731 (7b3cc99) into 4.18 (29c7b31) will increase coverage by 0.00%.
Report is 68 commits behind head on 4.18.
The diff coverage is 15.37%.

@@ Coverage Diff @@## 4.18 #7731 +/- ##
===========================================
Coverage 13.02% 13.02% - Complexity 9032 9222 +190 
===========================================
Files 2720 2720 Lines 257080 262616 +5536 Branches 40088 42312 +2224 ===========================================
+ Hits 33476 34211 +735 - Misses 219400 224114 +4714 - Partials 4204 4291 +87

Files	Coverage Δ
.../main/java/com/cloud/network/IpAddressManager.java	`100.00% <100.00%> (ø)`
...ava/com/cloud/network/as/AutoScaleVmProfileVO.java	`80.20% <100.00%> (+11.66%)`	⬆️
.../api/command/admin/ratelimit/ResetApiLimitCmd.java	`0.00% <ø> (ø)`
...ava/com/cloud/api/commands/StopNetScalerVMCmd.java	`0.00% <ø> (ø)`
...tungsten/api/command/ListTungstenFabricTagCmd.java	`0.00% <ø> (ø)`
...e/datastore/util/ListElastistorVolumeResponse.java	`0.00% <ø> (ø)`
...dstack/api/response/LdapConfigurationResponse.java	`0.00% <ø> (ø)`
...dstack/api/response/LinkAccountToLdapResponse.java	`93.33% <ø> (ø)`
...udstack/api/response/LinkDomainToLdapResponse.java	`85.71% <ø> (ø)`
...loud/api/auth/DefaultLoginAPIAuthenticatorCmd.java	`0.00% <ø> (ø)`
... and 37 more

... and 25 files with indirect coverage changes

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

DaanHoogland

clgtm

yadvr · 2023-07-07T17:29:56Z

@blueorangutan test

blueorangutan · 2023-07-07T17:32:04Z

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

blueorangutan · 2023-07-08T05:59:26Z

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

Test	Result	Time (s)	Test File
test_03_deploy_vm_wrong_checksum	`Error`	40.69	test_templates.py
test_09_list_templates_download_details	`Failure`	0.05	test_templates.py
test_01_migrate_VM_and_root_volume	`Error`	79.89	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	52.63	test_vm_life_cycle.py

DaanHoogland · 2023-07-10T07:50:18Z

@harikrishna-patnala can you add some testing information to the description?

yadvr · 2023-07-10T18:04:44Z

@blueorangutan package

blueorangutan · 2023-07-10T18:06:03Z

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

blueorangutan · 2023-07-10T18:57:56Z

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

DaanHoogland · 2023-07-11T05:59:12Z

@harikrishna-patnala can you have a look if it does make sense for the test_templates tests to now fail?

harikrishna-patnala · 2023-07-14T06:11:26Z

yes @DaanHoogland it seems to be related.
'Unable to delete template with id: 267 because VM instances: [VM instance {"id":205,"instanceName":"i-119-205-VM","type":"User","uuid":"810e3e54-aeb1-444d-bb9b-9b224df2bd08"}] are using it.'

I'll fix the tests too

weizhouapache · 2023-07-21T06:15:35Z

this parameter was introduced in PR #1773

changing the default value to false makes sense. However, it will break backwards compatibility.
once this is merged, we need to add a note to to release note (e.g. in Breaking changes section)

shwstppr

Code LGTM

weizhouapache · 2023-07-25T03:00:28Z

a similar issue:
due to a new flag introduced by #4617,
ansible needs to be updated: ngine-io/ansible-collection-cloudstack#122

kiranchavala

LGTM , tested manually with the api call

Also tested with project

Before the fix

2023-08-02 10:25:41,346 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (qtp1239807799-18:ctx-6c52ee9f ctx-71da430f ctx-dbb08cbb) (logid:2828153a) submit async job-113, details: AsyncJobVO: {id:113, userId: 2, accountId: 2, instanceType: Template, instanceId: 215, cmd: org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd, cmdInfo: {"apiKey":"LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q","signature":"YiZXnlLsEqPiSiiMrEBLTiSJAlo\u003d","response":"json","ctxUserId":"2","httpmethod":"GET","ctxStartEventId":"267","id":"44bb4d22-c43c-4ce5-a9c0-fcf61aeb1775","ctxDetails":"{\"interface com.cloud.template.VirtualMachineTemplate\":\"44bb4d22-c43c-4ce5-a9c0-fcf61aeb1775\"}","ctxAccountId":"2","uuid":"44bb4d22-c43c-4ce5-a9c0-fcf61aeb1775","cmdEventType":"TEMPLATE.DELETE"}, cmdVersion: 0, status: IN_PROGRESS, processStatus: 0, resultCode: 0, result: null, initMsid: 32986556793923, completeMsid: null, lastUpdated: null, lastPolled: null, created: null, removed: null}

After the fix


2023-08-02 10:21:24,306 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-41:ctx-f860df56 job-73) (logid:1cb54178) Executing AsyncJobVO: {id:73, userId: 2, accountId: 2, instanceType: Template, instanceId: 205, cmd: org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd, cmdInfo: {"apiKey":"LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q","signature":"bdb7Zdkui9CZUoFAr7RMvEoIpm8\u003d","response":"json","ctxUserId":"2","httpmethod":"GET","ctxStartEventId":"161","id":"2bc0c528-55f8-4860-9b00-8f1b0f2acff8","ctxDetails":"{\"interface com.cloud.template.VirtualMachineTemplate\":\"2bc0c528-55f8-4860-9b00-8f1b0f2acff8\"}","ctxAccountId":"2","uuid":"2bc0c528-55f8-4860-9b00-8f1b0f2acff8","cmdEventType":"TEMPLATE.DELETE"}, cmdVersion: 0, status: IN_PROGRESS, processStatus: 0, resultCode: 0, result: null, initMsid: 32988351955670, completeMsid: null, lastUpdated: null, lastPolled: null, created: null, removed: null}
2023-08-02 10:21:24,306 DEBUG [c.c.a.ApiServlet] (qtp1239807799-13:ctx-a0ba720f ctx-e324f833 ctx-89287e07) (logid:9ba795bc) ===END=== 10.0.3.251 -- GET apiKey=LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q&command=deleteTemplate&id=2bc0c528-55f8-4860-9b00-8f1b0f2acff8&response=json&signature=bdb7Zdkui9CZUoFAr7RMvEoIpm8%3D
2023-08-02 10:21:24,315 WARN [c.c.t.TemplateManagerImpl] (API-Job-Executor-41:ctx-f860df56 job-73 ctx-89c8cbb6) (logid:1cb54178) Unable to delete template with id: 205 because VM instances: [VM instance {"id":8,"instanceName":"i-2-8-VM","type":"User","uuid":"3dbd61ea-6608-4bf7-9144-ac510eec8358"}] are using it.
2023-08-02 10:21:24,320 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-41:ctx-f860df56 job-73) (logid:1cb54178) Complete async job-73, jobStatus: FAILED, resultCode: 431, result: org.apache.cloudstack.api.response.ExceptionResponse/null/{"uuidList":[],"errorcode":"431","errortext":"Unable to delete template with id: 205 because VM instances: [VM instance {"id":8,"instanceName":"i-2-8-VM","type":"User","uuid":"3dbd61ea-6608-4bf7-9144-ac510eec8358"}] are using it."

weizhouapache · 2023-08-10T13:59:09Z

@rohityadavcloud @harikrishna-patnala @DaanHoogland @shwstppr @kiranchavala
what should we proceed with this PR ?
it looks I am the only one disagree to merge this.

weizhouapache · 2023-08-15T11:56:37Z

moved to 4.18.2.0 milestone

yadvr

LGTM didn't test it

harikrishna-patnala · 2023-08-28T04:20:07Z

@blueorangutan package

blueorangutan · 2023-08-28T04:22: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.

blueorangutan · 2023-08-28T05:13:45Z

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

harikrishna-patnala · 2023-08-28T05:15:56Z

@blueorangutan test

blueorangutan · 2023-08-28T05:18:03Z

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

blueorangutan · 2023-08-28T16:53:41Z

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

Test	Result	Time (s)	Test File
test_03_deploy_vm_wrong_checksum	`Error`	40.63	test_templates.py
test_09_list_templates_download_details	`Failure`	0.06	test_templates.py

DaanHoogland · 2023-10-02T13:50:29Z

@harikrishna-patnala any resolution on this? cc @shwstppr

DaanHoogland · 2023-10-05T13:05:34Z

this parameter was introduced in PR #1773
changing the default value to false makes sense. However, it will break backwards compatibility. once this is merged, we need to add a note to to release note (e.g. in Breaking changes section)

@shwstppr , I think we can merge this if we take @weizhouapache 's remark into account. What do you think?

shwstppr · 2023-10-06T12:39:02Z

@DaanHoogland I'm okay to merge this.
@harikrishna-patnala if you're okay please take this out of draft

harikrishna-patnala · 2023-10-09T07:35:13Z

@DaanHoogland I'm okay to merge this. @harikrishna-patnala if you're okay please take this out of draft

yes @DaanHoogland This can be merged cc @shwstppr

DaanHoogland · 2023-10-09T09:30:00Z

@harikrishna-patnala
can you add a doc PR for this please?

harikrishna-patnala · 2023-10-10T11:01:11Z

@DaanHoogland I get that I'm breaking backward compatibility, in my opinion documenting this is not needed as we are not breaking anything that is correct or expected behaviour by someone. On the other hand I also dont know where exactly to document this if you can suggest that helps please.

DaanHoogland · 2023-10-10T11:03:42Z

as @weizhouapache sugested, it should be in the release notes in the breaking changes section. humans may expect this , but tools like ansible and terraform will not.

harikrishna-patnala · 2023-10-10T11:05:27Z

ah we have a breaking changes section, nice. Thanks @DaanHoogland I'll create one.

…pache#7731) * default value of force should be false * Added force flag in tests (cherry picked from commit a9f3af8) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

boring-cyborgBot added the component:api label Jul 7, 2023

DaanHoogland approved these changes Jul 7, 2023
View reviewed changes

yadvr approved these changes Jul 7, 2023
View reviewed changes

yadvr added this to the 4.18.1.0 milestone Jul 7, 2023

harikrishna-patnala marked this pull request as draft July 14, 2023 06:11

shwstppr approved these changes Jul 21, 2023
View reviewed changes

weizhouapache added the status:needs-review label Jul 26, 2023

kiranchavala approved these changes Aug 7, 2023
View reviewed changes

weizhouapache modified the milestones: 4.18.1.0, 4.18.2.0 Aug 15, 2023

yadvr approved these changes Aug 22, 2023
View reviewed changes

harikrishna-patnala added 2 commits August 28, 2023 09:48

default value of force should be false
6290f42

Added force flag in tests
7b3cc99

harikrishna-patnala force-pushed the DontForceDeleteTemplate branch from 29ac938 to 7b3cc99Compare August 28, 2023 04:19

boring-cyborgBot added component:integration-test Python Warning... Python code Ahead! labels Aug 28, 2023

DaanHoogland added the Severity:Major label Oct 5, 2023

harikrishna-patnala marked this pull request as ready for review October 9, 2023 07:34

DaanHoogland closed this Oct 9, 2023

DaanHoogland reopened this Oct 9, 2023

yadvr merged commit a9f3af8 into apache:4.18Oct 10, 2023

yadvr deleted the DontForceDeleteTemplate branch October 10, 2023 10:20

DaanHoogland mentioned this pull request Oct 30, 2023
Allow deleting used without "force" #8157
Closed

shwstppr mentioned this pull request Feb 5, 2024
4.19 release docs apache/cloudstack-documentation#381
Merged

JoaoJandre mentioned this pull request Mar 14, 2024
Do not retrieve VM's stats on normal VM listing #8782
Merged
13 tasks

Uh oh!

Conversation

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

Uh oh!

Description

Types of changes

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

harikrishna-patnala commented Jul 7, 2023

Uh oh!

blueorangutan commented Jul 7, 2023

Uh oh!

blueorangutan commented Jul 7, 2023

Uh oh!

codecovBot commented Jul 7, 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!

yadvr commented Jul 7, 2023

Uh oh!

blueorangutan commented Jul 7, 2023

Uh oh!

blueorangutan commented Jul 8, 2023

Uh oh!

DaanHoogland commented Jul 10, 2023

Uh oh!

yadvr commented Jul 10, 2023

Uh oh!

blueorangutan commented Jul 10, 2023

Uh oh!

blueorangutan commented Jul 10, 2023

Uh oh!

DaanHoogland commented Jul 11, 2023

Uh oh!

harikrishna-patnala commented Jul 14, 2023

Uh oh!

weizhouapache commented Jul 21, 2023

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Jul 25, 2023

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Aug 10, 2023

Uh oh!

weizhouapache commented Aug 15, 2023

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala commented Aug 28, 2023

Uh oh!

blueorangutan commented Aug 28, 2023

Uh oh!

blueorangutan commented Aug 28, 2023

Uh oh!

harikrishna-patnala commented Aug 28, 2023

Uh oh!

blueorangutan commented Aug 28, 2023

Uh oh!

blueorangutan commented Aug 28, 2023

Uh oh!

DaanHoogland commented Oct 2, 2023

Uh oh!

DaanHoogland commented Oct 5, 2023

Uh oh!

shwstppr commented Oct 6, 2023

Uh oh!

harikrishna-patnala commented Oct 9, 2023

Uh oh!

DaanHoogland commented Oct 9, 2023

Uh oh!

harikrishna-patnala commented Jul 7, 2023•
edited
Loading

codecovBot commented Jul 7, 2023•
edited
Loading