kubernetes_cluster: support specifying node_resource_group #3785
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I won't have time next week to take care after that, but I will do it week after. Docs should be easy, but could you suggest some implementation for tests? |
|
All you need to do is Copy and rename a test and then set the property, or if there is a |
invidian
commented
Jul 15, 2019
There was a problem hiding this comment.
Added test and documentation. Perhaps tests could be added to TestAccAzureRMKubernetesCluster_basic instead, I'm not sure about this.
Also 2 thoughts which came to my mind while doing documentation:
- should change of
node_resource_grouprecreate the cluster? - as deployment will fail when existing resource group is used, should we check if specified resource group exists before creating a cluster and fail early?
Closes #3354 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
AKS FAQ answers that, so I added |
tombuildsstuff
approved these changes
Jul 19, 2019
There was a problem hiding this comment.
hey @invidian
Thanks for pushing those changes - besides a couple of minor changes (which I hope you don't mind, but I'm going to push a commit to fix) this otherwise LGTM 👍
Thanks!
tombuildsstuff
dismissed
katbyte’s stale review
dismissing since changes have been pushed
invidian
commented
Jul 19, 2019
There was a problem hiding this comment.
@tombuildsstuff changes you pushed seems unrelated to the PR, but I'm fine with that. Otherwise LGTM 👍
Anything else for me to do to get that merged?
|
@infuerno nope, all good (I ended up re-ordering the file whilst passing through, nothing major) - I've just been waiting for the tests to run - once they've passed we're good to merge 👍 |
|
Tests look good - thanks again for this @invidian :) |
|
This has been released in version 1.32.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 = "~> 1.32.0"
}
# ... other configuration ... |
metacpp
pushed a commit
to VSChina/terraform-provider-azurerm
that referenced
this pull request
Aug 15, 2019
* r/storage_blob: refactoring the existing tests * Updating to use hashicorp#3857 * Add metadata to example usage * r/storage_blob: rewriting the acceptance tests * r/storage_blob: test fixes * Adding a todo * r/private_dns_a_record: fixing comments from code review * Updating to include hashicorp#3847 * Updating to include hashicorp#3849 * Add schedule backup functionality. * Add doc in app_service.html.markdown file * Add deafult value to backup_schedule_enabled * Add default value to backup_schedule_enabled parameter * delete backup-schedule_enabled parameter * Delete backupScheduleEnabled param * Add condition err in resourceArmDeleteScheduleBackup * In deleteConfiguration function delete check * Add HasChange in update function * Fix errcheck in DeleteBackupConfiguration * Frequency interval validation range limit * Delete backupSchedule in create function * Update azurerm/resource_arm_app_service.go Co-Authored-By: Matthew Frahry <mbfrahry@gmail.com> * Validate retention period range limit * Fix tabulation and add validate to backup_name * Add frequency_unit validation * azurerm_app_service: fixing the code comments from hashicorp#3330 ``` $ acctests azurerm TestAccAzureRMAppService_backup === RUN TestAccAzureRMAppService_backup === PAUSE TestAccAzureRMAppService_backup === CONT TestAccAzureRMAppService_backup --- PASS: TestAccAzureRMAppService_backup (660.26s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 660.313s ``` ``` $ acctests azurerm TestAccAzureRMAppService_basic === RUN TestAccAzureRMAppService_basic === PAUSE TestAccAzureRMAppService_basic === RUN TestAccAzureRMAppService_basicWindowsContainer === PAUSE TestAccAzureRMAppService_basicWindowsContainer === CONT TestAccAzureRMAppService_basic --- PASS: TestAccAzureRMAppService_basic (189.59s) === CONT TestAccAzureRMAppService_basicWindowsContainer --- PASS: TestAccAzureRMAppService_basicWindowsContainer (369.53s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 559.155s ``` * fwixing comments from code review * Updating to include hashicorp#3804 * [azurerm_recovery_services_protected_vm] - changing `backup_pol… (hashicorp#3822) Refer to hashicorp#3441 now changing the behaviour of backup_policy_id so it should not trigger recreation of resource when value changed (fixes hashicorp#3441) * Update CHANGELOG.md to include hashicorp#3822 * storage: using the environment as available * Fix documentation. Real resource name is azurerm_application_in… (hashicorp#3877) * Fix azurerm_redis_cache non-SSL port in docs * fix merging problem * change firewall test to explicit config fix bug of returning nil * fix some errors; enable adminUsers test but not working currently * fix linting error * fix linting error * fix admin users test * r/storage_share: switching the state migration over to 0.12 format ``` $ go test -v ./azurerm/ -run=TestAzureRMStorageShareMigrateState === RUN TestAzureRMStorageShareMigrateStateV0ToV1 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" --- PASS: TestAzureRMStorageShareMigrateStateV0ToV1 (0.00s) resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureChinaCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureGermanCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzurePublicCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureUSGovernmentCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! === RUN TestAzureRMStorageShareMigrateStateV1ToV2 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.chinacloudapi.cn/share1" 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.cloudapi.de/share1" 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.windows.net/share1" 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.usgovcloudapi.net/share1" --- PASS: TestAzureRMStorageShareMigrateStateV1ToV2 (0.00s) resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureChinaCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureGermanCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzurePublicCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureUSGovernmentCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 0.038s ``` * r/kubernetes_cluster: fixing comments from code review * r/kubernetes_cluster: ordering the sections alphabetically * adjust pr comments * r/kubernetes_cluster: switching to an import test * fixing comments from code review * sidebar: fixing the ordering * Updating to include hashicorp#3785 * add function app auth settings * docs * bug: missing link to azurerm_api_management_subscription (hashicorp#3888) * r/analysis_services_server: ensuring the ID is set in the create * updating to include hashicorp#3721 * updating to include hashicorp#3893 * Add auth_settings support to app service slot * Add auth_settings tests to app service slot * auth_settings block in app service slot docs * Missed import modules for test * return value for UpdateAuthSettingsSlot * Wrong resource name in tests * r/app_service_slot: wrapping the errors * r/app_service_slot: renaming the CreateUpdate method to Create since it's been split * r/storage_container: metadata should be computed ``` $ acctests azurerm TestAccAzureRMStorageContainer_metaData === RUN TestAccAzureRMStorageContainer_metaData === PAUSE TestAccAzureRMStorageContainer_metaData === CONT TestAccAzureRMStorageContainer_metaData --- PASS: TestAccAzureRMStorageContainer_metaData (165.53s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 165.571s ``` * [Feature] Azure Maps Account Support Adding Azure Map Accounts support to Terraform. Adds azurerm_maps_account data source. Adds azurerm_maps_account resource type. Adds website documentation for data source and resource. Adds data source and resource acceptance tests. * vendoring the maps sdk * r/maps_account: changes from code review * docs: adding to the sidebar * updating to include hashicorp#3897 * r/maps: removing dead code * d/cosmosdb_account: handling the document endpoint being nil (hashicorp#3899) * maps: making the sku lower-case to match the api * r/storage_container: support for containers named `$web` (hashicorp#3896) Fixes hashicorp#3894 * Update CHANGELOG.md to include hashicorp#3896 * maps: fixing the tests ``` $ acctests azurerm TestAccAzureRMMapsAccount_ === RUN TestAccAzureRMMapsAccount_basic === PAUSE TestAccAzureRMMapsAccount_basic === RUN TestAccAzureRMMapsAccount_sku === PAUSE TestAccAzureRMMapsAccount_sku === RUN TestAccAzureRMMapsAccount_tags === PAUSE TestAccAzureRMMapsAccount_tags === CONT TestAccAzureRMMapsAccount_basic === CONT TestAccAzureRMMapsAccount_tags --- PASS: TestAccAzureRMMapsAccount_basic (80.43s) --- PASS: TestAccAzureRMMapsAccount_tags (111.23s) === CONT TestAccAzureRMMapsAccount_sku --- PASS: TestAccAzureRMMapsAccount_sku (82.26s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 273.956s ``` * [azurerm_kubernetes_cluster ] - support for the `windows_profil… (hashicorp#3519) fixes hashicorp#3508 * Update CHANGELOG.md to include hashicorp#3519 * updating to include hashicorp#3698 * sorting/reordering the changelog * decrease gogc to prevent linting OOM * website: Fix a sidebar name (hashicorp#3911) * `azurerm_storage_account`: set `queue_properties` (hashicorp#3859) * Update CHANGELOG.md * azurerm_cosmosdb_account: validate max_interval_in_seconds and… (hashicorp#3906) fixes hashicorp#3551 * Update CHANGELOG.md to include hashicorp#3906 * kubernetes_cluster: support specifying load balancer sku (hashicorp#3890) Fixes hashicorp#3726 * Update CHANGELOG.md to include hashicorp#3890 * Update mysql_server.html.markdown (hashicorp#3914) * azurerm_application_gateway - Support for Managed Identity (resolves hashicorp#3640) (hashicorp#3648) resolves hashicorp#3640 * Update CHANGELOG.md to include hashicorp#3648 * v1.32.0 * Cleanup after v1.32.0 release * Sending nil certificate when no certificate is specified (hashicorp#3931) * azurerm_kubernetes_cluster - Remove duplicated line in docs (hashicorp#3940) * Bug Fix: `azurerm_api_managment_product` - Additional validation for `approval_required` (hashicorp#3945) * Update CHANGELOG.md * Bug Fix: `azurerm_storage_table` - Migrate id to new format hashicorp#3932 * Bug Fix: `azurerm_kubernetes_cluster` - suppress case difference on `load_balancer_sku` (hashicorp#3958) * Fix crash (hashicorp#3966) * Update CHANGELOG.md * Fix the typo in the application gateway name (hashicorp#3953) The application gateway was referenced by name `test` while it's defined as `network` * azurerm_storage_account - fix `enable_advanced_threat_protectio… (hashicorp#3947) fixes hashicorp#3920 * Update CHANGELOG.md to include hashicorp#3947 * Update CHANGELOG.md * Bug Fix - `azurerm_iot_dps` fix deletion issue with service principal hashicorp#3973 * Update CHANGELOG.md * Update hash function for os_profile_linux_config (hashicorp#3837) * Add new acc test to verify ssh key issue This commit adds a new acceptance test to verify that the vmss is not calculating correctly the hashcode for the `os_profile_linux_config` Related: hashicorp#3836 * Update OsProfileLinuxConfigHash generator Update the `resourceArmVirtualMachineScaleSetOsProfileLinuxConfigHash` method to include in the hash function the fields from the `ssh_keys` subelements. Fixes: hashicorp#3836 * Update CHANGELOG.md to include hashicorp#3837 * Bugfix: Fixes the acceptance tests for resource_arm_dev_test_*_virtual_machine_test.go (hashicorp#3717) * Update CHANGELOG.md to include hashicorp#3717 * azurerm_network_ddos_protection_plan - correctly decodes the re… (hashicorp#3975) Fixes hashicorp#3974 * Update CHANGELOG.md to include hashicorp#3975 * azurerm_virtual_machine_scale_set - changes made to `network… (hashicorp#3821) The acceptance tests added here are describing the issue as reported by this ticket hashicorp#3812 Initially tried to remove the custom Set method which calculates the hashcode similar to what is suggested by hashicorp/terraform#10520 and hashicorp/terraform#15420 but then we got into a position that the test framework was generating a perpetual diff. So the current solution was to include the other fields in the hashcode calculation. Many thanks to @kwilczynski Fixes: hashicorp#3812 Co-authored-by: Krzysztof Wilczynski kw@linux.com * Update CHANGELOG.md to include hashicorp#3821 * azurerm_postgresql_server - support for version `11.0` (hashicorp#3970) Fixes hashicorp#3969, and relates to hashicorp#3327 even so it doesn't solve the problem for 10.X versions. The Azure SDK for go doesn't provide version 10.7 for example, see here. But fortunately it has a version 11. * Update CHANGELOG.md to include hashicorp#3970 * Update CHANGELOG.md to include hashicorp#3986 * storage: always nil check resource group before deref (hashicorp#3986) * v1.32.1 * Cleanup after v1.32.1 release * Added LogAnalyticsDestinationType * Updated the docs * Added the attribute to the doc * Added the value to the error string * New Resource azurerm_dev_test_lab_schedule - Fix for issue 2334 (hashicorp#3554) This pull request is to fix issue hashicorp#2334, It has changes to add new resource provider **azurerm_dev_test_lab_schedule**. **Additional Points** * Resource provider has been created based on [microsoft.devtestlab/2016-05-15/schedules](https://docs.microsoft.com/en-us/azure/templates/microsoft.devtestlab/2016-05-15/schedules) (fixes hashicorp#2334) * Update CHANGELOG.md to include hashicorp#3554 * Bug Fix: `azurerm_app_service_plan` - workaround for missing error on 404 hashicorp#3990 * Update CHANGELOG.md * Fixing typo (hashicorp#3992) * New Feature: `azrurerm_iot_dps` - add support for `linked_hub` (hashicorp#3922) * Update CHANGELOG.md * azurerm_postgresql_server - remove unsupported version `10.2` (hashicorp#3915) Closes hashicorp#3327 * Update CHANGELOG.md to include hashicorp#3915 * Switch more client registrations to new format (hashicorp#3895) * Added property read and simplified validation * Added check that the log analytics workspace id is provided * Added log_analytics_destination_type to the logAnalyticsWorkspace test * Data source for azurerm_dev_test_virtual_network (hashicorp#3746) * Update CHANGELOG.md to include hashicorp#3554 * azurerm_function_app - support for cors (hashicorp#3949) * Update CHANGELOG.md to include hashicorp#3949 * azurerm_role_definition - enture role_definition_id is correctl… (hashicorp#3913) We encountered the issue where role_definition_id remained nil after create, returning 409s on RoleDefinitionsClient.CreateOrUpdate with code RoleDefinitionWithSameNameExists. Updating role definition resource was failing because resourceArmRoleDefinitionCreateUpdate checks if role_definition_id is nil and, if so, creates a new uuid and calls client.CreateOrUpdate which attempts to create a new resource instead of updating. * Update CHANGELOG.md to include hashicorp#3913 * Updated api_management_api_policy for XML file explanation (hashicorp#3994) Added a note for those seeking to utilize -PolicyFilePath from docs.microsoft.com/en-us/powershell/module/azurerm.apimanagement/set-azurermapimanagementpolicy?view=azurermps-6.13.0#parameters Tested in Azure, this works and would prevent work on implementing PolicyFilePath as another parameter. * `azurerm_traffic_manager_profile` - support for the `interval_… (hashicorp#3473) * Update CHANGELOG.md to include hashicorp#3473 * Migrate more clients to new registration pattern (hashicorp#3999) * azurerm_batch_certificate - fix thumbprint casing in tests (hashicorp#3977) * update CHANGELOG.md to include hashicorp#3977 * Update azurerm/resource_arm_cosmosdb_account.go * Update azurerm/resource_arm_cosmosdb_account.go * quote echo * update echo * change GOGC to 15 * Support for registering Data Sources and Resources from packages * tags: refactoring out to a new package ``` $ go test -v ./azurerm/internal/tags/ === RUN TestExpand --- PASS: TestExpand (0.00s) === RUN TestFilter --- PASS: TestFilter (0.00s) === RUN TestFlatten --- PASS: TestFlatten (0.00s) flatten_test.go:46: [DEBUG] Test "Empty" flatten_test.go:46: [DEBUG] Test "One Item" flatten_test.go:46: [DEBUG] Test "Multiple Items" === RUN TestValidateMaximumNumberOfTags --- PASS: TestValidateMaximumNumberOfTags (0.00s) === RUN TestValidateTagMaxKeyLength --- PASS: TestValidateTagMaxKeyLength (0.00s) === RUN TestValidateTagMaxValueLength --- PASS: TestValidateTagMaxValueLength (0.00s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags 0.021s * tags: moving the existing methods to deprecated.go * refactor: moving the top level methods * refactor: moving the networking specific method out * azurerm_analysis_services_server - correct import documentation (server -> servers) (hashicorp#4005) * change GOGC to 20 * Moved test changes to new test TestAccAzureRMMonitorDiagnosticSetting_logAnalyticsWorkspaceDedicated. * Update storage_account.html.markdown * azurerm_container_group - allow empty log_type This is not documented anywhere, as far as I can tell, but if you leave out `logType` when creating the container, you get the behavior of both `ContainerInsights` and `ContainerInstanceLogs` at once. If `logType` is not set, metadata can't be set either. * refactor: moving the validation functions into the validate package * refactor: moving into the deprecated.go * f/service-registration: updating the checklist needed to refactor into packages * azurem_container_group - add a test for unset log_type * fixing the linting * fixing the linting * refactor: moving locks into the internal package * Fix typo in private_dns_a_record docs * provider: add the disable_correlation_request_id option and red… (hashicorp#4023) * resolve conflicts * Add resource_arm_private_dns_cname_record * updating to include hashicorp#4024 * Fix review comments Co-Authored-By: Tom Harvey <tombuildsstuff@users.noreply.github.com> * Fix lint issue * dns_zone: deprecatiog the `zone_type` field * updating to include hashicorp#4028 * Update postgresql_server.html.markdown Clarified what geo_redundant_backup means. Source: https://docs.microsoft.com/en-us/azure/postgresql/concepts-backup * updating to include hashicorp#4033 * r/container_group: polling to ensure the NIC is removed from the Network Profile * removing a todo * provider: ensure test logging has date/time * updating to include hashicorp#3716 * r/storage_account: sorting the docs alphabtically * r/storage_account: defaulting the default_action field to 'Allow' * r/storage_account: moving the 'not applicable' text under the top level block definition * provider: misc golang inspection cleanup (hashicorp#4039) * Updating to include hashicorp#4013 * clarification on which API to use (hashicorp#3946) * go get github.com/hashicorp/terraform@v0.12.6; go mod tidy; go mod vendor; * updating to include hashicorp#4041 * Updating to include hashicorp#4037 * r/monitor_diagnostic_setting: fixing the formatting of the note * updating to include hashicorp#3987 * change graphrabc client registrations to new pattern * change compute client registrations to new pattern * change datalake client registrations to new pattern * change monitor client registrations to new pattern * Update servicebus_subscription_rule.html.markdown (hashicorp#4051) The import example for the subscription filter is incorrect- the resource type is 'azurerm_servicebus_subscription', where it should be 'azurerm_servicebus_subscription_rule' * change network client registrations to new pattern * change resource client registrations to new pattern * change sql client registrations to new pattern * Upped Storage share limit to 100TB; updated docs (hashicorp#4054) * Update CHANGELOG.md to include hashicorp#4054 * I noticed at the top of the page, it still said azurerm_dns_a_record instead of azurerm_private_dns_a_record * Update function_app.html.markdown (hashicorp#4073) * Update function_app.html.markdown I have run into hours of headaches with Azure in particular -- unless some default app settings are specified / seeded within my terraform code. Please include note specified under app settings (and/or something similar). We deploy with Azure DevOps release pipeline, and without these defaults, we cannot successfully deploy to a terraformed function app. * Update function_app.html.markdown * provider: increase linter deadline to 10m (hashicorp#4077) * New Resource: azurerm_mariadb_configuration (hashicorp#4060) * Update to include hashicorp#4060 * [azurerm_role_assignment] Add support for Management Groups (hashicorp#4063) Solves hashicorp#4040 * update CHANGELOG.md to include hashicorp#4063 * [resource_arm_function_app] Add missing vnet_name property (hashicorp#4078) * Update CHANGELOG.md to include hashicorp#4078
|
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
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3354
Signed-off-by: Mateusz Gozdek mateusz@kinvolk.io