TritonDataCenter / terraform-provider-triton

Terraform Joyent Triton provider
https://www.terraform.io/docs/providers/triton/
Mozilla Public License 2.0
15 stars 24 forks source link

Network data provider causes diff on every/next update #30

Closed jwreagor closed 7 years ago

jwreagor commented 7 years ago

Terraform development question here...

Instances always have a different IP pool UUID after provisioning. This will always differ from the network UUID given by the network data provider prior to creation.

I'm curious how we can fix this. I guess the provider should ignore updates to this field or I could try to get hack-ey with the Schema.DiffSuppressFunc.

Path: base.plan

  ~ triton_machine.base0
      nic.3296163886.network: "03bc2ec8-821e-4646-99a2-6db7bd2b74b3" => ""
      nic.4288238514.gateway: "" => "<computed>"
      nic.4288238514.ip:      "" => "<computed>"
      nic.4288238514.mac:     "" => "<computed>"
      nic.4288238514.netmask: "" => "<computed>"
      nic.4288238514.network: "" => "4232aea0-eb62-a4c1-8eb6-0af3e2f83abc"
      nic.4288238514.primary: "" => "<computed>"
      nic.4288238514.state:   "" => "<computed>"

  ~ triton_machine.base1
      nic.3296163886.network: "03bc2ec8-821e-4646-99a2-6db7bd2b74b3" => ""
      nic.4288238514.gateway: "" => "<computed>"
      nic.4288238514.ip:      "" => "<computed>"
      nic.4288238514.mac:     "" => "<computed>"
      nic.4288238514.netmask: "" => "<computed>"
      nic.4288238514.network: "" => "4232aea0-eb62-a4c1-8eb6-0af3e2f83abc"
      nic.4288238514.primary: "" => "<computed>"
      nic.4288238514.state:   "" => "<computed>"

Plan: 0 to add, 2 to change, 0 to destroy.
{
    "id": "03bc2ec8-821e-4646-99a2-6db7bd2b74b3",
    "name": "JoyentSDC-165.225.148.0/22",
    "public": true
}
{
    "id": "4232aea0-eb62-a4c1-8eb6-0af3e2f83abc",
    "name": "Joyent-SDC-Public",
    "public": true,
    "description": "Public networks with IP addresses routable over the internet"
}
berglh commented 7 years ago

@cheapRoc I'm experiencing this same problem.

However, I'd have a hard time actually calling it a bug. Using the sdc-createmachine tool, I've found that the original network UUID provided in the machine creation is not stored anywhere in the machine state in Triton/SDC. Instead, as you point out, the UUID of the nic instance is stored in the nic.instancen#.network and networks.0 attributes in the Terraform state fie.

From what I can tell, there is no way to map back to the appropriate Triton/SDC network, even inspecting the IP is not reliable:

~$ sdc-listnetworks
[
  {
    "id": "3a9e82d8-f4b7-43cd-89c6-d512a0431938",
    "name": "fabric_nat",
    "public": false
  },
  {
    "id": "8f1cb10a-f81c-4088-b42f-1887b64be049",
    "name": "zones",
    "public": false
  },
  {
    "id": "953e4283-db5f-4210-b204-839531da9c9",
    "name": "zones-public",
    "public": false
  }

So two points:

  1. Triton ~should~ could store the original network UUID attribute in the cluster state
  2. Any fix made in the Terraform provider will be a work-around until number one is addressed

Without being able to reference the original network UUID, it will be difficult to determine whether or not an update is needed or the current configuration is a match. One possible work around for this would be to use a machine tags to store data regarding the nic instance networks.

I'm also unsure on how the nic.instancenumber object gets it's number. In the example below, the number between the nic. prefix and the nic attribute; is this something allocated by Terraform or Triton? I couldn't see this anywhere in the Triton output. Maybe it's something that can be stored in the Terraform state file with the network id and compare that value instead?

      nic.3296163886.network: "03bc2ec8-821e-4646-99a2-6db7bd2b74b3" => ""
      nic.4288238514.gateway: "" => "<computed>"
      nic.4288238514.ip:      "" => "<computed>"
      nic.4288238514.mac:     "" => "<computed>"
      nic.4288238514.netmask: "" => "<computed>"
      nic.4288238514.network: "" => "4232aea0-eb62-a4c1-8eb6-0af3e2f83abc"
      nic.4288238514.primary: "" => "<computed>"
      nic.4288238514.state:   "" => "<computed>"
jwreagor commented 7 years ago

It appears there is a workaround to this issue that I found through code review. Via @sean-

lifecycle {
    ignore_changes = [
      "nic",
    ]
  }

As @Smithx10 pointed out in chat earlier, sdc-napi contains a mapping of network pools to their network UUID. I don't know the particulars of exposing this through CloudAPI but it's possible we could use this to match the network ID with the network pool ID. It's worth noting here anyway.

bahamat commented 7 years ago

Is there any reason we can't create a new data type of triton_pool, such that the pool uuid is only used at provision time and the network uuid is <computed>?

It seems to me that this would solve the problem. By design, with Triton when you provision with a pool the actual network uuid is not known ahead of time and is returned in the instance json. This doesn't seem to be any different than the ips value.

jwreagor commented 7 years ago

An offline discussion derived the following fix. It sounds like it's worth considering to resolve the mis-use of network pool UUID versus computed network UUID.

it might help to change the language of what we have today to network_pool and just store the network_uuid on the instance after its created

I'm also wondering if we keep nic.*.network as the attribute for the returned/computed network UUID and simply replace the input as network_pool.

Need to think about what this may or may not break and if it's doable.

bahamat commented 7 years ago

While the new behavior is desired, I don't think changing the name or terminology is necessary. Pools can be detected with ListNetworks. If the network doesn't include subnet, then it's a pool and the pool behavior (i.e., the network of the nic is computed) should be used.

But otherwise, I like this suggestion very much.

jwreagor commented 7 years ago

Sounds like we're going with accepting the computed value into a field that is used for the returned value only. nic.*.network will stay the same and we'll accept the computed value into provisioned_network.

jwreagor commented 7 years ago

Posting our resolution for this matter since this is now a deprecation.

From @stack72...


There was an issue that when you specified a pool as a ni network id, the provisioned network id would be different to that supplied to the machine and thus create a perpetual diff

We decided that networks for the machine to be part of and nics attached to the machine are different enough for them to be separate. Therefore, we un-deprecated networks and we now pass a list of network ids for the machine to be attached to

nic is now used as a computed parameter rather than an input. Meaning we can do the following:

provider "triton" {}

data "triton_image" "image" {
    name = "base-64-lts"
    version = "16.4.1"
}

data "triton_network" "public" {
    name = "Joyent-SDC-Public"
}

resource "triton_vlan" "test" {
      vlan_id = 100
      name = "test-vlan"
      description = "test vlan"
}

resource "triton_fabric" "test" {
    name = "test-network"
    description = "test network"
    vlan_id = "${triton_vlan.test.vlan_id}"

    subnet = "10.10.0.0/24"
    gateway = "10.10.0.1"
    provision_start_ip = "10.10.0.10"
    provision_end_ip = "10.10.0.250"

    resolvers = ["8.8.8.8", "8.8.4.4"]
}

resource "triton_machine" "test" {
    package = "g4-highcpu-128M"
    image   = "${data.triton_image.image.id}"
    networks = ["${data.triton_network.public.id}", "${triton_fabric.test.id}"]
}

Much simplier setup of triton_machine and doesn't give the perpetual diff.