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

refactor(example/gwlb_with_vmseries): Refactor Gateway Load Balancer example #4

Merged
merged 14 commits into from
Jan 29, 2024

Conversation

sebastianczech
Copy link
Contributor

@sebastianczech sebastianczech commented Jan 23, 2024

Description

PR is a part of #9 and it delivers Gateway Load Balancer in refactored example gwlb_with_vmseries.

Motivation and Context

In #3 all modules and examples are being refactored according to issue description. PR 372 in archived repository delivered changes in gwlb module, but it didn't contain refactored example with Gateway Load Balancer.

How Has This Been Tested?

Code was tested by deploying example in the lab.

Types of changes

  • New feature (non-breaking change which adds functionality)

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 added the refactor Related to code refactoring label Jan 23, 2024
@sebastianczech sebastianczech marked this pull request as ready for review January 24, 2024 19:30
Copy link
Contributor

@alperenkose alperenkose left a comment

Choose a reason for hiding this comment

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

IMO we should keep Common and GWLB examples separate as GWLB is not in the reference architecture design guide yet. We also have links from pan.dev to the repository examples for these designs so merging them with different example.tfvars can cause confusion.

General question; if we keep the GWLB example separate shall we still keep the un-compatible modules in the example code like NATGW or AppGW ? I believe these resources are not possible to integrate with GWLB, and it might bring confusion/doubts to the users trying out the example.

examples/common_vmseries/README.md Outdated Show resolved Hide resolved
examples/common_vmseries/variables.tf Outdated Show resolved Hide resolved
examples/gwlb_with_vmseries/example.tfvars Outdated Show resolved Hide resolved
examples/gwlb_with_vmseries/example.tfvars Outdated Show resolved Hide resolved
examples/gwlb_with_vmseries/example.tfvars Outdated Show resolved Hide resolved
examples/gwlb_with_vmseries/variables.tf Outdated Show resolved Hide resolved
examples/gwlb_with_vmseries/versions.tf Outdated Show resolved Hide resolved
@acelebanski
Copy link
Contributor

So summing up today's discussion during the planning meeting, we decided not to share common codebase with GWLB example, it will remain separate.

We will also transform the test_infrastructure example into a module and then integrate it with all examples but for now (in scope of this PR), I think the test infra can remain a part of the GWLB example.

@acelebanski acelebanski linked an issue Jan 25, 2024 that may be closed by this pull request
@sebastianczech
Copy link
Contributor Author

sebastianczech commented Jan 25, 2024

thanks @alperenkose for review, I updated the code, there is no Application Gateway and NAT Gateway in the main.tf.

@acelebanski I removed changes from common design and I left refactored example for GWLB together test VMs.

PR is ready to being reviewed.

@sebastianczech sebastianczech merged commit 19d7e93 into refactor-modules Jan 29, 2024
@sebastianczech sebastianczech deleted the refactor-example-gwlb branch January 29, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Related to code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor GWLB example
3 participants