backfill region from hcl for jobUpdate and jobPlan

[jazzyfresh]

Overview

This pull request is about region precedence in a job (http args > hcl job config > defaults). This feature adds a check in the jobPlan & jobUpdate http endpoints: it checks the hcl arg for region if none is provided in the region arg, and otherwise defaults to a "global" region.

Implementation

• In the command agent server, in the jobPlan & jobUpdate endpoints, we now backfill the region from the job hcl, if it is not already present in the http request as a separate parameter.
• I fixed a bug where we were persisting stale Job data to the Nomad internal datastore
• I fixed a bunch of tests that broke because they were using invalid region values (these tests passed because previous behavior ignored job hcl region and defaulted to "global", but my fix now picks up the invalid region value and the server rejects the job; so I fixed these tests to use the default region value "global").
  • I changed several tests back because they were depending on special values of region (TLS tests are looking for a Nomad server name with "regionFoo" because of hard-coded certificate test data)

Behavior

Existing behavior:

• When a user submits a job via the CLI, the CLI tool parses the job hcl for a region, and then adds that to the http request as a region parameter separate from the job hcl parameter.
• The UI submits a job via HTTP (bypassing the CLI parsing), which passes the region value inside the job hcl arg

New behavior:

• This feature adds a check in the jobPlan & jobUpdate http endpoints: it checks the hcl arg for region if none is provided in the region arg, and otherwise defaults to a "global" region.
• This feature preserves existing behavior for backwards compatibility.

region present in http arg	region present in job hcl	result
yes	yes	http region takes precedence
yes	no	http region takes precedence
no	yes	hcl takes precedence
no	no	default to global

This is ready for review

[hashicorp-cla]

All committers have signed the CLA.

[notnoop]

q q about job Region being nil - but code lgtm otherwise.

[endocrimes]

We'll also need to update the command/util_test.go helpers to default to the global region, which may involve also updating some other tests. The Config Region being ignored resulted in a few accidentally passing tests where job registration happens 😞

[endocrimes]

This is looking really good! - All that's left is updating the jobPlan code then it lgtm!

[jazzyfresh]

@endocrimes I made the suggested changes. I am just holding off on merging to make sure all the travis tests pass.

[jazzyfresh]

I forgot I also have to update the jobPlanRegion test. I am giving it parameterized test cases like jobUpdateRegion, but let me know if that is overkill

[jazzyfresh]

It looks like one more TLS bug fix is necessary. Now it's failing for the right reasons (there is no path to the "regionFoo" region) https://travis-ci.org/hashicorp/nomad/jobs/544292291#L620

UPDATE: not all TLS tests require "regionFoo", some had originally used "global"

[preetapan]

Looks ready to merge to me, but @endocrimes should give it the final thumbs up

[endocrimes]

This is great - thanks for getting all of the tests updated! 🎉

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.