[WIP] nomad_job: richer diff, and catch errors during plan

[apparentlymart]

Using the new CustomizeDiff mechanism, we can verify that the given jobspec is valid at plan time, and then include some information in the diff showing high-level changes that are implied by the jobspec changes.

We can also use the JobModifyIndex mechanism to detect if changes are made outside of Terraform between our various operations, thus helping us keep our promise that Terraform will do what the plan says or produce an error.

---

Here's what the new diff output looks like:

  + nomad_job.test
      id:                         <computed>
      datacenters.#:              "2"
      datacenters.2213181284:     "dc2"
      datacenters.2599503397:     "dc3"
      deregister_on_destroy:      "true"
      deregister_on_id_change:    "true"
      jobspec:                    "job \"foo\" {\n  datacenters = [\"dc2\", \"dc3\"]\n  region = \"global\"\n  type = \"service\"\n\n  group \"foo\" {\n    count = 2\n\n    task \"foo\" {\n      driver = \"raw_exec\"\n      config = {\n        command = \"/bin/sleep\"\n        args = [\"25\"]\n      }\n\n      resources {\n        cpu = 20\n        memory = 10\n      }\n    }\n  }\n}\n\n"
      modify_index:               <computed>
      name:                       "foo"
      task_group.#:               "1"
      task_group.0.count:         "2"
      task_group.0.meta.%:        0
      task_group.0.name:          "foo"
      task_group.0.task.#:        "1"
      task_group.0.task.0.driver: "raw_exec"
      task_group.0.task.0.meta.%: 0
      task_group.0.task.0.name:   "foo"

The attributes like datacenters, name, task_group etc are from the parsed jobspec. During an update diff we are able to show changes to these attributes, giving a better sense of the impact of a change without having to mentally parse the raw jobspec diff.

[apparentlymart]

@vancluever while writing this I noticed that the d.ForceNew(...) function doesn't seem to be having the desired effect in my CustomizeDiff implementation. Is that a side-effect of the diff comparison change we had to back out due to the "diffs didn't match during apply" regression?

If it is, then we'll probably need to hold on this until we have a change to tweak the internal handling of diffs so we can do such comparisons 😖, or I could revise this to continue to do the manual deregistering it was doing before, I suppose.

[vancluever]

Hey @apparentlymart, sorry for not getting back on this one...

ForceNew should still work, but it doesn't work by directly altering the diff, but rather by flagging the attribute as ForceNew in a copy of the schema and then letting the diff do its thing with that schema.

There are tests for it, so I'd hope it would be working in some regard at least :)

Looking quickly at your example, for ForceNew to work correctly, id or name would need to be changing. I'm not too sure how id shows up in a diff, as I think there's been some discussion on how it's a "special" attribute in the past, but name seems safe, as long as it's actually being altered.

[shantanugadgil]

@apparentlymart
Any updates on this, this would be a very welcome change.

My current workflow is all based on Nomad command-line usage, and thought Terraform+Nomad would be a much "easier to maintain" alternative.

Though, I see a few things that would help users (me) 😈

• exportable values from the nomad job like "name", "region", "datacenter", "allocation ids", etc..
  Until the terraform provider is enhanced, I can use the output variables with my existing nomad cmdline usage.

My usual workflow is (which I hope to simplify):

nomad status <job_name> (look at the latest alloc id)
nomad status <alloc id> (to check status, until Consul HC marks it healthy)

Regards,
Shantanu

[apparentlymart]

I am not currently working on this because my attention is focused on the next major release of Terraform Core.

I think the code here is a reasonable prototype but needs some more thought about how best to deal with the jobspec format so that the code here doesn't need to change each time upstream Nomad does.

We may also need to think about some upstream Nomad changes to make the diffs here comprehensive. Nomad diff output it currently aimed at the needs of its own UI, which has different needs than Terraform. (Terraform wants to see the old and new structures in full, not a list of changes.)

[kerscher]

If anyone were to pick this up to solve #1, what’s holding back finishing it? Could a first implementation attempt to read the jobspec, and if the format changed as mentioned above return an error that flags the provider has missing functionality?

[apparentlymart]

I'm a bit rusty here but I think this change as-is wouldn't address #1 because the CustomizeDiff function aborts early if the jobspec hasn't changed.

I think being able to detect upstream changes here would require both that the provider parse the jobspec during planning (as the code in this PR already does) and retrieve the current settings from the API during refresh, and then the CustomizeDiff function could in principle correlate the current settings (now stored in state) with the new settings (parsed from the jobspec) and force a diff to be produced using ResourceDiff.SetNew.

I think this could be done separately from what this PR was trying to achieve, which was to use the nomad plan functionality to also predict the effect of submitting the job. Excluding this extra bit of functionality will probably make things easier, since it won't be necessary to contend with how nomad's API returns the set of proposed changes.

The provider is already sensitive to jobspec changes because the parsing is happening client-side anyway, so I don't think implementing the above (that is, not including the "nomad plan" portion) would put us in any worse a position as we are now in that regard, but it would require mapping all of the configurable fields of Job and all of its nested fields into Terraform schema and translating between the API responses and Terraform's representation.

[cgbaker]

@shantanugadgil , you mentioned in your comment above that you would like to see allocation IDs as an exported variable... As part of merging this PR, I'm adding that capability for you. However, I'd like some feedback on the use case. The Nomad API endpoint for that returns all allocations associated with this job, stopped or otherwise. Is that okay for your use case? You suggested that you were manually using the Nomad CLI to grab that latest allocation.

[shantanugadgil]

@cgbaker thanks for this, though the reason for asking for an exportable value is now a bit hazy to me. Probably I wanted to make the "status" into an output variable and keep doing a terraform refresh.
(I am quite unsure at the moment).

Although the Nomad provider has been making excellent progress, I haven't been able to test it through Terraform in recent times.

[cgbaker]

I'm closing this in favor of #63 .