Skip to content

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

Merged
yadvr merged 3 commits intoapache:masterfrom
nuagenetworks:feature/cleanup_cpd
Sep 25, 2019
Merged

Refactoring to remove duplicate code (by Frank/Nuage)#3538
yadvr merged 3 commits intoapache:masterfrom
nuagenetworks:feature/cleanup_cpd

Conversation

@yadvr
Copy link
Copy Markdown
Member

@yadvr yadvr commented Aug 2, 2019

The #3537 would revert the merged PR #3152 as we're close to 4.13 RC1 and have low confidence on what other regressions it might add. This simply re-creates PR #3152 using the same source fork/branch. We can work on this PR and attempt again to merge towards 4.14 after fixing discovered regressions and a more thorough review and testing.

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)

@yadvr yadvr changed the title Refactoring to remove duplicate code Refactoring to remove duplicate code (by Frank/Nuage) Aug 2, 2019
@yadvr yadvr mentioned this pull request Aug 2, 2019
5 tasks
@yadvr yadvr added this to the 4.14.0.0 milestone Aug 2, 2019
@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Aug 2, 2019

@fmaximus please see #3537, we'll need to revert your PR #3152 for this purpose I've reopened your PR from the original branch. Kindly help test and fix regressions discovered in your branch, esp around registration and upload of archived templates and iso (templates/isos that are extractable like *.bz2, *gz etc).

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Aug 7, 2019

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

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

@blueorangutan
Copy link
Copy Markdown

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

@fmaximus fmaximus self-assigned this Aug 25, 2019
@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 10, 2019

@blueorangutan package

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 10, 2019

@fmaximus thanks for self assigning, can you help fix the conflict? I'll try and help you to get this merge now that master is not frozen anymore.

fmaximus and others added 3 commits September 13, 2019 13:23
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.
@fmaximus
Copy link
Copy Markdown
Contributor

@rhtyd Conflict is resolved.
Jenkins build has failed due to some build node issues:
FATAL: Couldn't find any executable in /home/jenkins/tools/maven/apache-maven-3.3.3

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 13, 2019

Thanks @fmaximus I'll kick a build
@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

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

@blueorangutan
Copy link
Copy Markdown

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

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 13, 2019

@blueorangutan test matrix

@blueorangutan
Copy link
Copy Markdown

@rhtyd a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 19, 2019

(el6 failure expected)
@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

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

@blueorangutan
Copy link
Copy Markdown

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

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 19, 2019

@blueorangutan test matrix

@blueorangutan
Copy link
Copy Markdown

@rhtyd a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

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

Test Result Time (s) Test File
test_01_scale_vm Failure 20.86 test_scale_vm.py

@blueorangutan
Copy link
Copy Markdown

Trillian test result (tid-375)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44053 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3538-t375-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 76 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_05_rvpc_multi_tiers Failure 553.46 test_vpc_redundant.py
test_05_rvpc_multi_tiers Error 595.79 test_vpc_redundant.py

@svenvogel
Copy link
Copy Markdown
Contributor

svenvogel commented Sep 20, 2019

@fmaximus @rhtyd does this also remove this global settings?

image

@fmaximus
Copy link
Copy Markdown
Contributor

@svenvogel No, that's not part of it. This was a change I had been working on,
which I still wanted to offer the community.

@svenvogel
Copy link
Copy Markdown
Contributor

@fmaximus thanks for answer. will this removed from you in a seperated PR or should the community track this?

@fmaximus
Copy link
Copy Markdown
Contributor

@svenvogel That should go in a separate PR for sure, as it's a bugfix which should be fixed in the same release as Remove Nuage Plugin.

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 24, 2019

CentOS7 job failed, rekicking
@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

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

@blueorangutan
Copy link
Copy Markdown

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

Test Result Time (s) Test File

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Sep 25, 2019

LGTM, tests LGTM as well. Using lgtms from the original PR, merging this.

@yadvr yadvr merged commit 7f91964 into apache:master Sep 25, 2019
@fmaximus fmaximus deleted the feature/cleanup_cpd branch September 25, 2019 13:29
ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
Refactor: Cleanup duplicate code

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants