Skip to content

Commit

Permalink
multiregion: validation should always return error for OSS (#8687)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tgross
tgross committed Aug 18, 2020
1 parent d8ac3f9 commit e85c318
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 330 deletions.
107 changes: 0 additions & 107 deletions command/deployment_status_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package command

import (
"fmt"
"testing"

"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/command/agent"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/testutil"
"github.com/mitchellh/cli"
"github.com/posener/complete"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -68,106 +64,3 @@ func TestDeploymentStatusCommand_AutocompleteArgs(t *testing.T) {
assert.Equal(1, len(res))
assert.Equal(d.ID, res[0])
}

func TestDeploymentStatusCommand_Multiregion(t *testing.T) {
t.Parallel()

cbe := func(config *agent.Config) {
config.Region = "east"
config.Datacenter = "east-1"
}
cbw := func(config *agent.Config) {
config.Region = "west"
config.Datacenter = "west-1"
}

srv, clientEast, url := testServer(t, true, cbe)
defer srv.Shutdown()

srv2, clientWest, _ := testServer(t, true, cbw)
defer srv2.Shutdown()

// Join with srv1
addr1 := fmt.Sprintf("127.0.0.1:%d",
srv.Agent.Server().GetConfig().SerfConfig.MemberlistConfig.BindPort)

if _, err := srv2.Agent.Server().Join([]string{addr1}); err != nil {
t.Fatalf("Join err: %v", err)
}

// wait for client node
testutil.WaitForResult(func() (bool, error) {
nodes, _, err := clientEast.Nodes().List(nil)
if err != nil {
return false, err
}
if len(nodes) == 0 {
return false, fmt.Errorf("missing node")
}
if _, ok := nodes[0].Drivers["mock_driver"]; !ok {
return false, fmt.Errorf("mock_driver not ready")
}
return true, nil
}, func(err error) {
t.Fatalf("err: %s", err)
})

ui := new(cli.MockUi)
cmd := &DeploymentStatusCommand{Meta: Meta{Ui: ui, flagAddress: url}}

// Register multiregion job in east
jobEast := testMultiRegionJob("job1_sfxx", "east", "east-1")
resp, _, err := clientEast.Jobs().Register(jobEast, nil)
require.NoError(t, err)
if code := waitForSuccess(ui, clientEast, fullId, t, resp.EvalID); code != 0 {
t.Fatalf("status code non zero saw %d", code)
}

// Register multiregion job in west
jobWest := testMultiRegionJob("job1_sfxx", "west", "west-1")
resp2, _, err := clientWest.Jobs().Register(jobWest, &api.WriteOptions{Region: "west"})
require.NoError(t, err)
if code := waitForSuccess(ui, clientWest, fullId, t, resp2.EvalID); code != 0 {
t.Fatalf("status code non zero saw %d", code)
}

jobs, _, err := clientEast.Jobs().List(&api.QueryOptions{})
require.NoError(t, err)
require.Len(t, jobs, 1)

deploys, _, err := clientEast.Jobs().Deployments(jobs[0].ID, true, &api.QueryOptions{})
require.NoError(t, err)
require.Len(t, deploys, 1)

// Grab both deployments to verify output
eastDeploys, _, err := clientEast.Jobs().Deployments(jobs[0].ID, true, &api.QueryOptions{Region: "east"})
require.NoError(t, err)
require.Len(t, eastDeploys, 1)

westDeploys, _, err := clientWest.Jobs().Deployments(jobs[0].ID, true, &api.QueryOptions{Region: "west"})
require.NoError(t, err)
require.Len(t, westDeploys, 1)

// Run command for specific deploy
if code := cmd.Run([]string{"-region=east", "-address=" + url, deploys[0].ID}); code != 0 {
t.Fatalf("expected exit 0, got: %d", code)
}

// Verify Multi-region Deployment info populated
out := ui.OutputWriter.String()
require.Contains(t, out, "Multiregion Deployment")
require.Contains(t, out, "Region")
require.Contains(t, out, "ID")
require.Contains(t, out, "Status")
require.Contains(t, out, "east")
require.Contains(t, out, eastDeploys[0].ID[0:7])
require.Contains(t, out, "west")
require.Contains(t, out, westDeploys[0].ID[0:7])

// this will always be pending because we're not really doing a multiregion
// register here in OSS
require.Contains(t, out, "pending")

require.NotContains(t, out, "<none>")

}
104 changes: 0 additions & 104 deletions command/job_status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,110 +385,6 @@ func TestJobStatusCommand_RescheduleEvals(t *testing.T) {
require.Contains(out, e.ID[:8])
}

// TestJobStatusCommand_Multiregion tests multiregion deployment output
func TestJobStatusCommand_Multiregion(t *testing.T) {
t.Parallel()

cbe := func(config *agent.Config) {
config.Region = "east"
config.Datacenter = "east-1"
}
cbw := func(config *agent.Config) {
config.Region = "west"
config.Datacenter = "west-1"
}

srv, clientEast, url := testServer(t, true, cbe)
defer srv.Shutdown()

srv2, clientWest, _ := testServer(t, true, cbw)
defer srv2.Shutdown()

// Join with srv1
addr := fmt.Sprintf("127.0.0.1:%d",
srv.Agent.Server().GetConfig().SerfConfig.MemberlistConfig.BindPort)

if _, err := srv2.Agent.Server().Join([]string{addr}); err != nil {
t.Fatalf("Join err: %v", err)
}

// wait for client node
testutil.WaitForResult(func() (bool, error) {
nodes, _, err := clientEast.Nodes().List(nil)
if err != nil {
return false, err
}
if len(nodes) == 0 {
return false, fmt.Errorf("missing node")
}
if _, ok := nodes[0].Drivers["mock_driver"]; !ok {
return false, fmt.Errorf("mock_driver not ready")
}
return true, nil
}, func(err error) {
t.Fatalf("err: %s", err)
})

ui := new(cli.MockUi)
cmd := &JobStatusCommand{Meta: Meta{Ui: ui, flagAddress: url}}

// Register multiregion job
// Register multiregion job in east
jobEast := testMultiRegionJob("job1_sfxx", "east", "east-1")
resp, _, err := clientEast.Jobs().Register(jobEast, nil)
require.NoError(t, err)
if code := waitForSuccess(ui, clientEast, fullId, t, resp.EvalID); code != 0 {
t.Fatalf("status code non zero saw %d", code)
}

// Register multiregion job in west
jobWest := testMultiRegionJob("job1_sfxx", "west", "west-1")
resp2, _, err := clientWest.Jobs().Register(jobWest, &api.WriteOptions{Region: "west"})
require.NoError(t, err)
if code := waitForSuccess(ui, clientWest, fullId, t, resp2.EvalID); code != 0 {
t.Fatalf("status code non zero saw %d", code)
}

jobs, _, err := clientEast.Jobs().List(&api.QueryOptions{})
require.NoError(t, err)
require.Len(t, jobs, 1)

deploys, _, err := clientEast.Jobs().Deployments(jobs[0].ID, true, &api.QueryOptions{})
require.NoError(t, err)
require.Len(t, deploys, 1)

// Grab both deployments to verify output
eastDeploys, _, err := clientEast.Jobs().Deployments(jobs[0].ID, true, &api.QueryOptions{Region: "east"})
require.NoError(t, err)
require.Len(t, eastDeploys, 1)

westDeploys, _, err := clientWest.Jobs().Deployments(jobs[0].ID, true, &api.QueryOptions{Region: "west"})
require.NoError(t, err)
// require.Len(t, westDeploys, 1)

// Run command for specific deploy
if code := cmd.Run([]string{"-address=" + url, jobs[0].ID}); code != 0 {
t.Fatalf("expected exit 0, got: %d", code)
}

// Verify Multi-region Deployment info populated
out := ui.OutputWriter.String()
require.Contains(t, out, "Multiregion Deployment")
require.Contains(t, out, "Region")
require.Contains(t, out, "ID")
require.Contains(t, out, "Status")
require.Contains(t, out, "east")
require.Contains(t, out, eastDeploys[0].ID[0:7])
require.Contains(t, out, "west")
require.Contains(t, out, westDeploys[0].ID[0:7])

// this will always be pending because we're not really doing a multiregion
// register here in OSS
require.Contains(t, out, "pending")

require.NotContains(t, out, "<none>")

}
func waitForSuccess(ui cli.Ui, client *api.Client, length int, t *testing.T, evalId string) int {
mon := newMonitor(ui, client, length)
monErr := mon.monitor(evalId, false)
Expand Down
35 changes: 0 additions & 35 deletions nomad/structs/structs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4591,41 +4591,6 @@ func (m *Multiregion) Copy() *Multiregion {
return copy
}

func (m *Multiregion) Validate(jobType string, jobDatacenters []string) error {
var mErr multierror.Error
seen := map[string]struct{}{}
for _, region := range m.Regions {
if _, ok := seen[region.Name]; ok {
mErr.Errors = append(mErr.Errors,
fmt.Errorf("Multiregion region %q can't be listed twice",
region.Name))
}
seen[region.Name] = struct{}{}
if len(region.Datacenters) == 0 && len(jobDatacenters) == 0 {
mErr.Errors = append(mErr.Errors,
fmt.Errorf("Multiregion region %q must have at least 1 datacenter",
region.Name),
)
}
}
if m.Strategy != nil {
switch jobType {
case JobTypeBatch:
if m.Strategy.OnFailure != "" || m.Strategy.MaxParallel != 0 {
mErr.Errors = append(mErr.Errors,
errors.New("Multiregion batch jobs can't have an update strategy"))
}
case JobTypeSystem:
if m.Strategy.OnFailure != "" {
mErr.Errors = append(mErr.Errors,
errors.New("Multiregion system jobs can't have an on_failure setting"))
}
default: // service
}
}
return mErr.ErrorOrNil()
}

type MultiregionStrategy struct {
MaxParallel int
OnFailure string
Expand Down
13 changes: 13 additions & 0 deletions nomad/structs/structs_oss.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// +build !ent

package structs

import "errors"

func (m *Multiregion) Validate(jobType string, jobDatacenters []string) error {
if m != nil {
return errors.New("Multiregion jobs are unlicensed.")
}

return nil
}
84 changes: 0 additions & 84 deletions nomad/structs/structs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5508,87 +5508,3 @@ func TestNodeResources_Merge(t *testing.T) {
},
}, res)
}

func TestMultiregion_Validate(t *testing.T) {
require := require.New(t)
cases := []struct {
Name string
JobType string
Case *Multiregion
Errors []string
}{
{
Name: "empty valid multiregion spec",
JobType: JobTypeService,
Case: &Multiregion{},
Errors: []string{},
},

{
Name: "non-empty valid multiregion spec",
JobType: JobTypeService,
Case: &Multiregion{
Strategy: &MultiregionStrategy{
MaxParallel: 2,
OnFailure: "fail_all",
},
Regions: []*MultiregionRegion{
{

Count: 2,
Datacenters: []string{"west-1", "west-2"},
Meta: map[string]string{},
},
{
Name: "east",
Count: 1,
Datacenters: []string{"east-1"},
Meta: map[string]string{},
},
},
},
Errors: []string{},
},

{
Name: "repeated region, wrong strategy, missing DCs",
JobType: JobTypeBatch,
Case: &Multiregion{
Strategy: &MultiregionStrategy{
MaxParallel: 2,
},
Regions: []*MultiregionRegion{
{
Name: "west",
Datacenters: []string{"west-1", "west-2"},
},

{
Name: "west",
},
},
},
Errors: []string{
"Multiregion region \"west\" can't be listed twice",
"Multiregion region \"west\" must have at least 1 datacenter",
"Multiregion batch jobs can't have an update strategy",
},
},
}

for _, tc := range cases {
t.Run(tc.Name, func(t *testing.T) {
err := tc.Case.Validate(tc.JobType, []string{})
if len(tc.Errors) == 0 {
require.NoError(err)
} else {
mErr := err.(*multierror.Error)
for i, expectedErr := range tc.Errors {
if !strings.Contains(mErr.Errors[i].Error(), expectedErr) {
t.Fatalf("err: %s, expected: %s", err, expectedErr)
}
}
}
})
}
}

0 comments on commit e85c318

Please sign in to comment.