Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

fix(module/vmseries): Reuse of existing public IP for 1 NIC caused creating ephemeral IPs for other NICs #232

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

sebastianczech
Copy link
Contributor

@sebastianczech sebastianczech commented Dec 18, 2023

Description

PR delivers fix for module vmseries, which resolve following problem : after adding existing public IPs for one or more NICs, for the rest of the NICs code was creating ephemeral, external IPs.

Motivation and Context

Customer opened an issue, in which:

  • they had VM-Series with 3 NICs
  • they add BYOIP to the interface NIC0
  • in the same time NIC1 and NIC2 were getting unwanted GCP public IP addresses

In the lab it was checked following scenario, where in the example code for 1 VM-Series instead of creating public IPs:

network_interfaces = [
{
vpc_network_key = "fw-untrust-vpc"
subnetwork_key = "fw-untrust-sub"
private_ip = "10.10.11.2"
create_public_ip = true
},
{
vpc_network_key = "fw-mgmt-vpc"
subnetwork_key = "fw-mgmt-sub"
private_ip = "10.10.10.2"
create_public_ip = true
},
{
vpc_network_key = "fw-trust-vpc"
subnetwork_key = "fw-trust-sub"
private_ip = "10.10.12.2"
}
]

there were reused existing IPs:

    network_interfaces = [
      {
        vpc_network_key = "fw-untrust-vpc"
        subnetwork_key  = "fw-untrust-sub"
        private_ip      = "10.10.11.2"
        public_ip       = "A1.B1.C1.D1"
      },
      {
        vpc_network_key = "fw-mgmt-vpc"
        subnetwork_key  = "fw-mgmt-sub"
        private_ip      = "10.10.10.2"
        public_ip       = "A2.B2.C2.D2"
      },
      {
        vpc_network_key = "fw-trust-vpc"
        subnetwork_key  = "fw-trust-sub"
        private_ip      = "10.10.12.2"
      }
    ]

where A1.B1.C1.D1 and A2.B2.C2.D2 represents some public IPs.

After changing TFVARS, it was added public_ip in example vpc_peering_common:

network_interfaces = [for v in each.value.network_interfaces :
{
subnetwork = module.vpc[v.vpc_network_key].subnetworks[v.subnetwork_key].self_link
private_ip = v.private_ip
create_public_ip = try(v.create_public_ip, false)
public_ip = try(v.public_ip, null)

Then while after applying changes on already deployed infrastructure Terraform was adding empty access config for the 3 NIC, which shouldn't have public IP:

Terraform will perform the following actions:

  # module.vmseries["fw-vmseries-01"].google_compute_instance.this will be updated in-place
  ~ resource "google_compute_instance" "this" {
        id                        = "projects/gcp-gcs-pso/zones/europe-west2-b/instances/sczechfw-vmseries-01"
        name                      = "sczechfw-vmseries-01"
        tags                      = [
            "vmseries",
        ]
        # (19 unchanged attributes hidden)

      ~ network_interface {
            name                        = "nic2"
            # (7 unchanged attributes hidden)

          + access_config {}
        }

        # (5 unchanged blocks hidden)
    }

  # module.vmseries["fw-vmseries-02"].google_compute_instance.this will be updated in-place
  ~ resource "google_compute_instance" "this" {
        id                        = "projects/gcp-gcs-pso/zones/europe-west2-c/instances/sczechfw-vmseries-02"
        name                      = "sczechfw-vmseries-02"
        tags                      = [
            "vmseries",
        ]
        # (19 unchanged attributes hidden)

      ~ network_interface {
            name                        = "nic2"
            # (7 unchanged attributes hidden)

          + access_config {}
        }

        # (5 unchanged blocks hidden)
    }

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

Changes to Outputs:
  ~ vmseries_public_ips  = {
      ~ fw-vmseries-01 = {
          + "2" = null
            # (2 unchanged attributes hidden)
        }
      ~ fw-vmseries-02 = {
          + "2" = null
            # (2 unchanged attributes hidden)
        }
    }

How Has This Been Tested?

Code was tested by changing module vmseries and applying example vpc_peering_common.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

@sebastianczech sebastianczech requested a review from a team as a code owner December 18, 2023 08:53
@sebastianczech sebastianczech changed the title fix(module/vmseries): Resolve issue with reusing existing public fix(module/vmseries): Resolve issue with reusing existing public IP Dec 18, 2023
@sebastianczech sebastianczech changed the title fix(module/vmseries): Resolve issue with reusing existing public IP fix(module/vmseries): Reuse of existing public IP for 1 NIC caused creating ephemeral IPs for other NICs Dec 18, 2023
Copy link
Contributor

@horiagunica horiagunica left a comment

Choose a reason for hiding this comment

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

👍 Would it be possible to add that option for the rest of the examples just so they are the same across ? Just the main.tf parameter option for vmseries :
public_ip = try(v.public_ip, null) .

@sebastianczech
Copy link
Contributor Author

Thank you @horiagunica for review and valuable suggestion - I added public_ip = try(v.public_ip, null) to all examples.

@sebastianczech sebastianczech merged commit 27bcc01 into main Dec 19, 2023
62 checks passed
@sebastianczech sebastianczech deleted the fix-vmseries-use-existing-public-ip branch December 19, 2023 07:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants