Improved VM network to allow reference vApp routed network #472

vbauzys · 2020-03-09T16:22:06Z

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

Improved check to allow reference not only isolated vApp network, but routed vApp network too
Improved tests
Improved import verifying if vApp network type

Signed-off-by: Vaidotas Bauzys <[email protected]>

…vider-vcd

…roviders/terraform-provider-vcd

…vider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd

…roviders/terraform-provider-vcd

…vider-vcd

…roviders/terraform-provider-vcd

…vider-vcd

…roviders/terraform-provider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd

…roviders/terraform-provider-vcd

…vider-vcd

…roviders/terraform-provider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd

…roviders/terraform-provider-vcd

Signed-off-by: Vaidotas Bauzys <[email protected]>

lvirbalas

First pass.

vcd/resource_vcd_vapp_network.go

vcd/resource_vcd_vapp_vm_multinetwork_test.go

lvirbalas · 2020-03-09T16:48:10Z

I confirm that with this fix I was able to update VM config to add vApp routed network!

  # vcd_vapp_vm.demo_vm_wordpress will be updated in-place
  ~ resource "vcd_vapp_vm" "demo_vm_wordpress" {
       
        # ...

        network {
            adapter_type       = "PCNet32"
            ip                 = "192.168.2.150"
            ip_allocation_mode = "DHCP"
            is_primary         = true
            mac                = "00:50:56:29:00:bb"
            name               = "demo-net-web"
            type               = "org"
        }
        network {
            adapter_type       = "E1000"
            ip                 = "192.168.2.51"
            ip_allocation_mode = "POOL"
            is_primary         = false
            mac                = "00:50:56:29:00:bc"
            name               = "demo-vapp-net-isolated"
            type               = "vapp"
        }
      + network {
          + ip_allocation_mode = "DHCP"
          + name               = "demo-vapp-net-w-org"
          + type               = "vapp"
        }

        override_template_disk {
            bus_number      = 0
            bus_type        = "parallel"
            iops            = 0
            size_in_mb      = 11112
            storage_profile = "*"
            unit_number     = 0
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

data.vcd_vapp.demo_vapp: Refreshing state...
data.vcd_edgegateway.demo-gw: Refreshing state...
vcd_vapp_vm.demo_vm_wordpress: Modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 10s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 20s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 30s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 40s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 50s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 1m0s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 1m10s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 1m20s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 1m30s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 1m40s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 1m50s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 2m0s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 2m10s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 2m20s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 2m30s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 2m40s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 2m50s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a, 3m0s elapsed]
vcd_vapp_vm.demo_vm_wordpress: Modifications complete after 3m10s [id=urn:vcloud:vm:bc7b07c4-9013-4b02-8c70-46aaf331693a]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

dataclouder

Some changes required

dataclouder · 2020-03-09T16:48:54Z

vcd/resource_vcd_vapp_network.go

@@ -515,7 +515,7 @@ func resourceVcdVappNetworkImport(d *schema.ResourceData, meta interface{}) ([]*
 	}

 	if vappNetworkToImport.Configuration.FenceMode == types.FenceModeBridged ||
-		(vappNetworkToImport.Configuration.FenceMode == types.FenceModeNAT && vappNetworkToImport.Configuration.Features.DhcpService == nil) {
+		(vappNetworkToImport.Configuration.FenceMode == types.FenceModeNAT && vappNetworkToImport.Configuration.IPScopes.IPScope[0].IsInherited == true) {


We should check for vappNetworkToImport.Configuration.IPScopes to be not nil , and for vappNetworkToImport.Configuration.IPScopes.IPScope to have elements

it always return, but added

dataclouder · 2020-03-09T16:49:39Z

vcd/resource_vcd_vapp_org_network.go

@@ -300,7 +300,7 @@ func resourceVcdVappOrgNetworkImport(d *schema.ResourceData, meta interface{}) (
 	}

 	if vappNetworkToImport.Configuration.FenceMode == types.FenceModeIsolated ||
-		(vappNetworkToImport.Configuration.FenceMode == types.FenceModeNAT && vappNetworkToImport.Configuration.Features.DhcpService != nil) {
+		(vappNetworkToImport.Configuration.FenceMode == types.FenceModeNAT && vappNetworkToImport.Configuration.IPScopes.IPScope[0].IsInherited == false) {


We should check for vappNetworkToImport.Configuration.IPScopes to be not nil , and for vappNetworkToImport.Configuration.IPScopes.IPScope to have elements

Also wondering what is the impact (or maybe none) of NetworkConfiguration.ParentNetwork

it always return, but added

dataclouder · 2020-03-09T16:51:04Z

vcd/resource_vcd_vapp_vm.go

+		if networkConfig.NetworkName == vAppNetworkName &&
+			networkConfig.Configuration.FenceMode == types.FenceModeIsolated ||
+			(networkConfig.Configuration.FenceMode == types.FenceModeNAT &&
+				networkConfig.Configuration.IPScopes.IPScope[0].IsInherited == false) {


We should check for networkConfig.Configuration.IPScopes to be not nil , and for networkConfig.Configuration.IPScopes.IPScope to have elements

Yes, and also this check needs an explanation comment, explaining what is the meaning of it.

it always return, but added

dataclouder · 2020-03-09T16:53:49Z

vcd/resource_vcd_vapp_vm_multinetwork_test.go

+  org = "{{.Org}}"
+  vdc = "{{.Vdc}}"
+
+  vapp_name  = vcd_vapp.{{.VAppName}}.name


This HCL block is not formatted

Didainius

Good one :) Also changelog needs a note or at least mentioning this PR on the other attachment resources.

vcd/resource_vcd_vapp_network.go

Didainius · 2020-03-09T18:32:41Z

vcd/resource_vcd_vapp_org_network.go

@@ -300,7 +300,7 @@ func resourceVcdVappOrgNetworkImport(d *schema.ResourceData, meta interface{}) (
 	}

 	if vappNetworkToImport.Configuration.FenceMode == types.FenceModeIsolated ||
-		(vappNetworkToImport.Configuration.FenceMode == types.FenceModeNAT && vappNetworkToImport.Configuration.Features.DhcpService != nil) {
+		(vappNetworkToImport.Configuration.FenceMode == types.FenceModeNAT && vappNetworkToImport.Configuration.IPScopes.IPScope[0].IsInherited == false) {


Also wondering what is the impact (or maybe none) of NetworkConfiguration.ParentNetwork

vcd/resource_vcd_vapp_org_network_test.go

vcd/resource_vcd_vapp_vm_multinetwork_test.go

vcd/resource_vcd_vapp_vm.go

Didainius · 2020-03-09T18:41:40Z

vcd/resource_vcd_vapp_vm.go

+		if networkConfig.NetworkName == vAppNetworkName &&
+			networkConfig.Configuration.FenceMode == types.FenceModeIsolated ||
+			(networkConfig.Configuration.FenceMode == types.FenceModeNAT &&
+				networkConfig.Configuration.IPScopes.IPScope[0].IsInherited == false) {


Yes, and also this check needs an explanation comment, explaining what is the meaning of it.

Signed-off-by: Vaidotas Bauzys <[email protected]>

lvirbalas · 2020-03-10T06:55:33Z

CHANGELOG.md

+* `vcd_vapp_vm` allows to add routed vApp network, not only isolated one. `network.name` can be reference 
+`vcd_vapp_network.name` [GH-472]


Suggested change

* `vcd_vapp_vm` allows to add routed vApp network, not only isolated one. `network.name` can be reference

`vcd_vapp_network.name` [GH-472]

* `vcd_vapp_vm` allows to add routed vApp network, not only isolated one. `network.name` can reference

`vcd_vapp_network.name` of a vApp network with `org_network_name` set [GH-472]

vcd/resource_vcd_vapp_org_network.go

Signed-off-by: Vaidotas Bauzys <[email protected]>

lvirbalas

Thank you for a quick & needed addition!

Signed-off-by: Vaidotas Bauzys <[email protected]>

dataclouder

LGTM

vbauzys added 30 commits July 15, 2019 15:39

Add missing file

6585559

Signed-off-by: Vaidotas Bauzys <[email protected]>

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

e247d25

…vider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

cf1e7c2

…roviders/terraform-provider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

be6790f

…vider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

6f4c5cb

…vider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

c73d402

…vider-vcd

git push origin master

261d142

Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

c431d43

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

414b3bf

…roviders/terraform-provider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

8b03ab8

…vider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

36c5487

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

db01eec

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

331e895

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

a85d555

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

e1996e3

…roviders/terraform-provider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

9c63eec

…vider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

5073f6c

…roviders/terraform-provider-vcd

git push origin master

f0bdef3

Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

24f169d

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

06b36d1

…roviders/terraform-provider-vcd

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

30d6a79

…vider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

0ca001f

…roviders/terraform-provider-vcd

git push origin master

8c9c7c4

Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

4c8d081

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

206bc9f

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

48970bc

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

e7ef092

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

cb5659a

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

1e097fd

…roviders/terraform-provider-vcd

git push origin masterMerge branch 'master' of github.com:terraform-p…

ae2d955

…roviders/terraform-provider-vcd

Added possibility to reference vapp routed network in VM.network

e1f94cc

Signed-off-by: Vaidotas Bauzys <[email protected]>

vbauzys self-assigned this Mar 9, 2020

ghost added the size/M label Mar 9, 2020

vbauzys changed the title ~~Vm vapp netw fix~~ Improved VM network to allow reference vApp routed network Mar 9, 2020

vbauzys requested review from dataclouder, Didainius and lvirbalas and removed request for dataclouder and Didainius March 9, 2020 16:22

lvirbalas requested changes Mar 9, 2020

View reviewed changes

vcd/resource_vcd_vapp_network.go Outdated Show resolved Hide resolved

vcd/resource_vcd_vapp_vm_multinetwork_test.go Show resolved Hide resolved

dataclouder suggested changes Mar 9, 2020

View reviewed changes

Didainius reviewed Mar 9, 2020

View reviewed changes

Improvements

760ec38

Signed-off-by: Vaidotas Bauzys <[email protected]>

ghost added size/L and removed size/M labels Mar 10, 2020

vbauzys added 2 commits March 10, 2020 07:52

Add changelog

403342e

Signed-off-by: Vaidotas Bauzys <[email protected]>

Fix

ff428e1

Signed-off-by: Vaidotas Bauzys <[email protected]>

lvirbalas reviewed Mar 10, 2020

View reviewed changes

vbauzys added 4 commits March 10, 2020 09:46

improve comments

9b22758

Signed-off-by: Vaidotas Bauzys <[email protected]>

improve changelog

edae5b3

Signed-off-by: Vaidotas Bauzys <[email protected]>

Removed as don't work with org user and other tests

4b1fd6b

Signed-off-by: Vaidotas Bauzys <[email protected]>

Removed as don't work with org user and other tests

79b13df

Signed-off-by: Vaidotas Bauzys <[email protected]>

lvirbalas approved these changes Mar 10, 2020

View reviewed changes

Add test improvement - removed dependency on existing network

8d66cae

Signed-off-by: Vaidotas Bauzys <[email protected]>

dataclouder approved these changes Mar 10, 2020

View reviewed changes

Didainius approved these changes Mar 10, 2020

View reviewed changes

vbauzys merged commit a1a3384 into vmware:master Mar 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved VM network to allow reference vApp routed network #472

Improved VM network to allow reference vApp routed network #472

vbauzys commented Mar 9, 2020

lvirbalas left a comment

lvirbalas commented Mar 9, 2020

dataclouder left a comment

dataclouder Mar 9, 2020

vbauzys Mar 10, 2020

dataclouder Mar 9, 2020

Didainius Mar 9, 2020

vbauzys Mar 10, 2020

dataclouder Mar 9, 2020

Didainius Mar 9, 2020

vbauzys Mar 10, 2020

dataclouder Mar 9, 2020

vbauzys Mar 10, 2020

Didainius left a comment

Didainius Mar 9, 2020

Didainius Mar 9, 2020

lvirbalas Mar 10, 2020

vbauzys Mar 10, 2020

lvirbalas left a comment

dataclouder left a comment

		* `vcd_vapp_vm` allows to add routed vApp network, not only isolated one. `network.name` can be reference
		`vcd_vapp_network.name` [GH-472]

Improved VM network to allow reference vApp routed network #472

Improved VM network to allow reference vApp routed network #472

Conversation

vbauzys commented Mar 9, 2020

lvirbalas left a comment

Choose a reason for hiding this comment

lvirbalas commented Mar 9, 2020

dataclouder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Didainius left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lvirbalas left a comment

Choose a reason for hiding this comment

dataclouder left a comment

Choose a reason for hiding this comment