[UI] Added attach and detach features to UI for ROOT disks #6201

BryanMLima · 2022-04-04T18:58:08Z

Description

It was already possible via API to attach and detach ROOT disks as well as DATADISKs. However, it was only possible to call these methods on DATADISKs through the UI. This PR implements this functionality on the ROOT disks via UI as well with the use of the deviceid parameter. Moreover, the API description for attachVolume was improved for a better understanding.

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

This was tested on a local lab with the following scenarios:

VM1 with ROOT disk and DATA disk:

Attach DATADISK with deviceid = 0 gives error, as expected.
Attach DATADISK with deviceid = 1 gives error, as expected.
Attach DATADISK with deviceid = 2 with success, as expected.

VM2 without ROOT disk:

Attach DATADISK with deviceid = 0. The DATADISK was recognized as ROOT disk, as expected.
Attach DATADISK with deviceid = 5 with success, as expected.

nvazquez · 2022-04-11T17:16:49Z

@blueorangutan package

blueorangutan · 2022-04-11T17:17:02Z

@nvazquez a 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 · 2022-04-11T18:04:09Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3127

nvazquez · 2022-04-12T16:21:29Z

@blueorangutan test

blueorangutan · 2022-04-12T16:22:03Z

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

nvazquez

@BryanMLima after I detach a VM's ROOT disk - the instances list is broken, can you please check?

blueorangutan · 2022-04-13T01:01:00Z

Trillian test result (tid-3880)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29760 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6201-t3880-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_iso.py

BryanMLima · 2022-04-14T20:39:30Z

Hello @nvazquez,
I tested this scenario at my local lab and it listed the instances normally. Furthermore, this error does not seem to be related to the changes I've made, as I only changed the sections related to the Attach and Detach of volumes. Could you please test again?

nvazquez · 2022-04-15T03:16:59Z

@blueorangutan package

blueorangutan · 2022-04-15T03:18:03Z

@nvazquez a 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 · 2022-04-15T03:57:17Z

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 3178

nvazquez · 2022-04-18T11:45:01Z

@blueorangutan package

blueorangutan · 2022-04-18T11:46:03Z

@nvazquez a 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 · 2022-04-18T12:34:03Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3199

nvazquez · 2022-04-19T15:24:36Z

Hi @BryanMLima I have tested again and I still have the same issue after detaching a VM's root disk - both times have tested your on a vanilla environment. I have traced the error is caused by this:

2022-04-19 15:17:37,061 DEBUG [c.c.a.m.AgentManagerImpl] (AgentManager-Handler-8:null) (logid:) SeqA 3-6270: Processing Seq 3-6270:  { Cmd , MgmtId: -1, via: 3, Ver: v1, Flags: 11, [{"com.cloud.agent.api.ConsoleProxyLoadReportCommand":{"_proxyVmId":"2","_loadInfo":"{
  "connections": []
}","wait":"0","bypassHostMaintenance":"false"}}] }
2022-04-19 15:17:37,064 DEBUG [c.c.a.m.AgentManagerImpl] (AgentManager-Handler-8:null) (logid:) SeqA 3-6270: Sending Seq 3-6270:  { Ans: , MgmtId: 32988268069116, via: 3, Ver: v1, Flags: 100010, [{"com.cloud.agent.api.AgentControlAnswer":{"result":"true","wait":"0","bypassHostMaintenance":"false"}}] }
2022-04-19 15:17:37,498 DEBUG [o.a.c.h.HAManagerImpl] (BackgroundTaskPollManager-5:ctx-4f360eb6) (logid:80937b13) HA health check task is running...
2022-04-19 15:17:39,780 DEBUG [c.c.a.ApiServlet] (qtp793138072-17:ctx-5f7bb05e) (logid:dc99935d) ===START===  10.0.3.251 -- GET  listall=true&page=1&pagesize=20&showIcon=true&command=listVirtualMachinesMetrics&response=json
2022-04-19 15:17:39,784 DEBUG [c.c.a.ApiServer] (qtp793138072-17:ctx-5f7bb05e ctx-162916e3) (logid:dc99935d) CIDRs from which account 'Acct[d088e28f-bf84-11ec-b127-1e00ae0000fc-admin] -- Account {"id": 2, "name": "admin", "uuid": "d088e28f-bf84-11ec-b127-1e00ae0000fc"}' is allowed to perform API calls: 0.0.0.0/0,::/0
2022-04-19 15:17:39,802 ERROR [c.c.a.ApiServer] (qtp793138072-17:ctx-5f7bb05e ctx-162916e3) (logid:dc99935d) unhandled exception executing api command: [Ljava.lang.String;@6baff340
java.lang.NullPointerException
	at com.cloud.api.ApiDBUtils.findDiskOfferingById(ApiDBUtils.java:1056)
	at com.cloud.api.query.dao.UserVmJoinDaoImpl.newUserVmResponse(UserVmJoinDaoImpl.java:187)
	at jdk.internal.reflect.GeneratedMethodAccessor330.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:344)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at com.cloud.utils.db.TransactionContextInterceptor.invoke(TransactionContextInterceptor.java:34)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
	at com.sun.proxy.$Proxy158.newUserVmResponse(Unknown Source)
	at com.cloud.api.ApiDBUtils.newUserVmResponse(ApiDBUtils.java:1812)
	at com.cloud.api.query.ViewResponseHelper.createUserVmResponse(ViewResponseHelper.java:154)
	at com.cloud.api.query.QueryManagerImpl.searchForUserVMs(QueryManagerImpl.java:931)
	at org.apache.cloudstack.api.ListVMsMetricsCmd.execute(ListVMsMetricsCmd.java:60)
	at com.cloud.api.ApiDispatcher.dispatch(ApiDispatcher.java:156)
	at com.cloud.api.ApiServer.queueCommand(ApiServer.java:776)
	at com.cloud.api.ApiServer.handleRequest(ApiServer.java:600)
	at com.cloud.api.ApiServlet.processRequestInContext(ApiServlet.java:334)

I can create a fix for this issue, can you please rebase main branch?

acs-robot · 2022-04-19T17:17:20Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

acs-robot · 2022-04-19T17:17:21Z

Found Java/XML changes, kicking packaging job
@blueorangutan package

acs-robot · 2022-04-19T17:55:06Z

PR Coverage Report

CLASS	INSTRUCTION MISSED	INSTRUCTION COVERED	BRANCH MISSED	BRANCH COVERED	LINE MISSED	LINE COVERED

nvazquez · 2022-04-20T16:00:59Z

@blueorangutan package

blueorangutan · 2022-04-20T16:02:02Z

@nvazquez a 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 · 2022-04-20T18:18:09Z

Packaging result: ✖️ el7 ✖️ debian. SL-JID 3246

…ceID for ROOT and DATADISK

acs-robot · 2022-04-20T19:33:56Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

blueorangutan · 2022-04-20T19:37:03Z

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

BryanMLima · 2022-04-20T19:38:03Z

Hi @nvazquez, thanks for creating a fix, I messed up the commit history, but I correctly rebased my branch with the main branch now.

blueorangutan · 2022-04-20T19:46:59Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6201 (SL-JID-1417)

acs-robot · 2022-04-20T20:19:38Z

PR Coverage Report

CLASS	INSTRUCTION MISSED	INSTRUCTION COVERED	BRANCH MISSED	BRANCH COVERED	LINE MISSED	LINE COVERED

nvazquez · 2022-04-21T00:11:50Z

Thanks @BryanMLima, I’ll test again with the latest changes

nvazquez · 2022-04-21T00:12:04Z

@blueorangutan package

blueorangutan · 2022-04-21T00:13:03Z

@nvazquez a 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 · 2022-04-21T00:49:13Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3254

nvazquez · 2022-04-21T02:19:18Z

@blueorangutan test

blueorangutan · 2022-04-21T02:20:03Z

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

nvazquez

LGTM - manually tested on KVM env.

One improvement request, outside the scope of this PR: in case the device ID is left empty it could be expected that CloudStack finds a suitable device ID depending on the number of attached volumes and the hypervisor (example: attach datadisk to a VM without disks and pass device ID empty -> expected device ID = 0, actual device ID = 1)

weizhouapache

code lgtm

blueorangutan · 2022-04-21T13:11:13Z

Trillian test result (tid-3956)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37692 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6201-t3956-kvm-centos7.zip
Smoke tests completed. 93 look OK, 3 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_add_primary_storage_disabled_host	`Error`	0.64	test_primary_storage.py
test_01_primary_storage_nfs	`Error`	0.12	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.23	test_primary_storage.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	30.82	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	57.44	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	42.14	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	31.82	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	120.28	test_kubernetes_clusters.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	301.81	test_hostha_kvm.py

nvazquez · 2022-04-22T02:28:37Z

Merged based on approvals, intermittent tests failures not related to this PR

BryanMLima changed the title ~~[UI'Added attach and detach features to UI for ROOT disks~~ [UI] Added attach and detach features to UI for ROOT disks Apr 4, 2022

nvazquez added type:enhancement component:UI labels Apr 11, 2022

nvazquez added this to the 4.17.0.0 milestone Apr 12, 2022

nvazquez reviewed Apr 12, 2022

View reviewed changes

nvazquez mentioned this pull request Apr 19, 2022

Fix: Prevent NPE on disk offering search while listing VMs #6287

Merged

12 tasks

boring-cyborg bot added component:agent component:api labels Apr 19, 2022

Added attach and detach features to UI for ROOT disks as well as devi…

d13332c

…ceID for ROOT and DATADISK

BryanMLima force-pushed the attach-and-detach-root-disk branch from 63caa02 to d13332c Compare April 20, 2022 19:32

nvazquez approved these changes Apr 21, 2022

View reviewed changes

nvazquez requested review from Pearl1594 and weizhouapache April 21, 2022 02:30

weizhouapache approved these changes Apr 21, 2022

View reviewed changes

weizhouapache removed the component:agent label Apr 21, 2022

apache deleted a comment from blueorangutan Apr 22, 2022

nvazquez merged commit 974e57a into apache:main Apr 22, 2022

nvazquez added the type:new-feature label Apr 26, 2022

[UI] Added attach and detach features to UI for ROOT disks #6201

[UI] Added attach and detach features to UI for ROOT disks #6201

Conversation

BryanMLima commented Apr 4, 2022

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

nvazquez commented Apr 11, 2022

blueorangutan commented Apr 11, 2022

blueorangutan commented Apr 11, 2022

nvazquez commented Apr 12, 2022

blueorangutan commented Apr 12, 2022

nvazquez left a comment • edited

Choose a reason for hiding this comment

blueorangutan commented Apr 13, 2022

BryanMLima commented Apr 14, 2022

nvazquez commented Apr 15, 2022

blueorangutan commented Apr 15, 2022

blueorangutan commented Apr 15, 2022

nvazquez commented Apr 18, 2022

blueorangutan commented Apr 18, 2022

blueorangutan commented Apr 18, 2022

nvazquez commented Apr 19, 2022

acs-robot commented Apr 19, 2022

acs-robot commented Apr 19, 2022

acs-robot commented Apr 19, 2022

PR Coverage Report

nvazquez commented Apr 20, 2022

blueorangutan commented Apr 20, 2022

blueorangutan commented Apr 20, 2022

acs-robot commented Apr 20, 2022

blueorangutan commented Apr 20, 2022

BryanMLima commented Apr 20, 2022

blueorangutan commented Apr 20, 2022

acs-robot commented Apr 20, 2022

PR Coverage Report

nvazquez commented Apr 21, 2022

nvazquez commented Apr 21, 2022

blueorangutan commented Apr 21, 2022

blueorangutan commented Apr 21, 2022

nvazquez commented Apr 21, 2022

blueorangutan commented Apr 21, 2022

nvazquez left a comment

Choose a reason for hiding this comment

weizhouapache left a comment

Choose a reason for hiding this comment

blueorangutan commented Apr 21, 2022

nvazquez commented Apr 22, 2022

nvazquez left a comment •

edited