luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

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 85760a13ae
commit 51e141be7a
13 changed files with 210 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api/compose_test.go
View File

@ -37,7 +37,7 @@ func TestCompose(t *testing.T) {
AddTask(task)
// Compose a job
job := NewServiceJob("job1", "myjob", "region1", 2).
job := NewServiceJob("job1", "myjob", "global", 2).
SetMeta("foo", "bar").
AddDatacenter("dc1").
Constrain(NewConstraint("kernel.name", "=", "linux")).
@ -45,7 +45,7 @@ func TestCompose(t *testing.T) {
// Check that the composed result looks correct
expect := &Job{
Region: stringToPtr("region1"),
Region: stringToPtr("global"),
ID: stringToPtr("job1"),
Name: stringToPtr("myjob"),
Type: stringToPtr(JobTypeService),

8
api/jobs_test.go
View File

@ -1206,9 +1206,9 @@ func TestJobs_JobSummary(t *testing.T) {
func TestJobs_NewBatchJob(t *testing.T) {
t.Parallel()
job := NewBatchJob("job1", "myjob", "region1", 5)
job := NewBatchJob("job1", "myjob", "global", 5)
expect := &Job{
Region: stringToPtr("region1"),
Region: stringToPtr("global"),
ID: stringToPtr("job1"),
Name: stringToPtr("myjob"),
Type: stringToPtr(JobTypeBatch),
@ -1221,9 +1221,9 @@ func TestJobs_NewBatchJob(t *testing.T) {
func TestJobs_NewServiceJob(t *testing.T) {
t.Parallel()
job := NewServiceJob("job1", "myjob", "region1", 5)
job := NewServiceJob("job1", "myjob", "global", 5)
expect := &Job{
Region: stringToPtr("region1"),
Region: stringToPtr("global"),
ID: stringToPtr("job1"),
Name: stringToPtr("myjob"),
Type: stringToPtr(JobTypeService),

2
api/util_test.go
View File

@ -39,7 +39,7 @@ func testJob() *Job {
SizeMB: intToPtr(25),
})
job := NewBatchJob("job1", "redis", "region1", 1).
job := NewBatchJob("job1", "redis", "global", 1).
AddDatacenter("dc1").
AddTaskGroup(group)

12
client/client_test.go
View File

@ -986,7 +986,7 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) {
assert := assert.New(t)
s1, addr := testServer(t, func(c *nomad.Config) {
c.Region = "regionFoo"
c.Region = "global"
})
defer s1.Shutdown()
testutil.WaitForLeader(t, s1.RPC)
@ -1006,7 +1006,7 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) {
{
req := structs.NodeSpecificRequest{
NodeID: c1.Node().ID,
QueryOptions: structs.QueryOptions{Region: "regionFoo"},
QueryOptions: structs.QueryOptions{Region: "global"},
}
testutil.WaitForResult(func() (bool, error) {
@ -1040,7 +1040,7 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) {
{
req := structs.NodeSpecificRequest{
NodeID: c1.Node().ID,
QueryOptions: structs.QueryOptions{Region: "regionFoo"},
QueryOptions: structs.QueryOptions{Region: "global"},
}
testutil.WaitForResult(func() (bool, error) {
var out structs.SingleNodeResponse
@ -1062,7 +1062,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) {
assert := assert.New(t)
s1, addr := testServer(t, func(c *nomad.Config) {
c.Region = "regionFoo"
c.Region = "global"
})
defer s1.Shutdown()
testutil.WaitForLeader(t, s1.RPC)
@ -1091,7 +1091,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) {
{
req := structs.NodeSpecificRequest{
NodeID: c1.Node().ID,
QueryOptions: structs.QueryOptions{Region: "regionFoo"},
QueryOptions: structs.QueryOptions{Region: "global"},
}
testutil.WaitForResult(func() (bool, error) {
var out structs.SingleNodeResponse
@ -1116,7 +1116,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) {
{
req := structs.NodeSpecificRequest{
NodeID: c1.Node().ID,
QueryOptions: structs.QueryOptions{Region: "regionFoo"},
QueryOptions: structs.QueryOptions{Region: "global"},
}
testutil.WaitForResult(func() (bool, error) {
var out structs.SingleNodeResponse

2
command/agent/config_test.go
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",

18
command/agent/job_endpoint.go
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,
},
}

168
command/agent/job_endpoint_test.go
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) {

6
command/agent/testingutils_test.go
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
}

2
command/util_test.go
View File

@ -42,7 +42,7 @@ func testJob(jobID string) *api.Job {
SizeMB: helper.IntToPtr(20),
})
job := api.NewBatchJob(jobID, jobID, "region1", 1).
job := api.NewBatchJob(jobID, jobID, "global", 1).
AddDatacenter("dc1").
AddTaskGroup(group)

2
internal/testing/apitests/util_test.go
View File

@ -62,7 +62,7 @@ func testJob() *api.Job {
SizeMB: intToPtr(25),
})
job := api.NewBatchJob("job1", "redis", "region1", 1).
job := api.NewBatchJob("job1", "redis", "global", 1).
AddDatacenter("dc1").
AddTaskGroup(group)

4
nomad/rpc_test.go
View File

@ -79,7 +79,7 @@ func TestRPC_forwardRegion(t *testing.T) {
s1 := TestServer(t, nil)
defer s1.Shutdown()
s2 := TestServer(t, func(c *Config) {
c.Region = "region2"
c.Region = "global"
})
defer s2.Shutdown()
TestJoin(t, s1, s2)
@ -87,7 +87,7 @@ func TestRPC_forwardRegion(t *testing.T) {
testutil.WaitForLeader(t, s2.RPC)
var out struct{}
err := s1.forwardRegion("region2", "Status.Ping", struct{}{}, &out)
err := s1.forwardRegion("global", "Status.Ping", struct{}{}, &out)
if err != nil {
t.Fatalf("err: %v", err)
}

2
nomad/serf_test.go
View File

@ -59,7 +59,7 @@ func TestNomad_RemovePeer(t *testing.T) {
s1 := TestServer(t, nil)
defer s1.Shutdown()
s2 := TestServer(t, func(c *Config) {
c.Region = "region2"
c.Region = "global"
})
defer s2.Shutdown()
TestJoin(t, s1, s2)

2
nomad/server_test.go
View File

@ -210,7 +210,7 @@ func TestServer_Regions(t *testing.T) {
func TestServer_Reload_Vault(t *testing.T) {
t.Parallel()
s1 := TestServer(t, func(c *Config) {
c.Region = "region1"
c.Region = "global"
})
defer s1.Shutdown()