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

Add support for node_network_profile for default node pool and extra node pools. #525

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lonegunmanb
Copy link
Member

@lonegunmanb lonegunmanb commented Mar 15, 2024

Describe your changes

node_public_ip_tags looks like a preview feature, but I found that this feature has already been added in extra node pool resource block, so I just add it into default node pool too.

Issue number

#524

Checklist before requesting a review

  • The pr title can be used to describe what this pr did in CHANGELOG.md file
  • I have executed pre-commit on my machine
  • I have passed pr-check on my machine

Thanks for your cooperation!

@lonegunmanb
Copy link
Member Author

Hi @zioproto would you please give this pr a review? Thanks!

@zioproto
Copy link
Collaborator

@lonegunmanb I am looking at the feature docs:

https://learn.microsoft.com/en-us/azure/aks/use-node-public-ips#use-public-ip-tags-on-node-public-ips-preview

I think we should hold any change until GA.

@zioproto
Copy link
Collaborator

I understand we want to align with was implemented already for the extra node pools to the default node pool.

I propose to add a partial block like we already have also for the default node pool:

dynamic "node_network_profile" {
        for_each = var.agents_pool_node_network_profile == null ? [] : ["node_network_profile"]

        content {
          node_public_ip_tags            = var.agents_pool_node_network_profile.node_public_ip_tags
        }
      }

And to avoid adding more preview features.

my 2 cents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants