Skip to content
This repository has been archived by the owner on Oct 31, 2019. It is now read-only.

This allows the user to specify an existing VCN instead of it being created #171

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

garthy
Copy link
Member

@garthy garthy commented Mar 21, 2018

simple specify the 3 extra parameters

VCN

vcn_id = "ocid1.vcn.oc1.phx.aaaaaaaa545hjqe26s77xpiiuyznb6baxym5ff6lnx5asgggnptwfcg3t7na"
dhcp_options_id = "ocid1.dhcpoptions.oc1.phx.aaaaaaaat27e4e6li545u4tnymwk7452gpxvqcszg6jbflpfqvtr7vkxjqnq"
public_routetable_id = "ocid1.routetable.oc1.phx.aaaaaaaab5obbsciewyf73r4ggm26uvmykqbyhwphwqiuvqpune3bosoyey

@garthy garthy self-assigned this Mar 21, 2018
@@ -112,6 +112,7 @@ The following input variables are used to configure the inbound security rules o
name | default | description
------------------------------------|-------------------------|------------
network_cidrs | See map in variables.tf | A CIDR notation IP range of the VCN and its subnets.
network_subnet_dns | See map in variables.tf | A DNS label for each of the subnet in the VCN (Max 15 characters)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A DNS label for each of the subnet in the VCN => A DNS label for each of the subnets in the VCN

k8s-oci.tf Outdated
@@ -31,10 +16,29 @@ module "k8s-tls" {
### Virtual Cloud Network

module "vcn" {
create_vcn = "${var.vcn_id == "" ? 1 : 0}"
Copy link
Member

@owainlewis owainlewis Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clever since the value is used later but from reading the code it feels like this should be a boolean and the later expressions later should reference it

count = "${var.create_vcn ? "1" : "0"}"

The above feels more consistent with how count is derived elsewhere in the code i.e in resource "oci_core_route_table" "NATInstanceAD1RouteTable"

@garthy garthy requested a review from tjfontaine March 21, 2018 14:23
@jan-g
Copy link
Contributor

jan-g commented Mar 21, 2018

This looks good to me; certainly works.

Do we need more docs around the constraints of using it with an existing VCN?

  • subnet ranges must not overlap with existing subnets
  • DNS labels must not clash with existing labels.

Copy link
Contributor

@tjfontaine tjfontaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to also handle the case where the user only has the use permission, which prevents them from creating subnets (or security lists, or route tables), but allows them to launch instances.

This is probably more common than the case of knowing the destination VCN but not already having the Subnets ready.

@garthy
Copy link
Member Author

garthy commented Mar 26, 2018

@tjfontaine I've moved this to a new feature request. #178 where we can discuss this new requirement.

@beide
Copy link

beide commented Apr 25, 2018

I add the new variables in the tfvar files but new vcn still created. Thanks,

@beide
Copy link

beide commented Apr 26, 2018

Is this feature ready for me to use? I tried it yesterday and it did not work. should I try it again? Thanks,

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants