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

Support application gateway backend pool configuration #1027

Merged

Conversation

abn
Copy link
Contributor

@abn abn commented Mar 24, 2018

Support application gateway pool configuration for both vmss and network interface.

This change-set preserves code changes implemented by @agolomoodysaada in PR #884. A few changes were made to the original commit to get vmss working and verified as expected.

In addition to vmss, this also adds support for application gateway backend pool configuration for network interface resources and data sources.

Supersedes: #884
Closes #857

Copy link
Member

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @abn

Thanks for this PR / fixing the outstanding issue from #884 - I've noticed one minor thing that needs to be fixed up but this otherwise LGTM :)

Thanks!

@@ -940,6 +947,14 @@ func flattenAzureRmVirtualMachineScaleSetNetworkProfile(profile *compute.Virtual
config["subnet_id"] = *properties.Subnet.ID
}

if properties.ApplicationGatewayBackendAddressPools != nil {
addressPools := make([]interface{}, 0, len(*properties.ApplicationGatewayBackendAddressPools))
Copy link
Member

Choose a reason for hiding this comment

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

can we move this assignment out of the if statement and make this:

addressPools := make([]interface{}, 0)
if properties.ApplicationGatewayBackendAddressPools != nil {
  # ...
}
config["application_gateway_backend_address_pool_ids"] = schema.NewSet(schema.HashString, addressPools)

this will ensure the application_gateway_backend_address_pool_ids variable is set to an empty list if the field is nil (which will mean any changes are detected)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack. Done.

agolomoodysaada and others added 2 commits March 27, 2018 22:02
Both network interface data source and resource now supports
configuration of applicaiton gateway backed pool ids.
@abn abn force-pushed the application-gateway-backend-pools branch from 8c069e5 to 558de7e Compare March 27, 2018 09:03
@abn
Copy link
Contributor Author

abn commented Mar 27, 2018

@tombuildsstuff thanks for the review; changed as requested. Let me know if you notice anything else.

Copy link
Member

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @abn

Thanks for pushing those changes - this now LGTM - I've kicked off the test suite and we should get the results shortly :)

Thanks!

@tombuildsstuff
Copy link
Member

Ignoring the known issue with the Application Gateway API - the tests pass:

screen shot 2018-03-28 at 08 23 04

screen shot 2018-03-28 at 08 23 51

Thanks for this!

@tombuildsstuff tombuildsstuff merged commit ef8baa4 into hashicorp:master Mar 28, 2018
tombuildsstuff added a commit that referenced this pull request Mar 28, 2018
@abn abn deleted the application-gateway-backend-pools branch March 28, 2018 23:10
@ghost
Copy link

ghost commented Mar 31, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Application Gateway Backend Pool in VMSS
3 participants