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.

Sign up for GitHub

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

Jump to bottom

AKS: add support for ingress-appgw addon #9419

Closed

derkoe wants to merge 2 commits into hashicorp:master from derkoe:add-aks-addon-ingress-appgw

Contributor

derkoe commented Nov 23, 2020

Closes #7384


          AKS: add support for ingress-appgw addon

a8251db

Closes #7384

ghost added the size/M label

tombuildsstuff added enhancement preview service/kubernetes-cluster labels

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

tombuildsstuff self-assigned this

picardsrcd commented Mar 9, 2021

@tombuildsstuff do you have an ETA to having this merged in and released?
From a workaround perspective, ill look at enabling the addon using CLI in the interim.


          Merge branch 'master' into add-aks-addon-ingress-appgw

adc5948

katbyte requested changes

View reviewed changes

Collaborator

katbyte left a comment

Hey @derkoe - thanks for the pr, however it appears to be missing a test for the new block and updates to the docs. Could we get those added?

azurerm/internal/services/containers/kubernetes_addons.go

@@ @@ -84,6 +87,39 @@ func schemaKubernetesAddOnProfiles() *schema.Schema { @@
               					},
               				},
+              				"ingress_appgw": {

Collaborator

katbyte Mar 29, 2021

could we not abbreviate this?

Suggested change

      
            				"ingress_appgw": {
          
            				"ingress_app_gateway": {

azurerm/internal/services/containers/kubernetes_addons.go

+              								Type:     schema.TypeBool,
+              								Required: true,
+              							},
+              							"appgw_id": {

Collaborator

katbyte Mar 29, 2021

Suggested change

      
            							"appgw_id": {
          
            							"gateway_id": {

azurerm/internal/services/containers/kubernetes_addons.go

+              								Optional:     true,
+              								ValidateFunc: azureHelpers.ValidateResourceID,
+              							},
+              							"appgw_name": {

Collaborator

katbyte Mar 29, 2021

is it possible to pull this out of the gateway id?

azurerm/internal/services/containers/kubernetes_addons.go

+              								Type:     schema.TypeString,
+              								Optional: true,
+              							},
+              							"appgw_subnet_id": {

Collaborator

katbyte Mar 29, 2021

given the block name containers app gateway could we

Suggested change

      
            							"appgw_subnet_id": {
          
            							"subnet_id": {

also is this something we could load from the gateway? or does it potentially have multiple?

azurerm/internal/services/containers/kubernetes_addons.go

+              								Optional:     true,
+              								ValidateFunc: azureHelpers.ValidateResourceID,
+              							},
+              							"appgw_subnet_prefix": {

Collaborator

katbyte Mar 29, 2021

Suggested change

      
            							"appgw_subnet_prefix": {
          
            							"subnet_prefix": {

also is this something we could load from the gateway? or does it potentially have multiple?

cwiederspan commented Apr 14, 2021

@derkoe Any chance this is still being worked on? I also have the same need and it looks like you're off to a great start to getting this going. I haven't been a contributor on this project yet, but I'm willing to try if there's anything I can do to help.

Contributor Author

derkoe commented Apr 15, 2021

@cwiederspan I'm sorry but we are not using Application Gateway anymore so my focus is not on this PR anymore. Maybe you can take over?

markrzasa mentioned this pull request

azurerm_kubernetes_cluster: support for ingress_application_gateway addon #11376

Merged

Collaborator

magodo commented Apr 20, 2021 •

edited

Loading

This feature has been picked up by #11376. Probably people here can also subscribe that PR, which contains more comprehensive test and document.

Collaborator

katbyte commented Apr 27, 2021

I'm going to close this in favour of #11376 as @derkoe is no longer working on this

katbyte closed this

katbyte pushed a commit that referenced this pull request


          AKS: support for ingress_application_gateway addon (#11376)

c16d32f

Hi Everyone,

I didn't realize the following pull request existed before starting this one:

#9419

However, it looks like that pull request is no longer in development. I tried to address the comments in that pull request in this pull request.

Please let me know what you think.

Thanks,
Mark

alvintang pushed a commit to alvintang/terraform-provider-azurerm that referenced this pull request


          AKS: support for ingress_application_gateway addon (hashicorp#11376)

6ef28d8

Hi Everyone,

I didn't realize the following pull request existed before starting this one:

hashicorp#9419

However, it looks like that pull request is no longer in development. I tried to address the comments in that pull request in this pull request.

Please let me know what you think.

Thanks,
Mark

github-actions bot commented May 28, 2021

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions bot locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement preview service/kubernetes-cluster size/M