-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add vApp network resources and data sources #455
Conversation
Signed-off-by: Vaidotas Bauzys <[email protected]>
…roviders/terraform-provider-vcd
Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
Merge branch 'master' of github.com:terraform-providers/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
…roviders/terraform-provider-vcd
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
|
||
## Attribute reference | ||
|
||
All attributes defined in [vApp network resource](/docs/providers/vcd/r/vapp_network.html#attribute-reference) are supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"[vApp network resource]" should be exact resource name as in similar references in pages of other data sources. Same applies to the other data source page as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation clean up
|
||
# vcd\_vapp\_org\_network | ||
|
||
Provides a data source for vCloud director Org network attached to vApp.. This can be used to access vApp Org VDC network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provides a data source for vCloud director Org network attached to vApp.. This can be used to access vApp Org VDC network. | |
Provides a data source for vCloud director Org network attached to vApp. This can be used to access vApp Org VDC network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
# vcd\_vapp\_network | ||
|
||
Provides a vCloud Director vApp network data source. This can be used to access vApp network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provides a vCloud Director vApp network data source. This can be used to access vApp network. | |
Provides a vCloud Director vApp network data source. This can be used to access a vApp network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
@@ -3,13 +3,12 @@ layout: "vcd" | |||
page_title: "vCloudDirector: vcd_vapp_network" | |||
sidebar_current: "docs-vcd-resource-vapp-network" | |||
description: |- | |||
Provides a vCloud Director vApp isolated Network. This can be used to create and delete internal networks for vApps to connect. | |||
Allows to provision vApp network and optionally connect it to existing Org VDC network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows to provision vApp network and optionally connect it to existing Org VDC network. | |
Allows to provision a vApp network and optionally connect it to an existing Org VDC network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
--- | ||
|
||
# vcd\_vapp\_network | ||
|
||
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. | ||
Allows to provision vApp network and optionally connect it to existing Org VDC network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows to provision vApp network and optionally connect it to existing Org VDC network. | |
Allows to provision a vApp network and optionally connect it to an existing Org VDC network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
* `dns_suffix` - (Optional) A FQDN for the virtual machines on this network. | ||
* `guest_vlan_allowed` (Optional) True if Network allows guest VLAN tagging. This value supported from vCD version 9.0 | ||
* `static_ip_pool` - (Optional) A range of IPs permitted to be used as static IPs for virtual machines; see [IP Pools](#ip-pools) below for details. | ||
* `dhcp_pool` - (Optional) A range of IPs to issue to virtual machines that don't have a static IP; see [IP Pools](#ip-pools) below for details. | ||
* `org_network_name` - (Optional; *v2.7+*) An Org network name to which vApp network is connected to. If not configured, then isolated network created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `org_network_name` - (Optional; *v2.7+*) An Org network name to which vApp network is connected to. If not configured, then isolated network created. | |
* `org_network_name` - (Optional; *v2.7+*) An Org network name to which vApp network is connected. If not configured, then an isolated network is created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
|
||
# vcd\_vapp\_org\_network | ||
|
||
Provides capability to attach existing Org VDC Network to vApp and toggle network features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provides capability to attach existing Org VDC Network to vApp and toggle network features. | |
Provides capability to attach an existing Org VDC Network to a vApp and toggle network features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
* `org` - (Optional) The name of organization to use, optional if defined at provider level. Useful when | ||
connected as sysadmin working across different organisations. | ||
* `vdc` - (Optional) The name of VDC to use, optional if defined at provider level. | ||
* `vapp_name` - (Required) The vApp this VM should belong to. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `vapp_name` - (Required) The vApp this VM should belong to. | |
* `vapp_name` - (Required) The vApp this network belongs to. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
--- | ||
|
||
# vcd\_vapp\_network | ||
|
||
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. | ||
Allows to provision vApp network and optionally connect it to existing Org VDC network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need an example below how to "connect it to existing Org VDC network". What about adding these lines to the example below:
# Comment below line to create an isolated vApp network
org_network_name = "my-org-network"
BTW, please polish from "#Optional" to "# Optional" in the example too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
vdc = "my-vdc" # Optional | ||
|
||
vapp_name = "my-vapp" | ||
# Comment below line to create an isolated vApp network |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a newline above to make the new parameter (and comment) stand-out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
* **New Resource:** `vcd_vapp_org_network` vApp organization network [GH-455] | ||
* `vcd_vapp_network` supports isolated network and vApp network connected to Org VDC networks [GH-455] | ||
* **New Data Source:** `vcd_vapp_org_network` vApp org network [GH-455] | ||
* **New Data Source:** `vcd_vapp_network` vApp network [GH-455] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a "DEPRECATIONS:" section with the note about behavior we're deprecating of VM creating vApp Org Network if it doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
vcd/resource_vcd_vapp_vm.go
Outdated
@@ -589,6 +589,9 @@ func addVdcNetwork(networkNameToAdd string, vdc *govcd.Vdc, vapp govcd.VApp, vcd | |||
} | |||
|
|||
if !isAlreadyVappNetwork { | |||
// TODO remove when major release is done | |||
_, _ = fmt.Fprintf(getTerraformStdout(), "DEPRECATED: attaching an Org network `%s` to a vApp `%s` through VM's network block alone is deprecated. "+ | |||
"Network should be first attached to a vApp by creating a vcd_vapp_org_network resource and only then referenced in the network block. \n", networkNameToAdd, vapp.VApp.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's quote vcd_vapp_org_network word too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ability to control all kinds of vApp networks are great additions and it's especially nice that they auto-address #264 too. The fact that we can now create and destroy vApp networks at a separate resource level, as opposed to the old way of inside the VM, makes the workflow and responsibilities of vApp vs. VM clearly defined. BTW, please close #264 together with the issue of this PR, unless you see otherwise.
I have added two more small requests to this PR, LGTM otherwise!
p.s. I was having issues with destroy workflows, but they turned out to be because of https://github.com/terraform-providers/terraform-provider-vcd/issues/464
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
CHANGELOG.md
Outdated
@@ -18,6 +22,10 @@ IMPROVEMENTS: | |||
* `vcd_vapp_vm` `disk` has new attribute `size_in_mb` [GH-433] | |||
* `datasource/*` - all data sources return an error when object is not found [GH-446] | |||
|
|||
DEPRECATIONS: | |||
* `resource/vcd_vapp_vm` `network.name` deprecated creation/attaching vApp network when doesn't exist. Requires to create/attach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `resource/vcd_vapp_vm` `network.name` deprecated creation/attaching vApp network when doesn't exist. Requires to create/attach | |
* `resource/vcd_vapp_vm` `network.name` deprecate automatic creation/attaching of a vApp network when it doesn't exist. Requires to create/attach |
Also, isn't this deprecation actual only for type
= org
? This message needs to be adjusted to reflect that. Now you can read it as if even isolated vApp network is being auto-created, which is not.
Also, the comment looks to be split into two lines needlessly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adjusted
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
@@ -49,15 +48,20 @@ The following arguments are supported: | |||
connected as sysadmin working across different organisations. | |||
* `vdc` - (Optional; *v2.0+*) The name of VDC to use, optional if defined at provider level. | |||
* `name` - (Required) A unique name for the network. | |||
* `vapp_name` - (Required) The vApp this VM should belong to. | |||
* `description` - (Optional; *v2.7+*) Description of vApp network | |||
* `vapp_name` - (Required) The vApp this network belongs to. | |||
* `netmask` - (Optional) The netmask for the new network. Default is `255.255.255.0`. | |||
* `gateway` (Optional) The gateway for this network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is not right here. So, after reading the new changelog message:
DEPRECATIONS:
resource/vcd_vapp_vm
network.name
deprecated automatic attaching of vApp Org network whennetwork.type=org
and it doesn't exist. Requires to create/attach vApp network withvcd_vapp_network
orvcd_vapp_org_network
before referencing it.
I interpret that I should be able to create the needed vApp network not only with vcd_vapp_org_network
, but also with vcd_vapp_network
. So, I'm trying this:
resource "vcd_vapp_network" "demo_vapp_net_w_org" {
name = "demo-vapp-net-w-org"
vapp_name = vcd_vapp.demo_vapp.name
org_network_name = vcd_network_routed.demo_routed_net_multi_pool.name
}
and then in VM:
network {
type = "org"
# 2.7.0 will throw a warning if vApp Network doesn't exist and it auto-creates it
name = vcd_vapp_network.demo_vapp_net_w_org.org_network_name
ip_allocation_mode = "DHCP"
}
However, it says that gateway
is required (which would conflict with the highlighted doc line of this comment):
$ terraform apply
Error: Missing required argument
on xendi.tf line 320, in resource "vcd_vapp_network" "demo_vapp_net_w_org":
320: resource "vcd_vapp_network" "demo_vapp_net_w_org" {
The argument "gateway" is required, but no definition was found.
So, I add the whole network definition:
resource "vcd_vapp_network" "demo_vapp_net_w_org" {
name = "demo-vapp-net-w-org"
vapp_name = vcd_vapp.demo_vapp.name
org_network_name = vcd_network_routed.demo_routed_net_multi_pool.name
gateway = "192.168.9.1"
netmask = "255.255.255.0"
dns1 = "192.168.9.1"
dns2 = "192.168.9.2"
dns_suffix = "mybiz.biz"
guest_vlan_allowed = true
# 2.7.0
#org_network_name = vcd_network_routed.demo_routed_net_multi_pool.name
static_ip_pool {
start_address = "192.168.9.51"
end_address = "192.168.9.100"
}
dhcp_pool {
start_address = "192.168.9.2"
end_address = "192.168.9.50"
}
}
And run apply, but then I get the DEPRECATED message even though I did create the vcd_vapp_network
as per your changelog message:
vcd_vapp_network.demo_vapp_net_w_org: Creating...
vcd_vapp_network.demo_vapp_net_w_org: Still creating... [10s elapsed]
vcd_vapp_network.demo_vapp_net_w_org: Still creating... [20s elapsed]
vcd_vapp_network.demo_vapp_net_w_org: Creation complete after 25s [id=b68affa0-c540-46c9-a2ec-0180041c8973]
vcd_vapp_vm.demo_vm_wordpress: Modifying... [id=urn:vcloud:vm:ef2bb818-6462-4db5-bd71-0df42cdec2ad]
vcd_vapp_vm.demo_vm_wordpress: Still modifying... [id=urn:vcloud:vm:ef2bb818-6462-4db5-bd71-0df42cdec2ad, 10s elapsed]
DEPRECATED: attaching an Org network `demo-net-multi_pool` to a vApp `demo-web` through VM's network block alone is deprecated. Network should be first attached to a vApp by creating a `vcd_vapp_org_network` resource and only then referenced in the network block.
Finally, the result of such config is that I get two networks in the vApp! My expected one "demo-vapp-net-w-org " from vcd_vapp_network
, but also one "demo-net-multi_pool" auto-created by the VM:
And, the VM did not connect to the "demo_vapp_net_w_org":
I believe the changelog message is incorrect and should only reference the vcd_vapp_org_network
, but I will leave it up to you to confirm.
CC: @Didainius
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, only vcd_vapp_org_network
. My aim was to tell everyone to use new resource more explicitly for every case. I doubt anyone have understood or noticed that automagic
which existed.
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved!
I've got some last minute issues with TestAccVcdVAppVmDhcpWait
test on some environments, but it looks to be related to the env stability in general as I've checked and VM simply didn't get DHCP lease, so the reporting test failed. Never fails on some other envs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Depends on vmware/go-vcloud-director#290
Ref: #329, #447, #264
vcd_vapp_org_network
- vApp organization networkvcd_vapp_org_network
new data sourcevcd_vapp_network
new data source