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

fix: replace zipmap in output.tf #449

Merged
merged 5 commits into from
Mar 23, 2023
Merged

fix: replace zipmap in output.tf #449

merged 5 commits into from
Mar 23, 2023

Conversation

Aashiq-J
Copy link
Member

@Aashiq-J Aashiq-J commented Mar 21, 2023

Description

When we had more than one subnet in one zone, the zipmap function used in the output.tf would give the following error
Call to function "zipmap" failed: number of keys (3) does not match number of values (6).

Types of changes in this PR

No release required

  • Examples or tests (addition or updates of examples or tests)
  • Documentation update
  • CI-related update (pipeline, etc.)
  • Other changes that don't affect Terraform code

Release required

  • Bug fix (patch release (x.x.X): Change that fixes an issue and is compatible with earlier versions)
  • New feature (minor release (x.X.x): Change that adds functionality and is compatible with earlier versions)
  • Breaking change (major release (X.x.x): Change that is likely incompatible with previous versions)
Release notes content

Replace this text with information that users need to know about the bug fixes, features, and breaking changes. This information helps the merger write the commit message that is published in the release notes for the module.


Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

Merge actions for mergers

  • Merge by using "Squash and merge".

  • Use a relevant conventional commit message that is based on the PR contents and any release notes provided by the PR author.

    The commit message determines whether a new version of the module is needed, and if so, which semver increment to use (major, minor, or patch).

@Aashiq-J Aashiq-J requested a review from imprateeksh March 21, 2023 14:39
@Aashiq-J Aashiq-J marked this pull request as ready for review March 23, 2023 05:58
@imprateeksh
Copy link
Member

imprateeksh commented Mar 23, 2023

Review was done offline after validating changes but some error was seen.
I slacked @Aashiq-J about the problem faced while validating this change using base-ocp module.
Error was seen as :
image

If tried multizone-cluster example with the below mentioned subnets and address prefixes :
subnets and address prefixes as mentioned below:

# Address Prefixes:
  default = {
    zone-1 = ["10.243.0.0/23", "10.243.5.0/24"]
    zone-2 = ["10.243.64.0/23", "10.243.69.0/24"]
    zone-3 = ["10.243.128.0/23", "10.243.133.0/24"]
  }

# Subnets
default = {
    zone-1 = [
      {
        acl_name = "vpc-acl"
        name     = "z1-subnet-a"
        cidr     = "10.243.0.0/23"
      },
      {
        acl_name = "vpc-acl"
        name     = "z1-subnet-b"
        cidr     = "10.243.5.0/24"
      }
    ],
    zone-2 = [
      {
        acl_name = "vpc-acl"
        name     = "z2-subnet-c"
        cidr     = "10.243.64.0/23"
      },
      {
        acl_name = "vpc-acl"
        name     = "z2-subnet-d"
        cidr     = "10.243.69.0/24"
      }
    ],
    zone-3 = [
      {
        acl_name = "vpc-acl"
        name     = "z3-subnet-e"
        cidr     = "10.243.128.0/23"
      },
      {
        acl_name = "vpc-acl"
        name     = "z3-subnet-f"
        cidr     = "10.243.133.0/24"
      }
    ]
  }
}

In this case , it was found out that failure is because the example is trying to deploy two different ocp cluster in the same subnets.
If used below format for subnets, it succeeds :
image

@ocofaigh @vburckhardt : Please advise will this be a correct way to solve this if need to modify the ocp module ?
Like ocp-base is one of the examples, exploring other modules in GE to see if similar error is seen.

@Aashiq-J Aashiq-J requested a review from ocofaigh March 23, 2023 10:07
@ocofaigh ocofaigh merged commit 8272639 into main Mar 23, 2023
@ocofaigh ocofaigh deleted the 4227 branch March 23, 2023 11:50
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 5.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

gmendel pushed a commit to gmendel/landing-zone-vpc that referenced this pull request Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants