Skip to content

Conversation

@DaMandal0rian
Copy link
Contributor

@DaMandal0rian DaMandal0rian commented Mar 14, 2025

User description

add multi-network gateway for auto-drive


PR Type

  • Enhancement

Description

  • Add multi-network gateway module configuration.

  • Introduce variables for multi-network gateway instances.

  • Adjust lifecycle to ignore instance_type changes.


Changes walkthrough 📝

Relevant files
Enhancement
main.tf
Add multi-network gateway module block                                     

resources/terraform/auto-drive/main.tf

  • Added module "ec2_multi_gateway" with configuration.
  • Configured instance details and EIP creation.
  • Set up IAM instance profile and block device.
  • +40/-0   
    variables.tf
    Add variables for multi-network gateway instances               

    resources/terraform/auto-drive/variables.tf

  • Introduced variable for multi-network gateway instance type.
  • Added variable for multi-network gateway instance count.
  • +13/-0   
    main.tf
    Update EC2 lifecycle ignore_changes configuration               

    templates/terraform/aws/ec2/main.tf

  • Updated lifecycle ignore_changes to include instance_type.
  • Applied changes in both aws_instance resources.
  • +2/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Config Issue

    The multi-network gateway module is configured with the variable gateway_instance_type rather than the newly introduced multi_network_gateway_instance_type, which may lead to inconsistent instance types. This should be reviewed to ensure proper variable usage.

    count                       = var.multi_network_gateway_instance_count
    ami                         = data.aws_ami.ubuntu_amd64.id
    instance_type               = var.gateway_instance_type
    Lifecycle Caution

    The addition of instance_type to the ignore_changes list may prevent intentional updates to the instance type. This behavior should be carefully validated to ensure it does not lead to undesired state drift.

      instance_type,
    ]

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Use correct instance type variable

    Replace var.gateway_instance_type with var.multi_network_gateway_instance_type for
    multi-network gateway instances to ensure the correct instance type is used.

    resources/terraform/auto-drive/main.tf [217-251]

     module "ec2_multi_gateway" {
       source                      = "../../../templates/terraform/aws/ec2"
       name                        = "${local.name}-multi-network-gateway"
       count                       = var.multi_network_gateway_instance_count
       ami                         = data.aws_ami.ubuntu_amd64.id
    -  instance_type               = var.gateway_instance_type
    +  instance_type               = var.multi_network_gateway_instance_type
       availability_zone           = element(module.vpc.azs, 0)
       subnet_id                   = element(module.vpc.public_subnets, 0)
       ...
     }
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies a bug where the wrong instance type variable is used and proposes replacing it with var.multi_network_gateway_instance_type, ensuring that the multi-network gateway instances are created with the intended configuration.

    High

    @DaMandal0rian DaMandal0rian force-pushed the feat/multi-net-gw branch 2 times, most recently from edbba0d to 6644266 Compare March 14, 2025 15:34
    @DaMandal0rian DaMandal0rian changed the title add multi-network gateway add multi-network gateway + taurus public AD Mar 14, 2025
    clostao
    clostao previously approved these changes Mar 14, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants