backfill region from job hcl in jobUpdate and jobPlan endpoints

- updated region in job metadata that gets persisted to nomad datastore
- fixed many unrelated unit tests that used an invalid region value
(they previously passed because hcl wasn't getting picked up and
the job would default to global region)
This commit is contained in:
Jasmine Dahilig
2019-05-02 13:00:21 -07:00
parent 01c267b92b
commit c467a94e2b
13 changed files with 210 additions and 22 deletions

View File

@@ -188,7 +188,7 @@ func TestConfig_Merge(t *testing.T) {
}
c3 := &Config{
Region: "region2",
Region: "global",
Datacenter: "dc2",
NodeName: "node2",
DataDir: "/tmp/dir2",

View File

@@ -8,6 +8,7 @@ import (
"github.com/golang/snappy"
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/jobspec"
"github.com/hashicorp/nomad/nomad/structs"
)
@@ -140,13 +141,20 @@ func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request,
return nil, CodedError(400, "Job ID does not match")
}
// Http region takes precedence over hcl region
if args.WriteRequest.Region != "" {
args.Job.Region = helper.StringToPtr(args.WriteRequest.Region)
}
// If no region given, region is canonicalized to 'global'
sJob := ApiJobToStructJob(args.Job)
planReq := structs.JobPlanRequest{
Job: sJob,
Diff: args.Diff,
PolicyOverride: args.PolicyOverride,
WriteRequest: structs.WriteRequest{
Region: args.WriteRequest.Region,
Region: sJob.Region,
},
}
s.parseWriteRequest(req, &planReq.WriteRequest)
@@ -374,6 +382,12 @@ func (s *HTTPServer) jobUpdate(resp http.ResponseWriter, req *http.Request,
return nil, CodedError(400, "Job ID does not match name")
}
// Http region takes precedence over hcl region
if args.WriteRequest.Region != "" {
args.Job.Region = helper.StringToPtr(args.WriteRequest.Region)
}
// If no region given, region is canonicalized to 'global'
sJob := ApiJobToStructJob(args.Job)
regReq := structs.JobRegisterRequest{
@@ -382,7 +396,7 @@ func (s *HTTPServer) jobUpdate(resp http.ResponseWriter, req *http.Request,
JobModifyIndex: args.JobModifyIndex,
PolicyOverride: args.PolicyOverride,
WriteRequest: structs.WriteRequest{
Region: args.WriteRequest.Region,
Region: sJob.Region,
AuthToken: args.WriteRequest.SecretID,
},
}

View File

@@ -465,6 +465,99 @@ func TestHTTP_JobUpdate(t *testing.T) {
})
}
func TestHTTP_JobUpdateRegion(t *testing.T) {
t.Parallel()
cases := []struct {
Name string
ConfigRegion string
APIRegion string
ExpectedRegion string
}{
{
Name: "api region takes precedence",
ConfigRegion: "not-global",
APIRegion: "north-america",
ExpectedRegion: "north-america",
},
{
Name: "config region is set",
ConfigRegion: "north-america",
APIRegion: "",
ExpectedRegion: "north-america",
},
{
Name: "api region is set",
ConfigRegion: "",
APIRegion: "north-america",
ExpectedRegion: "north-america",
},
{
Name: "falls back to default if no region is provided",
ConfigRegion: "",
APIRegion: "",
ExpectedRegion: "global",
},
}
for _, tc := range cases {
t.Run(tc.Name, func(t *testing.T) {
httpTest(t, func(c *Config) { c.Region = tc.ExpectedRegion }, func(s *TestAgent) {
// Create the job
job := MockRegionalJob()
if tc.ConfigRegion == "" {
job.Region = nil
} else {
job.Region = &tc.ConfigRegion
}
args := api.JobRegisterRequest{
Job: job,
WriteRequest: api.WriteRequest{
Namespace: api.DefaultNamespace,
Region: tc.APIRegion,
},
}
buf := encodeReq(args)
// Make the HTTP request
url := "/v1/job/" + *job.ID
req, err := http.NewRequest("PUT", url, buf)
require.NoError(t, err)
respW := httptest.NewRecorder()
// Make the request
obj, err := s.Server.JobSpecificRequest(respW, req)
require.NoError(t, err)
// Check the response
dereg := obj.(structs.JobRegisterResponse)
require.NotEmpty(t, dereg.EvalID)
// Check for the index
require.NotEmpty(t, respW.HeaderMap.Get("X-Nomad-Index"), "missing index")
// Check the job is registered
getReq := structs.JobSpecificRequest{
JobID: *job.ID,
QueryOptions: structs.QueryOptions{
Region: tc.ExpectedRegion,
Namespace: structs.DefaultNamespace,
},
}
var getResp structs.SingleJobResponse
err = s.Agent.RPC("Job.GetJob", &getReq, &getResp)
require.NoError(t, err)
require.NotNil(t, getResp.Job, "job does not exist")
require.Equal(t, tc.ExpectedRegion, getResp.Job.Region)
})
})
}
}
func TestHTTP_JobDelete(t *testing.T) {
t.Parallel()
httpTest(t, nil, func(s *TestAgent) {
@@ -1022,6 +1115,81 @@ func TestHTTP_JobPlan(t *testing.T) {
})
}
func TestHTTP_JobPlanRegion(t *testing.T) {
t.Parallel()
cases := []struct {
Name string
ConfigRegion string
APIRegion string
ExpectedRegion string
}{
{
Name: "api region takes precedence",
ConfigRegion: "not-global",
APIRegion: "north-america",
ExpectedRegion: "north-america",
},
{
Name: "config region is set",
ConfigRegion: "north-america",
APIRegion: "",
ExpectedRegion: "north-america",
},
{
Name: "api region is set",
ConfigRegion: "",
APIRegion: "north-america",
ExpectedRegion: "north-america",
},
{
Name: "falls back to default if no region is provided",
ConfigRegion: "",
APIRegion: "",
ExpectedRegion: "global",
},
}
for _, tc := range cases {
t.Run(tc.Name, func(t *testing.T) {
httpTest(t, func(c *Config) { c.Region = tc.ExpectedRegion }, func(s *TestAgent) {
// Create the job
job := MockRegionalJob()
if tc.ConfigRegion == "" {
job.Region = nil
} else {
job.Region = &tc.ConfigRegion
}
args := api.JobPlanRequest{
Job: job,
Diff: true,
WriteRequest: api.WriteRequest{
Region: tc.APIRegion,
Namespace: api.DefaultNamespace,
},
}
buf := encodeReq(args)
// Make the HTTP request
req, err := http.NewRequest("PUT", "/v1/job/"+*job.ID+"/plan", buf)
require.NoError(t, err)
respW := httptest.NewRecorder()
// Make the request
obj, err := s.Server.JobSpecificRequest(respW, req)
require.NoError(t, err)
// Check the response
plan := obj.(structs.JobPlanResponse)
require.NotNil(t, plan.Annotations)
require.NotNil(t, plan.Diff)
})
})
}
}
func TestHTTP_JobDispatch(t *testing.T) {
t.Parallel()
httpTest(t, nil, func(s *TestAgent) {

View File

@@ -109,3 +109,9 @@ func MockPeriodicJob() *api.Job {
}
return j
}
func MockRegionalJob() *api.Job {
j := MockJob()
j.Region = helper.StringToPtr("north-america")
return j
}