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 test and sample for AKS with route table. #2286

Closed
wants to merge 3 commits into from
Closed

Conversation

metacpp
Copy link
Contributor

@metacpp metacpp commented Nov 9, 2018

W/o specifying the route table and associate it with subnet, AKS will try to create a new route table instead and not associate it w/ subnet.

Before AKS team apply the fix, we'd like to update the test to cover customized route table scenario and encourage user to specify it instead of using default one, which is not working for now.

@tombuildsstuff tombuildsstuff self-assigned this Nov 14, 2018
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.

hi @metacpp

Thanks for this PR - apologies for the delayed review here.

This PR generally looks ok - I've left a few comments inline since I believe we should be able to combine some of the tests. I've been taking a look into #1820 today which has involved some changes which will conflict with this - rather than trying to reconcile those I'm going to pull these changes into #1820 and make the suggested fixes, I hope you don't mind!

Thanks!

@tombuildsstuff
Copy link
Contributor

Closing as these changes have been pulled into #1820

tombuildsstuff added a commit to timcurless/terraform-provider-azurerm that referenced this pull request Nov 14, 2018
tombuildsstuff pushed a commit that referenced this pull request Nov 15, 2018
* Adding support for AzureAD auth to AKS via RBAC

* formatting

* formatting

* Adding documentation updates

* Fixing issues with aadProfile server_app_secret always causing a new cluster

* Refactoring code to avoid repetition in some areas. Documentation updates.

* minor updates to testing section of main documentation index

* docs: client_id -> client_app_id

* fixing one more misnamed var in the docs

* AKS: switching RBAC to be an independent block

* Rewriting the documentation to be clearer (ala VM's)

* AKS: refactoring

* AKS: adding additional context to the errors

* updating a todo

* Data Source: refactoring

* Data Source: continued re-ordering/removing the existing structure

* d/AKS: adding the `role_based_access_control` block

* AKS: support for up to 100 agents

* Updating the versions of kubernetes being used

* Documentation: moving the AKS Examples into their own folder

* removing the older aks examples

* Updating to include the changes from #2286

* re-ordering/updating the errors

* removing the unused env variables

* fixing the linting

* fixing broken tests

* fixes from code review
@metacpp metacpp deleted the aks_doc_update branch November 15, 2018 18:14
@ghost
Copy link

ghost commented Mar 5, 2019

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 5, 2019
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.

2 participants