poddm commented on PR #67: URL: https://github.com/apache/cloudstack-terraform-provider/pull/67#issuecomment-3277202517
@kiranchavala, I saw the devlist you were releasing v0.6.0. I'd like to get traction here - i've had this PR open for quite awhile and am using it in our internal fork. I've rebased from the latest and a number of these resources have been added since. However, there are some minor issues in regards to implementation and provider conventions: https://developer.hashicorp.com/terraform/plugin/best-practices. 1. some of the resources have ambiguous fields that don't align to the API . For example `zone`, is it an ID or Name? I've back ported this PR change these instance to `zone_id` following terraform's best practices. 2. minor but `cloudstack_physicalnetwork` vs `cloudstack_physical_network` > Function names should be all lowercase and use underscores to separate words. The built-in functions in the Terraform language do not use underscores for historical reasons, but all functions defined in providers should use underscores to improve readability. 3. Physical network module was previously keyed off the name field instead of id. I believe this may cause issues where nested dependent resources are not recreated or marked as tainted. Example, if zone is recreated I dont think physical networks resource will be, as the name never changed. ```hcl resource "cloudstack_zone" "foo" { name = "terraform-zone" dns1 = "8.8.8.8" internal_dns1 = "8.8.4.4" network_type = "Advanced" } resource "cloudstack_physical_network" "foo" { name = "terraform-physical-network" zone = cloudstack_zone.foo.name # ----> If the name is the same the phsyical network wont be recreated broadcast_domain_range = "ZONE" isolation_methods = ["VLAN"] } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
