From 6ec0d5a3afd3c38377292fe8a539ad2ef8adf351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87etin=20ARDAL?= Date: Fri, 17 Sep 2021 18:46:26 +0200 Subject: [PATCH] fix(reserved IP): add feedback from PR#71 review - fix assign_public_ip test condition - rename terraform resources from to more meaningful name --- .pre-commit-config.yaml | 15 ------- CHANGELOG.adoc | 4 +- docs/terraformoptions.adoc | 18 ++++---- examples/instances_fixed_shape/main.tf | 8 ++-- examples/instances_fixed_shape/variables.tf | 6 +++ examples/instances_flex_shape/main.tf | 6 +-- examples/instances_flex_shape/variables.tf | 8 ++-- examples/instances_reserved_public_ip/main.tf | 3 +- .../instances_reserved_public_ip/variables.tf | 2 +- main.tf | 42 +++++++++---------- outputs.tf | 24 +++++------ variables.tf | 2 +- 12 files changed, 64 insertions(+), 74 deletions(-) delete mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml deleted file mode 100644 index 953ea22..0000000 --- a/.pre-commit-config.yaml +++ /dev/null @@ -1,15 +0,0 @@ -repos: -- repo: https://github.com/pre-commit/pre-commit-hooks - rev: v3.4.0 # Get the latest from: https://github.com/pre-commit/pre-commit-hooks/releases - hooks: - - id: trailing-whitespace - args: [--markdown-linebreak-ext=md] -- repo: https://github.com/gruntwork-io/pre-commit - rev: v0.1.12 # Get the latest from: https://github.com/gruntwork-io/pre-commit/releases - hooks: - - id: terraform-validate - # - id: terraform-fmt #* this version of tf-fmt detect problem and fails. You have to run tf fmt yourself -- repo: https://github.com/antonbabenko/pre-commit-terraform - rev: v1.46.0 # Get the latest from: https://github.com/antonbabenko/pre-commit-terraform/releases - hooks: - - id: terraform_fmt #* this version of tf-fmt actually runs fmt and fix files \ No newline at end of file diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index b9dc134..1dbbc8a 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -26,8 +26,8 @@ Given a version number MAJOR.MINOR.PATCH: * Add support for freeform and defined tags for instances, vnics and block volumes (Fix #10, #11, #12, #13, #18, #20) * Add "module watermark" freeform tags: module defined and user defined freeform tags are merged on the final resource * Add support to provide the `ssh_authorized_keys` argument as a string or as a file (Fix #67) -* Add support for reserved Public IP on instance first VNIC -* [ ] Define a backup policy for boot volume and additional block volumes +* Add support for reserved Public IP on instance first VNIC (fix #55) +* [ ] Define a backup policy for boot volume and additional block volumes (fix #64) * Add new outputs for each provisioned resources: "all_attributes" outputs have full provider coverage and are auto-updating. == 2.1.0 - 2021-03-02 diff --git a/docs/terraformoptions.adoc b/docs/terraformoptions.adoc index 3b866ab..a1bc1ae 100644 --- a/docs/terraformoptions.adoc +++ b/docs/terraformoptions.adoc @@ -24,15 +24,15 @@ No modules. [cols="a,a",options="header,autowidth"] |=== |Name |Type -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_instance[oci_core_instance.this] |resource -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_public_ip[oci_core_public_ip.this] |resource -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_volume[oci_core_volume.this] |resource -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_volume_attachment[oci_core_volume_attachment.this] |resource -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_instance_credentials[oci_core_instance_credentials.this] |data source -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_private_ips[oci_core_private_ips.this] |data source +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_instance[oci_core_instance.instance] |resource +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_public_ip[oci_core_public_ip.public_ip] |resource +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_volume[oci_core_volume.volume] |resource +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/resources/core_volume_attachment[oci_core_volume_attachment.volume_attachment] |resource +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_instance_credentials[oci_core_instance_credentials.crendential] |data source +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_private_ips[oci_core_private_ips.private_ips] |data source |https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_shapes[oci_core_shapes.ad1] |data source -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_subnet[oci_core_subnet.this] |data source -|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_vnic_attachments[oci_core_vnic_attachments.this] |data source +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_subnet[oci_core_subnet.instance_subnet] |data source +|https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/core_vnic_attachments[oci_core_vnic_attachments.vnic_attachment] |data source |https://registry.terraform.io/providers/hashicorp/oci/latest/docs/data-sources/identity_availability_domains[oci_identity_availability_domains.ad] |data source |=== @@ -150,7 +150,7 @@ No modules. |no |[[input_public_ip]] <> -|OCID of the Public IP to attach to primary vnic. Valid values are NONE, RESERVED or EPHEMERAL. +|Whether to create a Public IP to attach to primary vnic and wwhich lifetime. Valid values are NONE, RESERVED or EPHEMERAL. |`string` |`"NONE"` |no diff --git a/examples/instances_fixed_shape/main.tf b/examples/instances_fixed_shape/main.tf index 276ff36..d46e911 100644 --- a/examples/instances_fixed_shape/main.tf +++ b/examples/instances_fixed_shape/main.tf @@ -34,8 +34,8 @@ module "instance_nonflex" { # operating system parameters ssh_authorized_keys = var.ssh_authorized_keys # networking parameters - assign_public_ip = var.assign_public_ip - subnet_ocids = var.subnet_ocids + public_ip = var.public_ip # NONE, RESERVED or EPHEMERAL + subnet_ocids = var.subnet_ocids # storage parameters block_storage_sizes_in_gbs = var.block_storage_sizes_in_gbs } @@ -65,8 +65,8 @@ module "instance_nonflex_custom" { # operating system parameters ssh_authorized_keys = var.ssh_authorized_keys # networking parameters - assign_public_ip = var.assign_public_ip - subnet_ocids = var.subnet_ocids + public_ip = var.public_ip # NONE, RESERVED or EPHEMERAL + subnet_ocids = var.subnet_ocids # storage parameters block_storage_sizes_in_gbs = [] # no block volume will be created } diff --git a/examples/instances_fixed_shape/variables.tf b/examples/instances_fixed_shape/variables.tf index 74ee248..f8f78fb 100644 --- a/examples/instances_fixed_shape/variables.tf +++ b/examples/instances_fixed_shape/variables.tf @@ -117,6 +117,12 @@ variable "assign_public_ip" { default = false } +variable "public_ip" { + description = "Whether to create a Public IP to attach to primary vnic and wwhich lifetime. Valid values are NONE, RESERVED or EPHEMERAL." + type = string + default = "NONE" +} + variable "subnet_ocids" { description = "The unique identifiers (OCIDs) of the subnets in which the instance primary VNICs are created." type = list(string) diff --git a/examples/instances_flex_shape/main.tf b/examples/instances_flex_shape/main.tf index a1ac29d..cfbaef4 100644 --- a/examples/instances_flex_shape/main.tf +++ b/examples/instances_flex_shape/main.tf @@ -35,8 +35,8 @@ module "instance_flex" { # operating system parameters ssh_public_key = var.ssh_public_key # networking parameters - assign_public_ip = var.assign_public_ip - subnet_ocids = var.subnet_ocids + public_ip = var.public_ip # NONE, RESERVED or EPHEMERAL + subnet_ocids = var.subnet_ocids # storage parameters block_storage_sizes_in_gbs = var.block_storage_sizes_in_gbs } @@ -65,7 +65,7 @@ output "instance_flex" { # # operating system parameters # ssh_public_key = var.ssh_public_key # # networking parameters -# assign_public_ip = var.assign_public_ip +# public_ip = var.public_ip # NONE, RESERVED or EPHEMERAL # subnet_ocids = var.subnet_ocids # # storage parameters # block_storage_sizes_in_gbs = [] # no block volume will be created diff --git a/examples/instances_flex_shape/variables.tf b/examples/instances_flex_shape/variables.tf index 9070466..6011dc6 100644 --- a/examples/instances_flex_shape/variables.tf +++ b/examples/instances_flex_shape/variables.tf @@ -123,10 +123,10 @@ variable "ssh_public_key_path" { # networking parameters -variable "assign_public_ip" { - description = "Whether the VNIC should be assigned a public IP address." - type = bool - default = false +variable "public_ip" { + description = "Whether to create a Public IP to attach to primary vnic and wwhich lifetime. Valid values are NONE, RESERVED or EPHEMERAL." + type = string + default = "NONE" } variable "subnet_ocids" { diff --git a/examples/instances_reserved_public_ip/main.tf b/examples/instances_reserved_public_ip/main.tf index e188365..331a591 100644 --- a/examples/instances_reserved_public_ip/main.tf +++ b/examples/instances_reserved_public_ip/main.tf @@ -19,8 +19,7 @@ provider "oci" { # # * This module will create 1 Flex Compute Instances, with a reserved public IP module "instance_reserved_ip" { - source = "../../" - # source = "oracle-terraform-modules/compute-instance/oci" + source = "oracle-terraform-modules/compute-instance/oci" # general oci parameters compartment_ocid = var.compartment_ocid freeform_tags = var.freeform_tags diff --git a/examples/instances_reserved_public_ip/variables.tf b/examples/instances_reserved_public_ip/variables.tf index 133abca..7c532c5 100644 --- a/examples/instances_reserved_public_ip/variables.tf +++ b/examples/instances_reserved_public_ip/variables.tf @@ -112,7 +112,7 @@ variable "ssh_authorized_keys" { # networking parameters variable "public_ip" { - description = "OCID of the Public IP to attach to primary vnic." + description = "Whether to create a Public IP to attach to primary vnic and wwhich lifetime. Valid values are NONE, RESERVED or EPHEMERAL." type = string default = "NONE" } diff --git a/main.tf b/main.tf index 367140d..50b744d 100644 --- a/main.tf +++ b/main.tf @@ -24,7 +24,7 @@ locals { #################### # Subnet Datasource #################### -data "oci_core_subnet" "this" { +data "oci_core_subnet" "instance_subnet" { count = length(var.subnet_ocids) subnet_id = element(var.subnet_ocids, count.index) } @@ -56,7 +56,7 @@ locals { ############ # Instance ############ -resource "oci_core_instance" "this" { +resource "oci_core_instance" "instance" { count = var.instance_count // If no explicit AD number, spread instances on all ADs in round-robin. Looping to the first when last AD is reached availability_domain = var.ad_number == null ? element(local.ADs, count.index) : element(local.ADs, var.ad_number - 1) @@ -74,7 +74,7 @@ resource "oci_core_instance" "this" { } create_vnic_details { - assign_public_ip = var.public_ip == null ? var.assign_public_ip : false + assign_public_ip = var.public_ip == "NONE" ? var.assign_public_ip : false display_name = var.vnic_name == "" ? "" : var.instance_count != "1" ? "${var.vnic_name}_${count.index + 1}" : var.vnic_name hostname_label = var.hostname_label == "" ? "" : var.instance_count != "1" ? "${var.hostname_label}-${count.index + 1}" : var.hostname_label private_ip = element( @@ -83,7 +83,7 @@ resource "oci_core_instance" "this" { ) skip_source_dest_check = var.skip_source_dest_check // Current implementation requires providing a list of subnets when using ad-specific subnets - subnet_id = data.oci_core_subnet.this[count.index % length(data.oci_core_subnet.this.*.id)].id + subnet_id = data.oci_core_subnet.instance_subnet[count.index % length(data.oci_core_subnet.instance_subnet.*.id)].id freeform_tags = local.merged_freeform_tags defined_tags = var.defined_tags @@ -113,19 +113,19 @@ resource "oci_core_instance" "this" { ################################## # Instance Credentials Datasource ################################## -data "oci_core_instance_credentials" "this" { +data "oci_core_instance_credentials" "crendential" { count = var.resource_platform != "linux" ? var.instance_count : 0 - instance_id = oci_core_instance.this[count.index].id + instance_id = oci_core_instance.instance[count.index].id } ######### # Volume ######### -resource "oci_core_volume" "this" { +resource "oci_core_volume" "volume" { count = var.instance_count * length(var.block_storage_sizes_in_gbs) - availability_domain = oci_core_instance.this[count.index % var.instance_count].availability_domain + availability_domain = oci_core_instance.instance[count.index % var.instance_count].availability_domain compartment_id = var.compartment_ocid - display_name = "${oci_core_instance.this[count.index % var.instance_count].display_name}_volume${floor(count.index / var.instance_count)}" + display_name = "${oci_core_instance.instance[count.index % var.instance_count].display_name}_volume${floor(count.index / var.instance_count)}" size_in_gbs = element( var.block_storage_sizes_in_gbs, floor(count.index / var.instance_count), @@ -137,11 +137,11 @@ resource "oci_core_volume" "this" { #################### # Volume Attachment #################### -resource "oci_core_volume_attachment" "this" { +resource "oci_core_volume_attachment" "volume_attachment" { count = var.instance_count * length(var.block_storage_sizes_in_gbs) attachment_type = var.attachment_type - instance_id = oci_core_instance.this[count.index % var.instance_count].id - volume_id = oci_core_volume.this[count.index].id + instance_id = oci_core_instance.instance[count.index % var.instance_count].id + volume_id = oci_core_volume.volume[count.index].id use_chap = var.use_chap } @@ -149,32 +149,32 @@ resource "oci_core_volume_attachment" "this" { # Networking #################### -data "oci_core_vnic_attachments" "this" { +data "oci_core_vnic_attachments" "vnic_attachment" { count = var.instance_count compartment_id = var.compartment_ocid - instance_id = oci_core_instance.this[count.index].id + instance_id = oci_core_instance.instance[count.index].id depends_on = [ - oci_core_instance.this + oci_core_instance.instance ] } -data "oci_core_private_ips" "this" { +data "oci_core_private_ips" "private_ips" { count = var.instance_count - vnic_id = data.oci_core_vnic_attachments.this[count.index].vnic_attachments[0].vnic_id + vnic_id = data.oci_core_vnic_attachments.vnic_attachment[count.index].vnic_attachments[0].vnic_id depends_on = [ - oci_core_instance.this + oci_core_instance.instance ] } -resource "oci_core_public_ip" "this" { +resource "oci_core_public_ip" "public_ip" { count = var.public_ip == "NONE" ? 0 : var.instance_count compartment_id = var.compartment_ocid lifetime = var.public_ip - display_name = var.public_ip_display_name != null ? var.public_ip_display_name : oci_core_instance.this[count.index].display_name - private_ip_id = data.oci_core_private_ips.this[count.index].private_ips[0].id + display_name = var.public_ip_display_name != null ? var.public_ip_display_name : oci_core_instance.instance[count.index].display_name + private_ip_id = data.oci_core_private_ips.private_ips[count.index].private_ips[0].id # public_ip_pool_id = oci_core_public_ip_pool.test_public_ip_pool.id # * (BYOIP CIDR Blocks) are not supported yet by this module. freeform_tags = local.merged_freeform_tags diff --git a/outputs.tf b/outputs.tf index a296c71..868e0d0 100644 --- a/outputs.tf +++ b/outputs.tf @@ -3,7 +3,7 @@ locals { instances_details = [ // display name, Primary VNIC Public/Private IP for each instance - for i in oci_core_instance.this : < v } + value = { for k, v in oci_core_instance.instance : k => v } } output "public_ip_all_attributes" { description = "all attributes of created public ip" - value = { for k, v in oci_core_public_ip.this : k => v } + value = { for k, v in oci_core_public_ip.public_ip : k => v } } output "private_ips_all_attributes" { description = "all attributes of created private ips" - value = { for k, v in data.oci_core_private_ips.this : k => v } + value = { for k, v in data.oci_core_private_ips.private_ips : k => v } } output "vnic_attachment_all_attributes" { description = "all attributes of created vnic attachments" - value = { for k, v in data.oci_core_vnic_attachments.this : k => v } + value = { for k, v in data.oci_core_vnic_attachments.vnic_attachment : k => v } } output "volume_all_attributes" { description = "all attributes of created volumes" - value = { for k, v in oci_core_volume.this : k => v } + value = { for k, v in oci_core_volume.volume : k => v } } output "volume_attachment_all_attributes" { description = "all attributes of created volumes attachments" - value = { for k, v in oci_core_volume_attachment.this : k => v } + value = { for k, v in oci_core_volume_attachment.volume_attachment : k => v } } diff --git a/variables.tf b/variables.tf index 4bbfa77..0d005c7 100644 --- a/variables.tf +++ b/variables.tf @@ -156,7 +156,7 @@ variable "private_ips" { } variable "public_ip" { - description = "OCID of the Public IP to attach to primary vnic. Valid values are NONE, RESERVED or EPHEMERAL." + description = "Whether to create a Public IP to attach to primary vnic and wwhich lifetime. Valid values are NONE, RESERVED or EPHEMERAL." type = string default = "NONE"