Add NAT rule handling IDs #216

vbauzys · 2019-07-15T11:15:58Z

Ref: https://github.com/terraform-providers/terraform-provider-vcd/issues/244

Previous temporary functions were removed and new functions added to support ID handling. Majorly solves Nat rule removal.

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com> # Conflicts: # CHANGELOG.md # govcd/edgegateway.go # govcd/edgegateway_test.go

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Didainius

Round 1 :)

Issue (others inline):
I've started tests: go test -tags functional -check.vv -check.f "Test_.*NAT.*" -timeout=15m . on an empty edge gateway.

START: api_vcd_test.go:250: TestVCD.SetUpSuite
PASS: api_vcd_test.go:250: TestVCD.SetUpSuite   15.232s

START: edgegateway_test.go:352: TestVCD.Test_AddDNATRule
PASS: edgegateway_test.go:352: TestVCD.Test_AddDNATRule 44.961s

START: edgegateway_test.go:282: TestVCD.Test_AddSNATRule
PASS: edgegateway_test.go:282: TestVCD.Test_AddSNATRule 44.865s

START: edgegateway_test.go:31: TestVCD.Test_NATMapping
PASS: edgegateway_test.go:31: TestVCD.Test_NATMapping   14.136s

START: edgegateway_test.go:77: TestVCD.Test_NATPortMapping
PASS: edgegateway_test.go:77: TestVCD.Test_NATPortMapping       14.119s

START: api_vcd_test.go:787: TestVCD.TearDownSuite
removeLeftoverEntries: [INFO] Attempting cleanup of vapp 'TestSetUpSuite' instantiated by createTestVapp
removeLeftoverEntries: [INFO] Removed vapp 'TestSetUpSuite' created by createTestVapp
PASS: api_vcd_test.go:787: TestVCD.TearDownSuite        0.288s

OK: 4 passed
PASS
ok      github.com/vmware/go-vcloud-director/v2/govcd   133.743s

After the tests I've got 2 nat rules left:

govcd/edgegateway.go

vbauzys · 2019-07-16T08:10:24Z

Test_NATPortMapping

Test_NATPortMapping and Test_NATMapping do this. These two methods use old delete which isn't working correctly in these test cases.

Possbile solutions:

leave as these are deprecated methods and deprecated tetss
change tests to use new delete method - may be possible. Test still will verify deprecated methods.

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

lvirbalas · 2019-07-16T13:24:01Z

After the tests I've got 2 nat rules left:

@Didainius @vbauzysvmware was there resolution for this issue?

vbauzys · 2019-07-16T13:33:43Z

After the tests I've got 2 nat rules left:

@Didainius @vbauzysvmware was there resolution for this issue?

Test_NATPortMapping and Test_NATMapping do this - they are old unit tests. These two test methods use old delete which isn't working correctly in these test cases.

Possbile solutions:

leave as these are deprecated methods and deprecated tetss
change tests to use new delete method - may be possible. Test still will verify deprecated methods.

govcd/edgegateway.go

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

govcd/edgegateway.go

Didainius · 2019-07-17T06:20:38Z

After the tests I've got 2 nat rules left:

@Didainius @vbauzysvmware was there resolution for this issue?

Test_NATPortMapping and Test_NATMapping do this - they are old unit tests. These two test methods use old delete which isn't working correctly in these test cases.

Possbile solutions:
leave as these are deprecated methods and deprecated tetss
change tests to use new delete method - may be possible. Test still will verify deprecated m

I vote for option 2 and fixing tests. It constantly leaves mess in NAT table and we will not remove it soon.

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

dataclouder

Some changes required. Will review again after tests

govcd/edgegateway.go

dataclouder · 2019-07-17T09:20:30Z

govcd/edgegateway.go

-// Allows to assign specific network Org VDC or external. Old function AddNATPortMapping and
-// AddNATMapping assigns rule to first external network
-func (eGW *EdgeGateway) AddDNATRule(ruleDetails NatRule) ([]*types.NatRule, error) {
+// AddDNATRule creates DNAT rule and return created NAT struct or error.


"and return created NAT struct or error" -> "and returns the NAT struct that was created or an error"

govcd/edgegateway.go

dataclouder · 2019-07-17T09:23:35Z

govcd/edgegateway.go

-// AddNATMapping assigns rule to first external network
-func (eGW *EdgeGateway) AddSNATRule(networkHref, externalIP, internalIP, description string) ([]*types.NatRule, error) {
+// AddSNATRule creates SNAT rule and returns created NAT rule or error.
+// Allows to assign specific Org VDC or external network.


"to assign" -> "assigning"

govcd/edgegateway.go

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

vbauzys · 2019-07-17T10:53:00Z

After the tests I've got 2 nat rules left:

@Didainius @vbauzysvmware was there resolution for this issue?

Test_NATPortMapping and Test_NATMapping do this - they are old unit tests. These two test methods use old delete which isn't working correctly in these test cases.
Possbile solutions:
leave as these are deprecated methods and deprecated tetss
change tests to use new delete method - may be possible. Test still will verify deprecated m
I vote for option 2 and fixing tests. It constantly leaves mess in NAT table and we will not remove it soon.

Changed to use new delete - now clean.

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

govcd/edgegateway_test.go

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

govcd/edgegateway.go

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Didainius

Tests green on 9.5.

dataclouder

LGTM

lvirbalas

LGTM!

vbauzys added 10 commits July 9, 2019 17:03

Add functions to allow create SNAT and DNAT rules for ext netw

b3c63c8

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Fix SNAT, DNAT removing

7c5f595

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Fix comments

a9573b8

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Moved func args to struct

c1f0466

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

add comments

b6ba97e

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

add comments

20b7733

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Changed to support Nat Rule Id's

e3a3799

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

add comment

ab8bf72

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Merge branch 'master' into Nat_ExtNet_support

fbb5e82

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com> # Conflicts: # CHANGELOG.md # govcd/edgegateway.go # govcd/edgegateway_test.go

Add comments

2c35536

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

vbauzys requested review from lvirbalas, dataclouder and Didainius July 15, 2019 11:15

vbauzys self-assigned this Jul 15, 2019

vmwclabot added the cla-not-required label Jul 15, 2019

vbauzys mentioned this pull request Jul 15, 2019

NAT rule support IDs and external or Org networks vmware/terraform-provider-vcd#282

Merged

vbauzys changed the title ~~Nat ext net support~~ Add nat rule handling IDs Jul 15, 2019

vbauzys changed the title ~~Add nat rule handling IDs~~ Add NAT rule handling IDs Jul 15, 2019

Didainius requested changes Jul 16, 2019

View reviewed changes

vbauzys added 3 commits July 16, 2019 11:30

Fixes

0bbd623

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Fixes

068b848

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Added test for update

f37fe1b

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

lvirbalas requested changes Jul 16, 2019

View reviewed changes

Improved comments

5699abb

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Didainius reviewed Jul 17, 2019

View reviewed changes

govcd/edgegateway.go Outdated Show resolved Hide resolved

govcd/edgegateway.go Outdated Show resolved Hide resolved

Improved comments

3e31d5e

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

dataclouder suggested changes Jul 17, 2019

View reviewed changes

vbauzys added 2 commits July 17, 2019 13:12

Improvements

83698c0

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

change tests to use new delete

9075535

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

vbauzys added 2 commits July 17, 2019 14:25

Add uni test

6bfe2df

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Changed fetch/read to Get

b7d2fd2

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

dataclouder reviewed Jul 18, 2019

View reviewed changes

govcd/edgegateway_test.go Outdated Show resolved Hide resolved

vbauzys added 2 commits July 18, 2019 15:07

Moved test function

13a64fc

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Remove unused imports

a686dc8

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

dataclouder reviewed Jul 19, 2019

View reviewed changes

govcd/edgegateway.go Outdated Show resolved Hide resolved

dataclouder reviewed Jul 19, 2019

View reviewed changes

govcd/edgegateway.go Outdated Show resolved Hide resolved

vbauzys added 2 commits July 19, 2019 10:44

Add code improvements

222404d

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Make variable public

2100e4e

Signed-off-by: Vaidotas Bauzys <vbauzys@vmware.com>

Didainius approved these changes Jul 19, 2019

View reviewed changes

dataclouder approved these changes Jul 19, 2019

View reviewed changes

lvirbalas approved these changes Jul 19, 2019

View reviewed changes

vbauzys merged commit 3f6bb23 into vmware:master Jul 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NAT rule handling IDs #216

Add NAT rule handling IDs #216

vbauzys commented Jul 15, 2019

Didainius left a comment

vbauzys commented Jul 16, 2019

lvirbalas commented Jul 16, 2019 •

edited

Loading

vbauzys commented Jul 16, 2019

Didainius commented Jul 17, 2019

dataclouder left a comment

dataclouder Jul 17, 2019

vbauzys Jul 17, 2019

dataclouder Jul 17, 2019

vbauzys Jul 17, 2019

vbauzys commented Jul 17, 2019

Didainius left a comment

dataclouder left a comment

lvirbalas left a comment

Add NAT rule handling IDs #216

Add NAT rule handling IDs #216

Conversation

vbauzys commented Jul 15, 2019

Didainius left a comment

Choose a reason for hiding this comment

vbauzys commented Jul 16, 2019

lvirbalas commented Jul 16, 2019 • edited Loading

vbauzys commented Jul 16, 2019

Didainius commented Jul 17, 2019

dataclouder left a comment

Choose a reason for hiding this comment

dataclouder Jul 17, 2019

Choose a reason for hiding this comment

vbauzys Jul 17, 2019

Choose a reason for hiding this comment

dataclouder Jul 17, 2019

Choose a reason for hiding this comment

vbauzys Jul 17, 2019

Choose a reason for hiding this comment

vbauzys commented Jul 17, 2019

Didainius left a comment

Choose a reason for hiding this comment

dataclouder left a comment

Choose a reason for hiding this comment

lvirbalas left a comment

Choose a reason for hiding this comment

lvirbalas commented Jul 16, 2019 •

edited

Loading