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

r/kubernetes_cluster: support for outbound_type within the network_profile block #6120

Merged
merged 6 commits into from
Apr 7, 2020

Conversation

baboune
Copy link
Contributor

@baboune baboune commented Mar 16, 2020

I downsized all the changes from #5757 to the bare minimal go changes to add the AKS outboundType feature #5583.

Did not look at all the dependency changes.
Did not introduce azure-sdk-for-go v0.40.

Fixes #5583 .

Closes #5757

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Hi @baboune, thanks for the PR. Overall this looks great but I've left a couple comments inline - basically we don't ignore case without good reason.

@baboune
Copy link
Contributor Author

baboune commented Mar 21, 2020

@katbyte I did the updates. Waiting on your for the webhook vs WebHook part.

@baboune
Copy link
Contributor Author

baboune commented Mar 24, 2020

Hi @katbyte

Can this move forward or what else is required?

@twllight
Copy link

Really waiting for this one! Thank you everyone

@tombuildsstuff tombuildsstuff self-assigned this Mar 27, 2020
@baboune
Copy link
Contributor Author

baboune commented Apr 3, 2020

What else could I do to move this forward?
We really need this.

@tombuildsstuff
Copy link
Contributor

@baboune apologies for the delay with this PR - we've been trying to resolve the breaking changes with AKS (which happened in #2805). I hope you don't mind but so that we can take a look here I'm going to push a rebase of this PR

Thanks!

@tombuildsstuff tombuildsstuff force-pushed the 5583-fix-outboundType branch from 431ac01 to 84c24a0 Compare April 6, 2020 14:28
@ghost ghost added size/XS and removed size/S labels Apr 6, 2020
@tombuildsstuff tombuildsstuff added this to the v2.5.0 milestone Apr 6, 2020
@baboune
Copy link
Contributor Author

baboune commented Apr 7, 2020

@tombuildsstuff I appreciate you taking time to include this.

@tombuildsstuff tombuildsstuff requested review from tombuildsstuff and removed request for katbyte April 7, 2020 08:30
Copy link
Contributor

@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 @baboune

Thanks for this PR :)

I've taken a look through and left some comments inline, but this is off to a good start. Given that there's a number of AKS PR's open (which until recently have been blocked by breaking changes to the AKS API) - I'm currently working through the open AKS PR's to resolve any merge conflicts and get them merged.

As such I hope you don't mind but whilst I've left comments inline regarding which changes are needed - I'm going to push a commit to resolve this so that we can get this merged and ship this in the v2.5 release of the Azure Provider.

Thanks!

@ghost ghost added documentation size/M and removed size/XS labels Apr 7, 2020
@tombuildsstuff tombuildsstuff dismissed katbyte’s stale review April 7, 2020 11:53

dismissing since changes have been pushed

@tombuildsstuff tombuildsstuff changed the title Bare minimal go changes to add feature for AKS outboundType r/kubernetes_cluster: support for outbound_type within the network_profile block Apr 7, 2020
@ghost ghost added size/L and removed size/M labels Apr 7, 2020
@tombuildsstuff
Copy link
Contributor

Acceptance tests look good 👍

@tombuildsstuff tombuildsstuff merged commit 45570ed into hashicorp:master Apr 7, 2020
tombuildsstuff added a commit that referenced this pull request Apr 7, 2020
@baboune baboune deleted the 5583-fix-outboundType branch April 7, 2020 20:09
@ghost
Copy link

ghost commented Apr 9, 2020

This has been released in version 2.5.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.5.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented May 8, 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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators May 8, 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.

Support for AKS outboundType
4 participants