Skip to content
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

Feat/suffix separator variable #218

Merged
merged 10 commits into from
Jan 5, 2022
Merged

Feat/suffix separator variable #218

merged 10 commits into from
Jan 5, 2022

Conversation

thiagonache
Copy link
Contributor

Add new variable to handle hostname separator char in the compute_instance and umig modules.
Discussed on issue #217

@thiagonache
Copy link
Contributor Author

I've signed the CLA few seconds ago.

@comment-bot-dev
Copy link

comment-bot-dev commented Dec 17, 2021

Thanks for the PR! 🚀
✅ Lint checks have passed.

@thiagonache
Copy link
Contributor Author

I believe the error is not related to my code change. Can you check it, please @morgante ?

-----> Converging <disk-snapshot-local>...
$$$$$$ Reading the Terraform client version...
       Terraform v1.0.5
       on linux_amd64
       + provider registry.terraform.io/hashicorp/google v4.4.0
       + provider registry.terraform.io/hashicorp/null v3.1.0
       + provider registry.terraform.io/hashicorp/random v2.3.1
       
       Your version of Terraform is out of date! The latest version
       is 1.1.1. You can update by downloading from https://www.terraform.io/downloads.html
$$$$$$ Finished reading the Terraform client version.
$$$$$$ Verifying the Terraform client version is in the supported interval of >= 0.11.4, < 1.1.0...
$$$$$$ Finished verifying the Terraform client version.
$$$$$$ Selecting the kitchen-terraform-disk-snapshot-local Terraform workspace...
$$$$$$ Finished selecting the kitchen-terraform-disk-snapshot-local Terraform workspace.
$$$$$$ Downloading the modules needed for the Terraform configuration...
       - disk_snapshot in ../../../../examples/compute_instance/disk_snapshot
       - disk_snapshot.compute_instance in ../../../../modules/compute_instance
       - disk_snapshot.disk_snapshots in ../../../../modules/compute_disk_snapshot
       - disk_snapshot.instance_template in ../../../../modules/instance_template
$$$$$$ Finished downloading the modules needed for the Terraform configuration.
$$$$$$ Validating the Terraform configuration files...
       ╷
       │ Warning: Version constraints inside provider configuration blocks are deprecated
       │ 
       │   on network.tf line 18, in provider "random":
       │   18:   version = "~> 2.2"
       │ 
       │ Terraform 0.13 and earlier allowed provider version constraints inside the
       │ provider configuration block, but that is now deprecated and will be
       │ removed in a future version of Terraform. To silence this warning, move the
       │ provider version constraint into the required_providers block.
       ╵
       Success! The configuration is valid, but there were some
       validation warnings as shown above.
       
$$$$$$ Finished validating the Terraform configuration files.
$$$$$$ Building the infrastructure based on the Terraform configuration...
       ╷
       │ Warning: Version constraints inside provider configuration blocks are deprecated
       │ 
       │   on network.tf line 18, in provider "random":
       │   18:   version = "~> 2.2"
       │ 
       │ Terraform 0.13 and earlier allowed provider version constraints inside the
       │ provider configuration block, but that is now deprecated and will be
       │ removed in a future version of Terraform. To silence this warning, move the
       │ provider version constraint into the required_providers block.
       ╵
       ╷
       │ Error: Invalid function argument
       │ 
       │   on ../../../../modules/instance_template/main.tf line 97, in resource "google_compute_instance_template" "tpl":
       │   97:       email  = lookup(service_account.value, "email", null)
       │     ├────────────────
       │     │ service_account.value is null
       │ 
       │ Invalid value for "inputMap" parameter: argument must not be null.
       ╵
       ╷
       │ Error: Invalid function argument
       │ 
       │   on ../../../../modules/instance_template/main.tf line 98, in resource "google_compute_instance_template" "tpl":
       │   98:       scopes = lookup(service_account.value, "scopes", null)
       │     ├────────────────
       │     │ service_account.value is null
       │ 
       │ Invalid value for "inputMap" parameter: argument must not be null.

@thiagonache
Copy link
Contributor Author

I just tested out the master branch and it does fail with the same errors.

@thiagonache
Copy link
Contributor Author

@morgante I started to troubleshoot this and I've found that the example passes the service account as null (it was added 3 months ago)

module "instance_template" {
  source          = "../../../modules/instance_template"
  region          = var.region
  project_id      = var.project_id
  subnetwork      = var.subnetwork
  name_prefix     = "instance-disk-snapshot"
  service_account = null

but the module fails on the dynamic block that was added three years ago.

  dynamic "service_account" {
    for_each = [var.service_account]
    content {
      email  = lookup(service_account.value, "email", null)
      scopes = lookup(service_account.value, "scopes", null)
    }
  }

I don't know how to fix it properly. I've added service_account = { email = "", scopes = ["cloud-platform"] }. Please, let me know.

@thiagonache
Copy link
Contributor Author

@bharathkkb @morgante I don't know how it was working before but everything seems okay now. Can you help me to get this code merged, please? 😊

Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @thiagonache
I believe we don't test disk-snapshot-local in CI and hence this was not caught. CI was failing due to another flaky issue which I have opened a fix for in #219

Can we open an issue for the disk-snapshot-local and null lookup issue so that we can track a fix rather than fixing the example?

modules/umig/README.md Outdated Show resolved Hide resolved
thiagonache and others added 2 commits January 4, 2022 17:57
…l fail due to dynamic block in the instance template module"

This reverts commit 80f9276.
@thiagonache
Copy link
Contributor Author

Opened issue #220.

@thiagonache thiagonache requested a review from bharathkkb January 4, 2022 22:47
@bharathkkb bharathkkb merged commit d4e0e87 into terraform-google-modules:master Jan 5, 2022
@thiagonache thiagonache deleted the feat/suffix_separator_variable branch January 5, 2022 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants