Filter removed nics while listing LB vm instances by GutoVeronezi · Pull Request #6430 · apache/cloudstack

GutoVeronezi · 2022-06-01T19:05:34Z

Description

When we remove the NIC of a network from a VM and add a new NIC of the same network, we will have two registers in the database for the same VM and network, one of them being marked as removed. In API listLoadBalancerRuleInstances, specifically in the method LoadBalancingRulesManagerImpl#listLoadBalancerInstances, ACS retrieves the instances according to the user's account and the network where the LB is being created; however, it lists all the NICs, even the marked as removed, which causes the following error while listing the VMs:

2022-05-16 14:06:32,859 ERROR [c.c.a.ApiServer] (qtp365590665-27:ctx-5ce79f30 ctx-9de90969) (logid:a3518976) unhandled exception executing api command: [Ljava.lang.String;@55d45499
java.lang.IndexOutOfBoundsException: Index 2 out of bounds for length 2

This PR intends to fix this error by filtering the removed NICs while listing the LB intances.

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

Bug Severity

BLOCKER
Critical
Major
[ x] Minor
Trivial

How Has This Been Tested?

In a local lab, I created a VM with a random network. I added a NIC for a specific network (test) to the VM. I removed the NIC of network test and then added it again. I created a LB rule with this VM. Before the changes, while listing the LB rule instances I got Index 1 out of bounds for length 1. After the changes, the instance was listed.

acs-robot · 2022-06-01T19:06:44Z

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

blueorangutan · 2022-06-01T19:07:03Z

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

blueorangutan · 2022-06-01T19:13:39Z

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

sureshanaparti

code LGTM

sureshanaparti · 2022-06-02T05:06:12Z

@blueorangutan package

sureshanaparti · 2022-06-02T05:11:24Z

@blueorangutan package

blueorangutan · 2022-06-02T05:12:03Z

@sureshanaparti 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-06-02T06:18:36Z

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

sureshanaparti · 2022-06-02T06:32:37Z

@blueorangutan test

blueorangutan · 2022-06-02T06:39:02Z

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

blueorangutan · 2022-06-02T18:23:22Z

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

Test	Result	Time (s)	Test File
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Failure`	461.16	test_vpc_redundant.py

DaanHoogland

code looks good @GutoVeronezi , but did you check if the list is retrieved in other places as well? I agree it should work like this and if removed nics should be retrieved as well a method like listNicsIncludingRomoved(). This is not always implemented this way so care must be taken.

nvazquez · 2022-06-08T01:11:50Z

Hi @GutoVeronezi can you please target this fix to the 4.17 branch?

GutoVeronezi · 2022-06-21T13:22:23Z

Hi @GutoVeronezi can you please target this fix to the 4.17 branch?

Hi @nvazquez, done.

acs-robot · 2022-06-21T13:22:58Z

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

blueorangutan · 2022-06-21T13:24:03Z

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

blueorangutan · 2022-06-21T13:32:36Z

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

GutoVeronezi · 2022-06-21T13:35:03Z

code looks good @GutoVeronezi , but did you check if the list is retrieved in other places as well? I agree it should work like this and if removed nics should be retrieved as well a method like listNicsIncludingRomoved(). This is not always implemented this way so care must be taken.

Hi @DaanHoogland,

nicSearch is declared in the method's scope and is only used by AccountDataCenterVirtualSearch, which is only used by the method listVirtualNetworkInstancesByAcctAndNetwork, which is only called in the API listLoadBalancerRuleInstances workflow:

sonarqubecloud · 2022-06-21T14:28:47Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

DaanHoogland · 2022-06-21T16:12:29Z

code looks good @GutoVeronezi , but did you check if the list is retrieved in other places as well? I agree it should work like this and if removed nics should be retrieved as well a method like listNicsIncludingRomoved(). This is not always implemented this way so care must be taken.

Hi @DaanHoogland,

nicSearch is declared in the method's scope and is only used by AccountDataCenterVirtualSearch, which is only used by the method listVirtualNetworkInstancesByAcctAndNetwork, which is only called in the API listLoadBalancerRuleInstances workflow:

Ok, than renaming to listNicsIncluding removed() or not is free.

yadvr · 2022-06-30T06:34:59Z

@blueorangutan package

blueorangutan · 2022-06-30T06:36:03Z

@rohityadavcloud 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-06-30T11:01:55Z

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

sureshanaparti · 2022-06-30T12:25:30Z

@blueorangutan package

blueorangutan · 2022-06-30T12:27:03Z

@sureshanaparti 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-06-30T13:45:26Z

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

GutoVeronezi · 2022-07-05T19:49:48Z

@sureshanaparti could we run the tests again?

sureshanaparti · 2022-07-06T04:04:36Z

@blueorangutan package

blueorangutan · 2022-07-06T04:05:03Z

@sureshanaparti 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-07-06T05:06:38Z

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

sureshanaparti · 2022-07-06T05:19:33Z

@blueorangutan test

blueorangutan · 2022-07-06T05:20:03Z

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

blueorangutan · 2022-07-06T16:24:20Z

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

Test	Result	Time (s)	Test File
test_01_add_primary_storage_disabled_host	`Error`	0.59	test_primary_storage.py
test_01_primary_storage_nfs	`Error`	0.11	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.20	test_primary_storage.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	28.92	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	65.61	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	37.99	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	37.96	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	123.49	test_kubernetes_clusters.py
test_01_secure_vm_migration	`Error`	155.58	test_vm_life_cycle.py
test_02_unsecure_vm_migration	`Error`	269.05	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	`Error`	143.87	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	43.73	test_vm_life_cycle.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	304.79	test_hostha_kvm.py

DaanHoogland · 2022-07-07T07:48:44Z

@blueorangutan test

blueorangutan · 2022-07-07T07:49:03Z

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

blueorangutan · 2022-07-07T19:39:33Z

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

Test	Result	Time (s)	Test File
test_08_arping_in_ssvm	`Failure`	5.17	test_diagnostics.py

harikrishna-patnala

LGTM, test_09_arping_in_cpvm test failure does not look related to this PR change.

harikrishna-patnala · 2022-07-11T10:11:58Z

I was able to reproduce the issue and tested the fix also.

sureshanaparti approved these changes Jun 2, 2022

View reviewed changes

DaanHoogland reviewed Jun 3, 2022

View reviewed changes

nvazquez force-pushed the main branch from c56220f to 0f4c3af Compare June 6, 2022 15:14

nvazquez added the type:bug label Jun 8, 2022

nvazquez added this to the 4.17.1.0 milestone Jun 8, 2022

Filter removed nics while listing LB vm instances

dcfbe60

GutoVeronezi force-pushed the fix-listLoadBalancerRuleInstances-index-out-of-bound branch from fa2ffac to dcfbe60 Compare June 21, 2022 13:21

GutoVeronezi changed the base branch from main to 4.17 June 21, 2022 13:21

DaanHoogland approved these changes Jun 23, 2022

View reviewed changes

yadvr assigned harikrishna-patnala Jul 6, 2022

harikrishna-patnala approved these changes Jul 11, 2022

View reviewed changes

harikrishna-patnala merged commit b2cf82e into apache:4.17 Jul 11, 2022

Conversation

GutoVeronezi commented Jun 1, 2022

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Bug Severity

How Has This Been Tested?

Uh oh!

acs-robot commented Jun 1, 2022

Uh oh!

blueorangutan commented Jun 1, 2022

Uh oh!

blueorangutan commented Jun 1, 2022

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jun 2, 2022

Uh oh!

sureshanaparti commented Jun 2, 2022

Uh oh!

blueorangutan commented Jun 2, 2022

Uh oh!

blueorangutan commented Jun 2, 2022

Uh oh!

sureshanaparti commented Jun 2, 2022

Uh oh!

blueorangutan commented Jun 2, 2022

Uh oh!

blueorangutan commented Jun 2, 2022

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jun 8, 2022

Uh oh!

GutoVeronezi commented Jun 21, 2022

Uh oh!

acs-robot commented Jun 21, 2022

Uh oh!

blueorangutan commented Jun 21, 2022

Uh oh!

blueorangutan commented Jun 21, 2022

Uh oh!

GutoVeronezi commented Jun 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jun 21, 2022

Uh oh!

DaanHoogland commented Jun 21, 2022

Uh oh!

yadvr commented Jun 30, 2022

Uh oh!

blueorangutan commented Jun 30, 2022

Uh oh!

blueorangutan commented Jun 30, 2022

Uh oh!

sureshanaparti commented Jun 30, 2022

Uh oh!

blueorangutan commented Jun 30, 2022

Uh oh!

blueorangutan commented Jun 30, 2022

Uh oh!

GutoVeronezi commented Jul 5, 2022

Uh oh!

sureshanaparti commented Jul 6, 2022

Uh oh!

blueorangutan commented Jul 6, 2022

Uh oh!

blueorangutan commented Jul 6, 2022

Uh oh!

sureshanaparti commented Jul 6, 2022

Uh oh!

blueorangutan commented Jul 6, 2022

Uh oh!

blueorangutan commented Jul 6, 2022

Uh oh!

DaanHoogland commented Jul 7, 2022

Uh oh!

blueorangutan commented Jul 7, 2022

GutoVeronezi commented Jun 21, 2022 •

edited

Loading