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

azurerm_container_group - Support of dns_config #7912

Merged
merged 16 commits into from
Sep 27, 2020

Conversation

zparnold
Copy link
Contributor

Closes #6360

This PR attempts to add in the DNS config functionality for ACI. Please let me know if I missed something for the AzureRM provider. I think? I hit all the required terraform bits. Reference to the JSON schema for ACI below that I implemented.

https://github.com/Azure/azure-rest-api-specs/blob/21f0dcbb89544101d79be7fc889a86ce0911901b/specification/containerinstance/resource-manager/Microsoft.ContainerInstance/stable/2018-10-01/examples/ContainerGroupsCreateOrUpdate.json#L73

@zparnold zparnold changed the title initial dns config attempt azurerm_container_group add dns_config block to definition Jul 29, 2020
@zparnold
Copy link
Contributor Author

@jackofallops I'm not sure if you're a maintainer, but if you are, and aren't too busy could I please have a review?

@zparnold
Copy link
Contributor Author

zparnold commented Aug 3, 2020

Bump. We are needing this functionality for some internal dev work we have.

Copy link
Collaborator

@magodo magodo left a comment

Choose a reason for hiding this comment

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

@zparnold Thank you for this PR 👍

Having look thourgh the code and left some comments mainly on the schema design. Besides, can you please add the test for this new property to ensure it works?

Thank you!

@ash007osh
Copy link

Has this been implemented in Terraform release?

@zparnold
Copy link
Contributor Author

@magodo I hopefully have addressed all PR comments.

@zparnold zparnold requested a review from magodo September 25, 2020 03:43
Copy link
Collaborator

@magodo magodo left a comment

Choose a reason for hiding this comment

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

Thank you for the update! I have left some additional mostly minor comments that once addressed this should be good to merge 👍

website/docs/r/container_group.html.markdown Outdated Show resolved Hide resolved
@zparnold
Copy link
Contributor Author

@magodo anything else wrong with this?

@zparnold
Copy link
Contributor Author

@magodo when I take your suggestion and do the []interface{} approach here is what I get when I run acceptance tests locally. I strongly advise that we go back to my original function signature and design:

=== RUN   TestAccAzureRMContainerGroup_virtualNetwork
=== PAUSE TestAccAzureRMContainerGroup_virtualNetwork
=== CONT  TestAccAzureRMContainerGroup_virtualNetwork
panic: interface conversion: interface {} is *schema.Set, not []interface {}

goroutine 495 [running]:
github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers.expandContainerGroupDnsConfig(0x55f5760, 0xc001243320, 0xc001338f80)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers/container_group_resource.go:1447 +0x7cf
github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers.resourceArmContainerGroupCreate(0xc000b8dce0, 0x51bb4a0, 0xc000824dc0, 0x0, 0x0)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers/container_group_resource.go:508 +0x4ff
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Resource).Apply(0xc00050d5f0, 0xc00207d5e0, 0xc000b37cc0, 0x51bb4a0, 0xc000824dc0, 0xc0010d9901, 0xc001409770, 0xc0010d99a0)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/schema/resource.go:310 +0x365
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Provider).Apply(0xc000132b00, 0xc001db79c8, 0xc00207d5e0, 0xc000b37cc0, 0xc0010d9738, 0xc0012db9f0, 0x5187860)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider.go:294 +0x99
github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin.(*GRPCProviderServer).ApplyResourceChange(0xc0012dbb28, 0x623d6c0, 0xc0013a8030, 0xc000b8d7a0, 0xc0012dbb28, 0xc0013a8030, 0xc000329b30)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin/grpc_provider.go:885 +0x8b4
github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5._Provider_ApplyResourceChange_Handler(0x56112e0, 0xc0012dbb28, 0x623d6c0, 0xc0013a8030, 0xc000e03440, 0x0, 0x623d6c0, 0xc0013a8030, 0xc0018bb800, 0x79c)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5/tfplugin5.pb.go:3305 +0x217
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0016d7680, 0x62a48e0, 0xc0017ae180, 0xc000e78b00, 0xc0017471d0, 0x94dc0c0, 0x0, 0x0, 0x0)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/google.golang.org/grpc/server.go:1024 +0x4f4
google.golang.org/grpc.(*Server).handleStream(0xc0016d7680, 0x62a48e0, 0xc0017ae180, 0xc000e78b00, 0x0)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/google.golang.org/grpc/server.go:1313 +0xd97
google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc0016375e0, 0xc0016d7680, 0x62a48e0, 0xc0017ae180, 0xc000e78b00)
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/google.golang.org/grpc/server.go:722 +0xbb
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/Users/zarnold/go/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/google.golang.org/grpc/server.go:720 +0xa1

Process finished with exit code 1

@zparnold
Copy link
Contributor Author

I have added something which fixes the acceptance test. Let me know if it breaks something else.

@magodo
Copy link
Collaborator

magodo commented Sep 27, 2020

@zparnold LGTM!

image

@magodo magodo added this to the v2.30.0 milestone Sep 27, 2020
@magodo magodo changed the title azurerm_container_group add dns_config block to definition azurerm_container_group - Support of dns_config Sep 27, 2020
@magodo magodo merged commit 02da519 into hashicorp:master Sep 27, 2020
magodo added a commit that referenced this pull request Sep 27, 2020
update CHANGELOG.md for #7912
@ghost
Copy link

ghost commented Oct 1, 2020

This has been released in version 2.30.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.30.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Oct 27, 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 as resolved and limited conversation to collaborators Oct 27, 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 azurerm_container_group for setting DNS in private networks
3 participants