Merge pull request #5664 from hashicorp/f-http-hcl-region

backfill region from hcl for jobUpdate and jobPlan

api/compose_test.go

@@ -37,15 +37,15 @@ 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")).
 		AddTaskGroup(grp)
 
 	// Check that the composed result looks correct
 	expect := &Job{
-		Region:   stringToPtr("region1"),
+		Region:   stringToPtr("global"),
 		ID:       stringToPtr("job1"),
 		Name:     stringToPtr("myjob"),
 		Type:     stringToPtr(JobTypeService),

api/jobs_test.go

@@ -1213,9 +1213,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),
@@ -1228,9 +1228,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),

api/util_test.go

@@ -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)
 

client/client_test.go

@@ -987,7 +987,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)
@@ -1007,7 +1007,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) {
@@ -1041,7 +1041,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
@@ -1063,7 +1063,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)
@@ -1092,7 +1092,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
@@ -1117,7 +1117,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

command/agent/config_test.go

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

command/agent/job_endpoint.go

@@ -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)
@@ -376,6 +384,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{
@@ -384,7 +398,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,
 		},
 	}

command/agent/job_endpoint_test.go

@@ -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) {
@@ -1025,6 +1118,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) {

command/agent/testingutils_test.go

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

command/util_test.go

@@ -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)
 

internal/testing/apitests/util_test.go

@@ -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)
 

nomad/rpc_test.go

@@ -79,15 +79,15 @@ 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)
 	testutil.WaitForLeader(t, s1.RPC)
 	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)
 	}