From bde6fa01a79943de859d51505dd26f5ff76ecb70 Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Thu, 1 Aug 2019 18:14:05 -0700 Subject: [PATCH 1/5] job region defaults to node region --- api/jobs.go | 2 +- command/agent/job_endpoint.go | 8 ++++++-- command/agent/job_endpoint_test.go | 8 +++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/api/jobs.go b/api/jobs.go index 901b41e7f17..6054b56820b 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -704,7 +704,7 @@ func (j *Job) Canonicalize() { j.Stop = boolToPtr(false) } if j.Region == nil { - j.Region = stringToPtr("global") + j.Region = stringToPtr("") } if j.Namespace == nil { j.Namespace = stringToPtr("default") diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index a42ce63de9f..4d64c52cdec 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -384,14 +384,18 @@ 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 + // Region in api request query param takes precedence over region in job config 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) + // If no region given, defaults to region of the node + if sJob.Region == "" { + sJob.Region = s.agent.config.Region + } + regReq := structs.JobRegisterRequest{ Job: sJob, EnforceIndex: args.EnforceIndex, diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index ba2912bbcb9..fdc2f4a1282 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -493,11 +493,17 @@ func TestHTTP_JobUpdateRegion(t *testing.T) { ExpectedRegion: "north-america", }, { - Name: "falls back to default if no region is provided", + Name: "defaults to node region global if no region is provided", ConfigRegion: "", APIRegion: "", ExpectedRegion: "global", }, + { + Name: "defaults to node region not-global if no region is provided", + ConfigRegion: "", + APIRegion: "", + ExpectedRegion: "not-global", + }, } for _, tc := range cases { From 274d938812248da84366dfe778bae4ceab19f86f Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Fri, 2 Aug 2019 12:04:42 -0700 Subject: [PATCH 2/5] set region defaults before job canonicalization --- api/jobs.go | 2 +- command/agent/job_endpoint.go | 26 ++++++++++++++++++-------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/api/jobs.go b/api/jobs.go index 6054b56820b..901b41e7f17 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -704,7 +704,7 @@ func (j *Job) Canonicalize() { j.Stop = boolToPtr(false) } if j.Region == nil { - j.Region = stringToPtr("") + j.Region = stringToPtr("global") } if j.Namespace == nil { j.Namespace = stringToPtr("default") diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 4d64c52cdec..97bf9b28892 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -141,12 +141,17 @@ 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 + // Region in http request query param takes precedence over region in job hcl config if args.WriteRequest.Region != "" { args.Job.Region = helper.StringToPtr(args.WriteRequest.Region) } + // If 'global' region is specified or if no region is given, + // default to region of the node you're submitting to + if args.Job.Region == helper.StringToPtr("global") || + args.Job.Region == helper.StringToPtr("") || args.Job.Region == nil { + args.Job.Region = &s.agent.config.Region + } - // If no region given, region is canonicalized to 'global' sJob := ApiJobToStructJob(args.Job) planReq := structs.JobPlanRequest{ @@ -157,6 +162,8 @@ func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request, Region: sJob.Region, }, } + // parseWriteRequest overrides Namespace, Region and AuthToken + // based on values from the original http request s.parseWriteRequest(req, &planReq.WriteRequest) planReq.Namespace = sJob.Namespace @@ -384,18 +391,19 @@ func (s *HTTPServer) jobUpdate(resp http.ResponseWriter, req *http.Request, return nil, CodedError(400, "Job ID does not match name") } - // Region in api request query param takes precedence over region in job config + // Region in http request query param takes precedence over region in job hcl config if args.WriteRequest.Region != "" { args.Job.Region = helper.StringToPtr(args.WriteRequest.Region) } + // If 'global' region is specified or if no region is given, + // default to region of the node you're submitting to + if args.Job.Region == helper.StringToPtr("global") || + args.Job.Region == helper.StringToPtr("") || args.Job.Region == nil { + args.Job.Region = &s.agent.config.Region + } sJob := ApiJobToStructJob(args.Job) - // If no region given, defaults to region of the node - if sJob.Region == "" { - sJob.Region = s.agent.config.Region - } - regReq := structs.JobRegisterRequest{ Job: sJob, EnforceIndex: args.EnforceIndex, @@ -406,6 +414,8 @@ func (s *HTTPServer) jobUpdate(resp http.ResponseWriter, req *http.Request, AuthToken: args.WriteRequest.SecretID, }, } + // parseWriteRequest overrides Namespace, Region and AuthToken + // based on values from the original http request s.parseWriteRequest(req, ®Req.WriteRequest) regReq.Namespace = sJob.Namespace From 6011d872804e2e39931d9e7d0803dc14c6e2426d Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Fri, 2 Aug 2019 20:00:48 -0700 Subject: [PATCH 3/5] fix pointers --- command/agent/job_endpoint.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 97bf9b28892..9bdcea2728b 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -147,8 +147,7 @@ func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request, } // If 'global' region is specified or if no region is given, // default to region of the node you're submitting to - if args.Job.Region == helper.StringToPtr("global") || - args.Job.Region == helper.StringToPtr("") || args.Job.Region == nil { + if args.Job.Region == nil || *args.Job.Region == "" || *args.Job.Region == "global" { args.Job.Region = &s.agent.config.Region } @@ -397,8 +396,7 @@ func (s *HTTPServer) jobUpdate(resp http.ResponseWriter, req *http.Request, } // If 'global' region is specified or if no region is given, // default to region of the node you're submitting to - if args.Job.Region == helper.StringToPtr("global") || - args.Job.Region == helper.StringToPtr("") || args.Job.Region == nil { + if args.Job.Region == nil || *args.Job.Region == "" || *args.Job.Region == "global" { args.Job.Region = &s.agent.config.Region } From d0ed978f52814d6525269e2a2c5c6309e0243b8c Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Fri, 2 Aug 2019 20:15:22 -0700 Subject: [PATCH 4/5] use constant for magic 'global' region --- api/api.go | 4 ++-- api/jobs.go | 7 ++++++- command/agent/job_endpoint.go | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/api/api.go b/api/api.go index 87aa45ec751..bacb7c3c9ef 100644 --- a/api/api.go +++ b/api/api.go @@ -453,8 +453,8 @@ func (c *Client) getNodeClientImpl(nodeID string, timeout time.Duration, q *Quer // If the client is configured for a particular region use that region = c.config.Region default: - // No region information is given so use the default. - region = "global" + // No region information is given so use GlobalRegion as the default. + region = GlobalRegion } // Get an API client for the node diff --git a/api/jobs.go b/api/jobs.go index 901b41e7f17..e0521006709 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -25,6 +25,11 @@ const ( // DefaultNamespace is the default namespace. DefaultNamespace = "default" + + // GlobalRegion is a sentinel region value that users may specify + // to indicate the job should be run on the region of the node + // that the job was submitted to + GlobalRegion = "global" ) const ( @@ -704,7 +709,7 @@ func (j *Job) Canonicalize() { j.Stop = boolToPtr(false) } if j.Region == nil { - j.Region = stringToPtr("global") + j.Region = stringToPtr(GlobalRegion) } if j.Namespace == nil { j.Namespace = stringToPtr("default") diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 9bdcea2728b..6353c51e2c2 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -147,7 +147,7 @@ func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request, } // If 'global' region is specified or if no region is given, // default to region of the node you're submitting to - if args.Job.Region == nil || *args.Job.Region == "" || *args.Job.Region == "global" { + if args.Job.Region == nil || *args.Job.Region == "" || *args.Job.Region == api.GlobalRegion { args.Job.Region = &s.agent.config.Region } @@ -396,7 +396,7 @@ func (s *HTTPServer) jobUpdate(resp http.ResponseWriter, req *http.Request, } // If 'global' region is specified or if no region is given, // default to region of the node you're submitting to - if args.Job.Region == nil || *args.Job.Region == "" || *args.Job.Region == "global" { + if args.Job.Region == nil || *args.Job.Region == "" || *args.Job.Region == api.GlobalRegion { args.Job.Region = &s.agent.config.Region } From fcf7c0dce91d76d7bb4810a621b0c88a21c67777 Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Mon, 5 Aug 2019 11:41:36 -0700 Subject: [PATCH 5/5] add better comments for GlobalRegion --- api/jobs.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/jobs.go b/api/jobs.go index e0521006709..9964990235c 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -26,9 +26,11 @@ const ( // DefaultNamespace is the default namespace. DefaultNamespace = "default" - // GlobalRegion is a sentinel region value that users may specify - // to indicate the job should be run on the region of the node - // that the job was submitted to + // For Job configuration, GlobalRegion is a sentinel region value + // that users may specify to indicate the job should be run on + // the region of the node that the job was submitted to. + // For Client configuration, if no region information is given, + // the client node will default to be part of the GlobalRegion. GlobalRegion = "global" )