DimensionDataResearch / terraform-provider-ddcloud

Terraform provider for Dimension Data cloud compute.
MIT License
16 stars 13 forks source link

When "dns_primary" && "dns_secondary" not specified, ddcloud_server cannot not be deployed #62

Closed mingsheng36 closed 7 years ago

mingsheng36 commented 7 years ago

server.tf:

resource "ddcloud_server" "Test_Server_1" {
  networkdomain = "${ddcloud_networkdomain.nd_1.id}"
  name = "Test Server 1"
  description = "Test Server 1 created using Terraform"
  admin_password = "secret"
  os_image_name = "CentOS 7 64-bit 2 CPU"
  memory_gb = 1
  cpu_count = 1
  cores_per_cpu = 1
  cpu_speed = "Standard"
  primary_adapter_ipv4 = "10.0.2.100"
  primary_adapter_type = "VMXNET3"
  dns_primary = "8.8.8.8" # cannot be deployed when not specified
  dns_secondary = "8.8.4.4" # cannot be deployed when not specified
  auto_start = "FALSE"
  disk {
    scsi_unit_id = 0
    size_gb = 10
    speed = "STANDARD"
  }
}

Error Message as follow:

* ddcloud_server.Test_Server_1: Request to deploy server 'Test Server 1' failed with 
status code 400 (INVALID_INPUT_DATA): primaryDns  is not valid IPv4 address.
* ddcloud_server.Test_Server_1: Request to deploy server 'Test Server 1' failed with status
code 400 (INVALID_INPUT_DATA): secondaryDns  is not valid IPv4 address.

Version: terraform --> v0.7.12 provider --> v1.1.8

OS: Windows 10, 64-bit

tintoy commented 7 years ago

Hmm - I could either make those mandatory or just make the default Google's DNS (8.8.8.8, 8.8.4.4). Do you have a preference?

tintoy commented 7 years ago

Fixed. Will be available in v1.1.9.

mingsheng36 commented 7 years ago

Can I suggest to leave it empty instead of specifying Google's DNS?

I think When Primary and Secondary DNS are not specified, CloudControl will default them to use MCP's Cloud DNS resolver. (Tested)

https://docs.mcp-services.net/display/CCD/Cloud+DNS+Resolvers for different regions.

tintoy commented 7 years ago

Wasn't it complaining about them not being supplied?

mingsheng36 commented 7 years ago

From CloudControl API point of view, if you do not declare primary_dns or secondary_dns, it will still allow you to deploy the server using its own default DNS for each region.

From dd-cloud-compute-terraform, you have to declare the primary_dns and secondary_dns no matter what. (Even if you leave it as blank or did not specify)

e.g. Server will not be able to deploy with the codes below, it will prompt you to specify primary_dns and secondary_dns

resource "ddcloud_server" "Test_Server_1" {
  networkdomain = "${ddcloud_networkdomain.nd_1.id}"
  name = "Test Server 1"
  description = "Test Server 1 created using Terraform"
  admin_password = "Cloud@123"
  os_image_name = "Redhat 7 64-bit 2 CPU"
  memory_gb = 1
  cpu_count = 1
  cores_per_cpu = 1
  cpu_speed = "HIGHPERFORMANCE"
  primary_adapter_ipv4 = "10.0.2.100"
  primary_adapter_type = "VMXNET3"
// dns_primary = "8.8.8.8" 
// dns_secondary = "8.8.4.4"
  auto_start = "TRUE"
  disk {
    scsi_unit_id = 0
    size_gb = 21
    speed = "HIGHPERFORMANCE"
  }
}
tintoy commented 7 years ago

Ok - we'll leave the field out entirely; CloudControl API docs say it should be optional. So yes it'll now use the CloudControl defaults if you don't specify one.

tintoy commented 7 years ago

Yes - have a look at the change I just made - we now omit those fields from the JSON if they are empty (which makes all the difference).

mingsheng36 commented 7 years ago

Looks good to me

mingsheng36 commented 7 years ago

One last thing before I close this issue. Is it possible to remove the "force new" from primary & secondary dns? Reason is because these are configurable from the OS and does not make sense to re-deploy the VM because of update on the DNS entry? If there are updates to the primary & secondary DNS I think a prompt or reminder for them to change at OS will be good enough.

Also if the below validation could be added during "plan" it would be great!

Any view on this @tintoy ?

wninobla commented 7 years ago

Not sure if I'm reading this right but changing the DNS values should not force a new deploy of new systems. If I recall it wasn't that behavior in my deployments. As for the the DNS fields in general, due to howTerraform is scoped out I would say that a change on one invites changes on both since this is a configuration toool for deployments and not one for OS changes. That should be done via post config tools like Ansible.

Just my two cents worth.

tintoy commented 7 years ago

Hmm, I think this might be due to a mismatch in expectations. What ForceNew implies is that you can't change a value after the resource has been created. In this case, you can't ask CloudControl to change the DNS IPs because that only works during deployment.

So how do we communicate to the user that primary_dns and secondary_dns cannot be modified (via Terraform at least) after deployment?

Or to put it another way, let's turn it around - why do you want to modify primary_dns and / or secondary_dns for a ddcloud_server that has already been deployed?

tintoy commented 7 years ago

BTW, @mingsheng36, cross-property validation cannot be done during the plan stage. Only during apply (Terraform limitation unfortunately).

wninobla commented 7 years ago

Actually that makes sense now that I think about it as the DNS values are considered database values from the cloud UI standpoint. It just so happens that it will use the guest OS customization to deploy that initially. Changes to the values because of that will then precipitate a new server build. TheIpv4 address may be different as it uses the VM tools to change stuff but its not at all consistent. So, I’d tend to agree with Adam here on the behavior.

tintoy commented 7 years ago

But I believe we could validate those 2 properties against each other if we moved them down a level:

resource "ddcloud_server" "xxx" {
  name = "yyy"
  # ...
  dns {
    primary   = "8.8.8.8"
    secondary = "8.8.4.4"
  }
}

Then we can have an overall validator for the dns property that checks values for primary and secondary`.

tintoy commented 7 years ago

But I believe we could validate those 2 properties against each other if we moved them down a level

Actually, no, it looks like we can't (just checked the Terraform source code, it you can't have plan-time validation for Map-type properties. Ugh. Sorry, this is in the too-hard basket for now. We'll have to stick with doing any and all cross-property validation at apply-time.

Having said that, I could make the entire dns property optional but then make both its primary and its secondary properties required. That'd accomplish what you're after - "either both or neither, but not just one". Unless you expect them to be able to supply primary but not secondary? In which case it can't be done until we apply :-/

mingsheng36 commented 7 years ago

I would agree with @tintoy and choose to go with both or nothing.

Single Primary DNS i think is quite uncommon to me. But @wninobla might have some opinion on that? :)

tintoy commented 7 years ago

I'm closing this issue, but we can create a new issue for the enhancement to split up the DNS fields so they're either both required or neither.