[4.11] Fix set initial reservation on public IP ranges by nvazquez · Pull Request #2980 · apache/cloudstack

nvazquez · 2018-10-29T15:09:52Z

Description

Fix initial reservation on public IP range addition. Also, if the range is dedicated for system VMs do not allow dedicating it to any account

Fixes: #2929

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)

Screenshots (if appropriate):

How Has This Been Tested?

Set reservation to a domain, add range. Verify domain is set to the domain.

yadvr · 2018-10-30T04:14:20Z

@blueorangutan package

blueorangutan · 2018-10-30T04:15:17Z

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

blueorangutan · 2018-10-30T04:46:20Z

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

yadvr · 2018-10-30T04:46:58Z

@blueorangutan test

blueorangutan · 2018-10-30T04:47:18Z

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

DaanHoogland · 2018-10-30T06:39:01Z

                    // This VLAN is domain-wide, so create a DomainVlanMapVO entry
-                    //final DomainVlanMapVO domainVlanMapVO = new DomainVlanMapVO(domain.getId(), vlan.getId());
-                    //_domainVlanMapDao.persist(domainVlanMapVO);
+                    final DomainVlanMapVO domainVlanMapVO = new DomainVlanMapVO(domain.getId(), vlan.getId());


the rest of the change seems trivial. I just wonder why these lines where commented in the first place. Can you explain @nvazquez ?

Hi @nvazquez as i can say. we tested it. it works now! Great work!

Thanks @svenvogel for testing it.

@DaanHoogland it was conflicting with the public IP range retrieval for system VMs. If the selected range was for system VMs then it won't be considered because of the entry on domain_vlan_map

@nvazquez is there any problem to merge it?

blueorangutan · 2018-10-30T11:49:28Z

Trillian test result (tid-3129)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 23742 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2980-t3129-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent 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
runTest	`Error`	0.00	test_vm_life_cycle.py
test_04_rvpc_network_garbage_collector_nics	`Failure`	261.39	test_vpc_redundant.py

svenvogel · 2018-11-01T10:09:20Z

@nvazquez will it merged?

nvazquez · 2018-11-07T12:26:13Z

@DaanHoogland @borisstoyanov @rafaelweingartner are we ok with this fix?

borisstoyanov

LGTM

Reverte o !1037 Closes apache#2980 See merge request scclouds/scclouds!1257

nvazquez added 2 commits October 29, 2018 11:46

Fix initial reservation on public IP ranges

ddde7d1

Do not allow dedicating a system VM IP range

5d4b8cd

nvazquez changed the title ~~Iprangedomainuifix~~ Fix set initial reservation on public IP ranges Oct 29, 2018

nvazquez added the type:enhancement label Oct 29, 2018

nvazquez requested review from DaanHoogland and yadvr October 29, 2018 15:10

nvazquez changed the title ~~Fix set initial reservation on public IP ranges~~ [4.11] Fix set initial reservation on public IP ranges Oct 29, 2018

DaanHoogland reviewed Oct 30, 2018

View reviewed changes

DaanHoogland mentioned this pull request Oct 30, 2018

Fix set initial reservation on public IP ranges #2981

Closed

5 tasks

nvazquez requested a review from rafaelweingartner November 7, 2018 12:22

borisstoyanov approved these changes Nov 7, 2018

View reviewed changes

rafaelweingartner approved these changes Nov 7, 2018

View reviewed changes

rafaelweingartner assigned nvazquez Nov 7, 2018

rafaelweingartner added this to the 4.11.2.0 milestone Nov 7, 2018

rafaelweingartner merged commit 7d8eb37 into apache:4.11 Nov 7, 2018

rafaelweingartner added a commit that referenced this pull request Nov 7, 2018

Forwardmerge #2980 - Fix set initial reservation on public IP ranges

f183268

nvazquez deleted the iprangedomainuifix branch November 7, 2018 16:04

Tonitzpp pushed a commit to scclouds/cloudstack that referenced this pull request Dec 18, 2025

Merge branch 'revert-1037' into '4.20.0.0-scclouds'

5644e7c

Reverte o !1037 Closes apache#2980 See merge request scclouds/scclouds!1257

Conversation

nvazquez commented Oct 29, 2018

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

yadvr commented Oct 30, 2018

Uh oh!

blueorangutan commented Oct 30, 2018

Uh oh!

blueorangutan commented Oct 30, 2018

Uh oh!

yadvr commented Oct 30, 2018

Uh oh!

blueorangutan commented Oct 30, 2018

Uh oh!

DaanHoogland Oct 30, 2018

Choose a reason for hiding this comment

Uh oh!

svenvogel Oct 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nvazquez Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

svenvogel Nov 1, 2018

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 30, 2018

Uh oh!

svenvogel commented Nov 1, 2018

Uh oh!

nvazquez commented Nov 7, 2018

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

svenvogel Oct 30, 2018 •

edited

Loading