[CLOUDSTACK-10307] Remove unused things from HostDaoImpl #2438

rafaelweingartner · 2018-01-30T20:29:25Z

Remove unnecessary annotation of HostDaoImpl. While removing this annotation I noticed that one of the methods were not necessary. While removing it, I found some code in CloudZonesStartupProcessor that was also not used, and removed it.

DaanHoogland · 2018-01-31T09:56:50Z

@rafaelweingartner how did you determine they were not used? I hope you didn't rely on an IDE for that.

rafaelweingartner · 2018-01-31T10:00:05Z

What method are you talking about?
I used the IDE call hierarchy function. It works if all of the projects are compiling without problems. However, I also did a file search. If there is no reference, the method is not used. Then, I built CloudStack to see if something would break.

DaanHoogland · 2018-01-31T10:03:43Z

great, and with noredist? i'll kick regression tests on this.

DaanHoogland · 2018-01-31T10:04:03Z

@blueorangutan package

blueorangutan · 2018-01-31T10:04:55Z

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

rafaelweingartner · 2018-01-31T10:05:04Z

I forgot the no redistributable, but I can do it right now.

blueorangutan · 2018-01-31T10:34:38Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1685

rafaelweingartner · 2018-01-31T10:36:32Z

@DaanHoogland I just finished a noredist build, and everything is ok.
I used the following maven command: mvn clean install -P developer,systemvm -Dsimulator -Dnoredist -Dmaven.test.skip=false

blueorangutan · 2018-01-31T11:44:56Z

@DaanHoogland I understand these words: "help", "hello", "thanks", "package", "test"
Test command usage: test [mgmt os] [hypervisor] [additional tests]
Mgmt OS options: ['centos6', 'centos7', 'ubuntu']
Hypervisor options: ['kvm-centos6', 'kvm-centos7', 'kvm-ubuntu', 'xenserver-71', 'xenserver-65sp1', 'xenserver-62sp1', 'vmware-65', 'vmware-60u2', 'vmware-55u3', 'vmware-51u1', 'vmware-50u1']
Additional tests: list of space separated tests with paths relative to the test/integration directory, for example: component/test_acl_listvm.py component/test_volumes.py
Note: when additional tests are passed, you need to specify mgmt server os and hypervisor or use the matrix command.

Blessed contributors for kicking Trillian test jobs: ['rhtyd', 'nvazquez', 'PaulAngus', 'borisstoyanov', 'DaanHoogland']

DaanHoogland · 2018-01-31T11:50:22Z

@blueorangutan test centos7 vmware-60u2

blueorangutan · 2018-01-31T11:50:57Z

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

borisstoyanov

@rafaelweingartner can you please add JIRA item

borisstoyanov · 2018-02-25T09:31:16Z

@blueorangutan test centos7 vmware-60u2

blueorangutan · 2018-02-25T09:31:52Z

@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + vmware-60u2) has been kicked to run smoke tests

rafaelweingartner · 2018-02-26T10:26:51Z

@borisstoyanov done.
Thanks for the review here.

borisstoyanov · 2018-02-26T11:59:44Z

@blueorangutan test

blueorangutan · 2018-02-26T11:59:57Z

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

blueorangutan · 2018-02-26T19:13:07Z

Trillian test result (tid-2283)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 24033 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2438-t2283-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 66 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_hostha_enable_ha_when_host_in_maintenance	`Error`	5.01	test_hostha_kvm.py

rafaelweingartner · 2018-02-26T19:29:46Z

@borisstoyanov is this "test_hostha_enable_ha_when_host_in_maintenance" failure a false positive?
I have seen this around in some other PRs.
In the log file I see the following:
2018-02-26 19:01:11,248 DEBUG [c.c.a.m.AgentManagerImpl] (API-Job-Executor-63:ctx-62ff4a7f job-2987 ctx-3ead5878) (logid:c3cec415) Can not send command com.cloud.agent.api.MaintainCommand due to Host 2 is not up

It looks like an environment problem.

DaanHoogland · 2018-02-27T11:30:30Z

@blueorangutan test

blueorangutan · 2018-02-27T11:31:02Z

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

blueorangutan · 2018-02-27T19:32:38Z

Trillian test result (tid-2288)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 27218 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2438-t2288-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_public_ip_range.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_usage.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 64 look OK, 3 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_04_extract_template	`Failure`	128.28	test_templates.py
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py
test_06_download_detached_volume	`Failure`	136.56	test_volumes.py

rafaelweingartner · 2018-02-27T19:39:10Z

It seems that in this new run the number of errors increased?

rafaelweingartner · 2018-03-13T14:11:46Z

@borisstoyanov are these errors persistent ones, or should I take a look into them?

borisstoyanov · 2018-03-14T08:04:03Z

let me re-run them again, I think they might be ignored, but still let's see if they'll appear again
@blueorangutan test

blueorangutan · 2018-03-14T08:04:26Z

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

blueorangutan · 2018-03-14T15:18:48Z

Trillian test result (tid-2360)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 24338 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2438-t2360-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 65 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_04_rvpc_network_garbage_collector_nics	`Failure`	262.94	test_vpc_redundant.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	0.62	test_hostha_kvm.py

rafaelweingartner · 2018-03-14T15:20:26Z

Well, now, errors changed. I am not sure if this is a good sign though.

borisstoyanov · 2018-03-15T09:57:11Z

@blueorangutan test

blueorangutan · 2018-03-15T09:57:30Z

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

GabrielBrascher · 2018-03-16T15:25:34Z

Thanks, @rafaelweingartner!
Based on code review and that you have checked all the ways that a code has to be called, this PR LGTM.

borisstoyanov

LGTM based on test results

DaanHoogland · 2018-03-19T08:34:45Z

@borisstoyanov @rafaelweingartner i have not seen any succesful tests for vmware on this; not merging!
@blueorangutan test centos7 vmware-60u2

blueorangutan · 2018-03-19T08:35:47Z

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

rafaelweingartner · 2018-03-19T09:50:25Z

So, I wait for the results.

rafaelweingartner · 2018-03-22T13:55:11Z

@DaanHoogland do you have the test results? It seems that something went wrong and they never got posted here.

DaanHoogland · 2018-03-26T06:59:56Z

sorry for the late response, @rafaelweingartner . I have no idea what is keeping them, I'll look.

DaanHoogland · 2018-03-26T07:00:34Z

@blueorangutan test centos7 vmware-65

blueorangutan · 2018-03-26T07:01:18Z

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

blueorangutan · 2018-03-26T14:54:55Z

Trillian test result (tid-2415)
Environment: vmware-65 (x2), Advanced Networking with Mgmt server 7
Total time taken: 26863 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2438-t2415-vmware-65.zip
Smoke tests completed. 67 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

rafaelweingartner · 2018-03-26T15:10:08Z

@DaanHoogland the test result came and everything seems to be ok.
Do you want to do other tests?

DaanHoogland · 2018-03-27T10:46:14Z

lgtm

Remove unused code from CloudZonesStartupProcessor

fea7b9b

rafaelweingartner added the type:cleanup label Jan 30, 2018

rafaelweingartner added this to the 4.12 milestone Jan 30, 2018

rafaelweingartner self-assigned this Jan 30, 2018

rafaelweingartner requested a review from GabrielBrascher January 30, 2018 20:29

Removed unused @component annotation from HostDaoImpl And unused method

0a4785e

rafaelweingartner force-pushed the removeUnusedThingsHostDao branch from 7bf18e2 to 0a4785e Compare January 30, 2018 21:12

apache deleted a comment from blueorangutan Jan 31, 2018

borisstoyanov reviewed Feb 25, 2018

View reviewed changes

rafaelweingartner changed the title ~~Remove unused things from HostDaoImpl~~ [CLOUDSTACK-10307] Remove unused things from HostDaoImpl Feb 26, 2018

borisstoyanov approved these changes Mar 19, 2018

View reviewed changes

DaanHoogland merged commit 0afcec6 into apache:master Mar 27, 2018

[CLOUDSTACK-10307] Remove unused things from HostDaoImpl #2438

[CLOUDSTACK-10307] Remove unused things from HostDaoImpl #2438

Conversation

rafaelweingartner commented Jan 30, 2018

DaanHoogland commented Jan 31, 2018 • edited Loading

rafaelweingartner commented Jan 31, 2018

DaanHoogland commented Jan 31, 2018

DaanHoogland commented Jan 31, 2018

blueorangutan commented Jan 31, 2018

rafaelweingartner commented Jan 31, 2018

blueorangutan commented Jan 31, 2018

rafaelweingartner commented Jan 31, 2018

blueorangutan commented Jan 31, 2018

DaanHoogland commented Jan 31, 2018

blueorangutan commented Jan 31, 2018

borisstoyanov left a comment

Choose a reason for hiding this comment

borisstoyanov commented Feb 25, 2018

blueorangutan commented Feb 25, 2018

rafaelweingartner commented Feb 26, 2018

borisstoyanov commented Feb 26, 2018

blueorangutan commented Feb 26, 2018

blueorangutan commented Feb 26, 2018

rafaelweingartner commented Feb 26, 2018

DaanHoogland commented Feb 27, 2018

blueorangutan commented Feb 27, 2018

blueorangutan commented Feb 27, 2018

rafaelweingartner commented Feb 27, 2018

rafaelweingartner commented Mar 13, 2018

borisstoyanov commented Mar 14, 2018

blueorangutan commented Mar 14, 2018

blueorangutan commented Mar 14, 2018

rafaelweingartner commented Mar 14, 2018

borisstoyanov commented Mar 15, 2018

blueorangutan commented Mar 15, 2018

GabrielBrascher commented Mar 16, 2018

borisstoyanov left a comment

Choose a reason for hiding this comment

DaanHoogland commented Mar 19, 2018

blueorangutan commented Mar 19, 2018

rafaelweingartner commented Mar 19, 2018

rafaelweingartner commented Mar 22, 2018

DaanHoogland commented Mar 26, 2018

DaanHoogland commented Mar 26, 2018

blueorangutan commented Mar 26, 2018

blueorangutan commented Mar 26, 2018

rafaelweingartner commented Mar 26, 2018

DaanHoogland commented Mar 27, 2018

DaanHoogland commented Jan 31, 2018 •

edited

Loading