Add vapp network resource #155

vbauzys · 2019-01-31T11:27:21Z

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

New capability to create vapp network which will be isolated.
Example of usage:

resource "vcd_vapp_network" "terraform1" {
  name = "terraform1"

  vapp_name          = "vApp_system_1"
  gateway            = "192.168.2.1"
  netmask            = "255.255.255.0"
  dns1               = "192.168.2.1"
  dns2               = "192.168.2.2"
  dns_suffix         = "mano.biz"
  guest_vlan_allowed = true

  static_ip_pool {
    start_address = "192.168.2.5"
    end_address   = "192.168.2.10"
  }
}

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

spengilley · 2019-01-31T15:42:17Z

I'm waiting on this with relish :)

vbauzys · 2019-02-01T04:28:16Z

You may try and provide feedback :)

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

lvirbalas · 2019-02-01T07:57:50Z

The overall implementation looks quite nice. Three open (all related) conceptual questions though:

1.) Should we keep name of the resource vcd_vapp_network or vcd_vapp_network_isolated?

As a reference, we have three types for Org VDC networks:

vcd_network_direct
vcd_network_isolated
vcd_network_routed

2.) How would you suggest modelling (in .tf) a vApp network which is connected to an Org VDC Network?

3.) How would you suggest modelling (in .tf) vApp network's services (DHCP, Firewall, NAT, Static Routing)?

vbauzys · 2019-02-04T07:19:17Z

Answers:

According vApp UI you can have isolated(when create adding new vapp network), natRouted when use connection dropdown and bridged when add exisitng org vdc network. So I recommend to stick with isolated for name.
There are a few possibilities:

vcd_vapp_network Resource allows to configure connection with parent network and DHCP, NAT, Firewall, static routing - as such makes it very complex.
vcd_vapp_network Resource allows to configure connection with parent network and additional resources or resources configure DHCP, NAT, Firewall, static routing for that network

I suggest to use existing configuration principles which we have already for DHCP, Firewall, NAT in other places

lvirbalas · 2019-02-04T09:27:01Z

We came to the following summary. In this PR:

Have a single vcd_vapp_network resource. This implements the isolated vApp network type.

In further PRs:

Add three new parameters to vcd_vapp_network which allow defining a routed vApp network:

resource "vcd_vapp_network" "VAppNetwork" {
  name         = "vapp-net"
  vapp_name = "vapp-test-net"

  # Parameters (all optional) which allow connecting this vApp network to the Org VDC network
  network = “org-vdc-net1” 
  nat_enabled = “true”
  firewall_enabled = “true”
...
}

Add new resources two manage Firewall, NAT and Static Routing at vApp network level:

vcd_vapp_dnat
vcd_vapp_snat
vcd_vapp_firewall_rule
vcd_vapp_static_route

lvirbalas

After discussing implementation of isolated vs routed vApp network I realised that the PR is missing the DHCP definition. DHCP configuration applies to both routed and isolated networks. We need that here.

For reference, Org VDC Network has it declared as follows:
https://www.terraform.io/docs/providers/vcd/r/network_routed.html

spengilley · 2019-02-05T08:42:22Z

My use case is that I have an isolated vApp network, but also I want the vApp connected to 2 other vOrg level networks. That doesn't appear possible in the above solution. Will this PR be done in concert with #74 ?

lvirbalas · 2019-02-05T10:17:44Z

@spengilley This PR is for vApp isolated network only. Multiple network support will be handled separately, hopefully, as part of #74

dataclouder

Some changes required. Will run a test once all the change requests have been addressed

dataclouder · 2019-02-05T12:00:04Z

vcd/resource_vcd_vapp_network_test.go

+}
+
+resource "vcd_vapp_network" "{{.resourceName}}" {
+  org          = "{{.Org}}"


This snippet is not formatted correctly. The properties should be aligned.

dataclouder · 2019-02-05T12:06:39Z

vcd/resource_vcd_vapp_vm.go

@@ -238,6 +210,81 @@ func resourceVcdVAppVmCreate(d *schema.ResourceData, meta interface{}) error {
 	return resourceVcdVAppVmUpdate(d, meta)
 }

+// Adds existing org VDC network to VM network configuration
+func addVdcNetwork(d *schema.ResourceData, vdc govcd.Vdc, vapp govcd.VApp, vcdClient *VCDClient) ([]*types.OrgVDCNetwork, string, error) {


What is this function returning?
Either add the definition of the return values in a comment or use named return types to show what we are returning.
For example:

func addVdcNetwork(d *schema.ResourceData, vdc govcd.Vdc, vapp govcd.VApp, vcdClient *VCDClient) (vdcNetworks []*types.OrgVDCNetwork, networkName string, err error)

dataclouder · 2019-02-05T12:08:42Z

vcd/resource_vcd_vapp_network.go

+
+	err = task.WaitTaskCompletion()
+	if err != nil {
+		return fmt.Errorf("error waiting from task to complete: %+v", err)


"waiting from task" -> "waiting for task"

dataclouder · 2019-02-05T12:09:09Z

vcd/resource_vcd_vapp_network.go

+
+	vAppNetworkConfig, err := vapp.GetNetworkConfig()
+	if err != nil {
+		return fmt.Errorf("error getting vAPP networks: %#v", err)


"vAPP" -> "vApp"

dataclouder · 2019-02-05T12:10:36Z

vcd/resource_vcd_vapp_network.go

+
+	err = task.WaitTaskCompletion()
+	if err != nil {
+		return fmt.Errorf("error waiting from task to complete: %+v", err)


"waiting from task" -> "waiting for task"

vcd/resource_vcd_vapp_network_test.go

dataclouder · 2019-02-05T12:12:39Z

vcd/resource_vcd_vapp_vm.go

+	} else {
+
+		if netName == "" {
+			return []*types.OrgVDCNetwork{}, "", fmt.Errorf("'network_name' must be valid when adding VM to raw vapp")


"vapp" -> "vApp"

dataclouder · 2019-02-05T12:13:24Z

vcd/resource_vcd_vapp_vm.go

+	}
+
+	if vAppNetworkName != netName {
+		return []*types.OrgVDCNetwork{}, "", fmt.Errorf("the VDC network '%s' must be assigned to the vApp. Currently the vApp network date is %s", netName, vAppNetworkName)


dataclouder · 2019-02-05T12:13:51Z

vcd/resource_vcd_vapp_vm.go

+
+	for _, networkConfig := range vAppNetworkConfig.NetworkConfig {
+		if networkConfig.NetworkName == vAppNetworkName {
+			log.Printf("[TRACE] Vapp network found: %s", vAppNetworkName)


"Vapp" -> "vApp"

dataclouder · 2019-02-05T12:14:06Z

vcd/resource_vcd_vapp_vm.go

+		}
+	}
+
+	return "", fmt.Errorf("configured vAPP network isn't found: %#v", err)


"vAPP" -> "vApp"

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

dn1s · 2019-02-05T16:20:22Z

vcd/resource_vcd_vapp_vm.go

@@ -238,6 +210,82 @@ func resourceVcdVAppVmCreate(d *schema.ResourceData, meta interface{}) error {
 	return resourceVcdVAppVmUpdate(d, meta)
 }

+// Adds existing org VDC network to VM network configuration
+// Returns configured OrgVDCNetwork for Vm, networkName, error if any occur
+func addVdcNetwork(d *schema.ResourceData, vdc govcd.Vdc, vapp govcd.VApp, vcdClient *VCDClient) (vdcNetworks []*types.OrgVDCNetwork, networkName string, err error) {


I don't agree with having this function added to this code base it should be in underlying go-vcloud-director library.

I agree regarding this and refactor will happen outside this PR. We will review vapp,vm and network functionality which is quite old code and functionality wise.

dn1s · 2019-02-05T16:22:01Z

vcd/resource_vcd_vapp_vm.go

+	return vdcNetworks, netName, nil
+}
+
+// Adds existing org vApp network to VM network configuration


Same here I don't agree with having this function added to this code base it should be in underlying go-vcloud-director library.

This functionality in latest commits was changed and just works as check.

dn1s · 2019-02-05T16:33:48Z

Let me propose some changes against your repo @vbauzysvmware which in my opinion would be better.

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

vbauzys · 2019-02-11T15:14:25Z

Add requested DHCP pool capabilities.
Add support for adding network when already exist others.

lvirbalas

Ask to update one comment in docs about the version, and then it's time to get this code to the branch and try everything out!

lvirbalas · 2019-02-12T06:09:20Z

website/docs/r/vapp_network.html.markdown

+ Provides a vCloud Director vApp isolated Network. This can be used to create and delete internal networks for vApps to connect.
+ This network is not attached to external networks or routers.
+
+Supported in provider *v2.0+*


Given that we are following semver, this new feature will bump the minor number, so we can point that in the comment above:
Supported in provider *v2.1+*

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

lvirbalas · 2019-02-12T07:46:22Z

website/docs/r/vapp_vm.html.markdown

@@ -69,6 +69,7 @@ The following arguments are supported:
 * `cpus` - (Optional) The number of virtual CPUs to allocate to the vApp
 * `initscript` (Optional) A script to be run only on initial boot
 * `network_name` - (Optional) Name of the network this VM should connect to
+* `vapp_network_name` - ((Optional; *v2.1*)) Name of the vApp network this VM should connect to


Good addition. However, too many brackets and missing plus. Should be:
(Optional; *v2.1+*)

Good spot, fixed that

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

jgreenback · 2019-09-11T17:31:20Z

The below quote mentioned features that would be great to see for those of us vCD Terraform users who rely heavily on routed vapp networks (Enterprise vCD user). Any estimates on when these features will make it into a release?

We came to the following summary. In this PR:

Have a single vcd_vapp_network resource. This implements the isolated vApp network type.

In further PRs:

Add three new parameters to vcd_vapp_network which allow defining a routed vApp network:
resource "vcd_vapp_network" "VAppNetwork" {
  name         = "vapp-net"
  vapp_name = "vapp-test-net"

  # Parameters (all optional) which allow connecting this vApp network to the Org VDC network**
  network = “org-vdc-net1”
  nat_enabled = “true”
  firewall_enabled = “true”
...
}
Add new resources two manage Firewall, NAT and Static Routing at vApp network level:

vcd_vapp_dnat

vcd_vapp_snat

vcd_vapp_firewall_rule

vcd_vapp_static_route

lvirbalas · 2019-09-12T07:17:27Z

Hi @jgreenback , I created a separate issue to track this. Don't hesitate to comment there too:
https://github.com/terraform-providers/terraform-provider-vcd/issues/329

It's not planned for the current release, but is in the backlog as such.
BTW, which company are you coming from?

vbauzys · 2019-09-12T07:50:35Z

Hi @jgreenback would be nice if you attach on https://github.com/terraform-providers/terraform-provider-vcd/issues/329 some screenshot which resembles your business need :)

jgreenback · 2019-09-16T23:26:42Z

Thanks @lvirbalas / @vbauzysvmware for looking at this; I'll work on additional details as well as provide visuals for: #329

@lvirbalas; coming from Varian Medical Systems Inc.

vbauzys added 3 commits January 30, 2019 12:47

Update vendor govcd

8f7ecc3

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

Add new implementation for resource to create vApp network

7880f41

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

Add documentation for vApp network resource

e6cf3c0

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

ghost added size/XL dependencies documentation labels Jan 31, 2019

vbauzys added 2 commits January 31, 2019 15:23

Added support for using vapp network for NIC

aec7173

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

Updated documentation

f38d8e4

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

Updated documentation

dc970af

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

vbauzys requested review from lvirbalas and dataclouder February 1, 2019 05:48

lvirbalas requested changes Feb 4, 2019

View reviewed changes

lvirbalas mentioned this pull request Feb 5, 2019

[WIP] Merge in code as per v2Plan.md #74

Closed

dataclouder suggested changes Feb 5, 2019

View reviewed changes

vbauzys added 8 commits February 5, 2019 14:46

Fix mistype in vApp

fa4252b

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

Fix grammar and add comments

0126830

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

Updated formatting for tf part

7beb690

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

Rename file due mistype

b608093

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

Improve doc grammar

229abf0

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

Improve doc grammar

6e8dc52

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

Improve doc grammar

6688634

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

Improve doc grammar

c36c145

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

dataclouder approved these changes Feb 5, 2019

View reviewed changes

dn1s reviewed Feb 5, 2019

View reviewed changes

vbauzys added 3 commits February 6, 2019 15:25

Update vendor govcd

53ac367

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

Update vendor govcd

1b19700

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

Add DHCP pool support

1c9b050

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

ghost added size/XXL and removed size/XL labels Feb 11, 2019

Fix code to support networks if vApp already have ones.

d6eff94

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

lvirbalas approved these changes Feb 12, 2019

View reviewed changes

vbauzys added 3 commits February 12, 2019 08:23

Update version in docs

191ea7b

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

Add missing code

4505bd8

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

Fix error

337cc8d

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

lvirbalas reviewed Feb 12, 2019

View reviewed changes

Fix docs

6d44d4b

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

vbauzys merged commit 5cab61f into vmware:vmware-stash-while-master-pr-open Feb 12, 2019

lvirbalas mentioned this pull request Mar 22, 2019

Spring is here - vCD Provider 2.1.0 #200

Merged

lvirbalas mentioned this pull request Sep 12, 2019

Routed vApp Network #329

Closed

Add vapp network resource #155

Add vapp network resource #155

Conversation

vbauzys commented Jan 31, 2019 • edited by dataclouder Loading

spengilley commented Jan 31, 2019

vbauzys commented Feb 1, 2019

lvirbalas commented Feb 1, 2019

vbauzys commented Feb 4, 2019

lvirbalas commented Feb 4, 2019

lvirbalas left a comment

Choose a reason for hiding this comment

spengilley commented Feb 5, 2019

lvirbalas commented Feb 5, 2019

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

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

dn1s Feb 5, 2019 • edited Loading

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

dn1s commented Feb 5, 2019

vbauzys commented Feb 11, 2019

lvirbalas 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

jgreenback commented Sep 11, 2019

lvirbalas commented Sep 12, 2019

vbauzys commented Sep 12, 2019

jgreenback commented Sep 16, 2019

vbauzys commented Jan 31, 2019 •

edited by dataclouder

Loading

dn1s Feb 5, 2019 •

edited

Loading