Refactoring to remove duplicate code. by fmaximus · Pull Request #3152 · apache/cloudstack

fmaximus · 2019-01-28T16:16:49Z

Description

Removing Code Duplication by refactoring Admin Commands.

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)

How Has This Been Tested?

Added new Unit Tests.

yadvr

LGTM, regression testing needed.

yadvr · 2019-03-26T07:31:54Z

@fmaximus master is unfrozen now, can you fix the conflict. We can merge this after running a final regression test. Thanks.

fmaximus · 2019-04-03T21:26:34Z

Sorry for the delay. I've resolved the conflicts.

yadvr · 2019-05-27T12:33:02Z

@blueorangutan package

blueorangutan · 2019-05-27T12:33:15Z

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

blueorangutan · 2019-05-27T13:56:20Z

Packaging result: ✔centos6 ✖centos7 ✔debian. JID-2790

DaanHoogland · 2019-05-27T14:57:06Z

@blueorangutan package

blueorangutan · 2019-05-27T14:57:17Z

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

blueorangutan · 2019-05-27T16:36:40Z

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

DaanHoogland · 2019-05-27T17:03:48Z

@blueorangutan test

blueorangutan · 2019-05-27T17:04:16Z

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

yadvr · 2019-06-12T20:23:25Z

@blueorangutan package

blueorangutan · 2019-06-12T20:24:25Z

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

blueorangutan · 2019-06-12T20:45:28Z

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

yadvr · 2019-06-12T20:48:46Z

@blueorangutan test

blueorangutan · 2019-06-12T20:49:24Z

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

blueorangutan · 2019-06-13T04:37:11Z

Trillian test result (tid-3691)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 26532 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3152-t3691-kvm-centos7.zip
Smoke tests completed. 71 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

yadvr · 2019-06-20T17:38:39Z

@blueorangutan package

blueorangutan · 2019-06-20T17:38:59Z

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

blueorangutan · 2019-06-20T17:43:40Z

Packaging result: ✖centos6 ✖centos7 ✖debian. JID-2915

yadvr · 2019-06-25T08:34:28Z

Hi @DaanHoogland - can you help review and stabilize this PR? Thanks.

blueorangutan · 2019-07-09T01:03:19Z

Trillian test result (tid-121)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 28651 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3152-t121-kvm-centos7.zip
Smoke tests completed. 71 look OK, 1 have error(s)
Only failed tests results shown below:

yadvr · 2019-07-10T12:27:58Z

cc @shwstppr @borisstoyanov @anuragaw

anuragaw

LGTM - did some manual testing locally on admin vs user API commands but did not test all possible API calls.

borisstoyanov

@fmaximus can you fix the merge conflict pls?

Make use of Java 8 default implementation in interfaces, to remove code duplication between XxxCmd and XxxCmdAsAdmin.

Refactor checkFormat by pre-calculating the supported extensions. Also make use of this in ImageStoreUtil. Makes it easier to add new file and compression formats.

yadvr · 2019-07-22T05:41:36Z

@blueorangutan package

blueorangutan · 2019-07-22T05:42:03Z

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

blueorangutan · 2019-07-22T07:01:44Z

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

yadvr · 2019-07-22T09:47:19Z

@blueorangutan test

blueorangutan · 2019-07-22T09:48:08Z

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

yadvr · 2019-07-25T06:45:31Z

@blueorangutan package

blueorangutan · 2019-07-25T06:46:17Z

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

blueorangutan · 2019-07-25T07:06:51Z

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

yadvr · 2019-07-25T07:19:34Z

@blueorangutan test

blueorangutan · 2019-07-25T07:20:18Z

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

blueorangutan · 2019-07-25T15:58:08Z

Trillian test result (tid-223)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29550 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3152-t223-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 77 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

yadvr · 2019-07-26T06:44:50Z

@blueorangutan test centos7 xenserver-71

blueorangutan · 2019-07-26T06:45:21Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + xenserver-71) has been kicked to run smoke tests

yadvr · 2019-07-26T06:45:38Z

@blueorangutan test centos7 vmware-65u2

blueorangutan · 2019-07-26T06:46:22Z

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

blueorangutan · 2019-07-26T15:54:39Z

Trillian test result (tid-226)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31027 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3152-t226-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Smoke tests completed. 75 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_list_snapshots	`Error`	0.05	test_list_ids_parameter.py
test_01_scale_vm	`Failure`	16.58	test_scale_vm.py

blueorangutan · 2019-07-26T18:12:59Z

Trillian test result (tid-227)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38914 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3152-t227-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_routers.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Smoke tests completed. 73 look OK, 4 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_reboot_router	`Error`	333.39	test_network.py
test_08_start_router	`Error`	68.72	test_routers.py
test_09_reboot_router	`Error`	0.16	test_routers.py
test_02_list_snapshots_with_removed_data_store	`Error`	1.24	test_snapshots.py
test_01_volume_usage	`Error`	10.43	test_usage.py

This reverts commit 7a27e35. We're near 4.13 RC1, we've low confidence if the changes from apache#3152 would cause other regressions so reverting this. The author may send a PR again towards 4.14. Regressions found are all related to template and iso registration and upload.

yadvr · 2019-08-02T07:38:01Z

Reopened it here: #3538

Revert #3152

fmaximus added type:improvement type:cleanup labels Jan 28, 2019

yadvr requested review from nvazquez and rafaelweingartner March 26, 2019 07:31

yadvr approved these changes Mar 26, 2019

View reviewed changes

fmaximus force-pushed the feature/cleanup_cpd branch from 09edb79 to 21557ad Compare April 3, 2019 21:25

fmaximus force-pushed the feature/cleanup_cpd branch from 21557ad to 39b35b0 Compare April 16, 2019 12:35

yadvr added this to the 4.13.0.0 milestone May 27, 2019

yadvr assigned DaanHoogland Jun 25, 2019

fmaximus force-pushed the feature/cleanup_cpd branch from c2fa404 to 58c4058 Compare June 25, 2019 12:22

anuragaw approved these changes Jul 12, 2019

View reviewed changes

borisstoyanov requested changes Jul 18, 2019

View reviewed changes

fmaximus added 2 commits July 19, 2019 15:24

Refactor: Cleanup duplicate code

80458eb

Make use of Java 8 default implementation in interfaces, to remove code duplication between XxxCmd and XxxCmdAsAdmin.

Refactor: Cleanup duplicate code

10a5ccf

Refactor checkFormat by pre-calculating the supported extensions. Also make use of this in ImageStoreUtil. Makes it easier to add new file and compression formats.

fmaximus force-pushed the feature/cleanup_cpd branch from 99afbe1 to 10a5ccf Compare July 19, 2019 13:29

yadvr merged commit 7a27e35 into apache:master Jul 27, 2019

yadvr mentioned this pull request Aug 1, 2019

Revert #3152 #3537

Merged

5 tasks

yadvr mentioned this pull request Aug 2, 2019

Refactoring to remove duplicate code (by Frank/Nuage) #3538

Merged

5 tasks

andrijapanicsb added a commit that referenced this pull request Aug 2, 2019

Merge pull request #3537 from shapeblue/upload-iso-template-fix

3dad7f3

Revert #3152

Uh oh!

Conversation

fmaximus commented Jan 28, 2019

Description

Types of changes

How Has This Been Tested?

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Mar 26, 2019

Uh oh!

fmaximus commented Apr 3, 2019

Uh oh!

yadvr commented May 27, 2019

Uh oh!

blueorangutan commented May 27, 2019

Uh oh!

blueorangutan commented May 27, 2019

Uh oh!

DaanHoogland commented May 27, 2019

Uh oh!

blueorangutan commented May 27, 2019

Uh oh!

blueorangutan commented May 27, 2019

Uh oh!

DaanHoogland commented May 27, 2019

Uh oh!

blueorangutan commented May 27, 2019

Uh oh!

yadvr commented Jun 12, 2019

Uh oh!

blueorangutan commented Jun 12, 2019

Uh oh!

blueorangutan commented Jun 12, 2019

Uh oh!

yadvr commented Jun 12, 2019

Uh oh!

blueorangutan commented Jun 12, 2019

Uh oh!

blueorangutan commented Jun 13, 2019

Uh oh!

yadvr commented Jun 20, 2019

Uh oh!

blueorangutan commented Jun 20, 2019

Uh oh!

blueorangutan commented Jun 20, 2019

Uh oh!

yadvr commented Jun 25, 2019

Uh oh!

blueorangutan commented Jul 9, 2019

Uh oh!

yadvr commented Jul 10, 2019

Uh oh!

anuragaw left a comment

Choose a reason for hiding this comment

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Jul 22, 2019

Uh oh!

blueorangutan commented Jul 22, 2019

Uh oh!

blueorangutan commented Jul 22, 2019

Uh oh!

yadvr commented Jul 22, 2019

Uh oh!

blueorangutan commented Jul 22, 2019

Uh oh!

yadvr commented Jul 25, 2019

Uh oh!

blueorangutan commented Jul 25, 2019

Uh oh!

blueorangutan commented Jul 25, 2019

Uh oh!

yadvr commented Jul 25, 2019

Uh oh!

blueorangutan commented Jul 25, 2019

Uh oh!