From f92e564e7632ed0b53cc216a4be7d8778703e1d0 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 3 Nov 2017 16:07:06 -0500 Subject: [PATCH] Added more unit tests for testing rollback when job has identical spec to AllocHealth and DeploymentStatus endpoints. --- nomad/deployment_endpoint_test.go | 87 +++++++++++++++++++ .../deployments_watcher_test.go | 60 +++++++++++++ nomad/structs/structs.go | 2 +- 3 files changed, 148 insertions(+), 1 deletion(-) diff --git a/nomad/deployment_endpoint_test.go b/nomad/deployment_endpoint_test.go index bef70390c..c03bfb129 100644 --- a/nomad/deployment_endpoint_test.go +++ b/nomad/deployment_endpoint_test.go @@ -860,6 +860,93 @@ func TestDeploymentEndpoint_SetAllocHealth_Rollback(t *testing.T) { assert.EqualValues(2, jout.Version, "reverted job version") } +// tests rollback upon alloc health failure to job with identical spec does not succeed +func TestDeploymentEndpoint_SetAllocHealth_NoRollback(t *testing.T) { + t.Parallel() + s1 := testServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + defer s1.Shutdown() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + assert := assert.New(t) + state := s1.fsm.State() + + // Create the original job + j := mock.Job() + j.Stable = true + j.TaskGroups[0].Update = structs.DefaultUpdateStrategy.Copy() + j.TaskGroups[0].Update.MaxParallel = 2 + j.TaskGroups[0].Update.AutoRevert = true + assert.Nil(state.UpsertJob(998, j), "UpsertJob") + + // Create the second job, deployment and alloc. Job has same spec as original + j2 := j.Copy() + j2.Stable = false + + d := mock.Deployment() + d.TaskGroups["web"].AutoRevert = true + d.JobID = j2.ID + d.JobVersion = j2.Version + + a := mock.Alloc() + a.JobID = j.ID + a.DeploymentID = d.ID + + assert.Nil(state.UpsertJob(999, j2), "UpsertJob") + assert.Nil(state.UpsertDeployment(1000, d), "UpsertDeployment") + assert.Nil(state.UpsertAllocs(1001, []*structs.Allocation{a}), "UpsertAllocs") + + // Set the alloc as unhealthy + req := &structs.DeploymentAllocHealthRequest{ + DeploymentID: d.ID, + UnhealthyAllocationIDs: []string{a.ID}, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + // Fetch the response + var resp structs.DeploymentUpdateResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Deployment.SetAllocHealth", req, &resp), "RPC") + assert.NotZero(resp.Index, "bad response index") + assert.Nil(resp.RevertedJobVersion, "revert version must be nil") + + // Lookup the evaluation + ws := memdb.NewWatchSet() + eval, err := state.EvalByID(ws, resp.EvalID) + assert.Nil(err, "EvalByID failed") + assert.NotNil(eval, "Expect eval") + assert.Equal(eval.CreateIndex, resp.EvalCreateIndex, "eval index mismatch") + assert.Equal(eval.TriggeredBy, structs.EvalTriggerDeploymentWatcher, "eval trigger") + assert.Equal(eval.JobID, d.JobID, "eval job id") + assert.Equal(eval.DeploymentID, d.ID, "eval deployment id") + assert.Equal(eval.Status, structs.EvalStatusPending, "eval status") + + // Lookup the deployment + expectedDesc := structs.DeploymentStatusDescriptionRollbackNoop(structs.DeploymentStatusDescriptionFailedAllocations, 0) + dout, err := state.DeploymentByID(ws, d.ID) + assert.Nil(err, "DeploymentByID failed") + assert.Equal(dout.Status, structs.DeploymentStatusFailed, "wrong status") + assert.Equal(dout.StatusDescription, expectedDesc, "wrong status description") + assert.Equal(resp.DeploymentModifyIndex, dout.ModifyIndex, "wrong modify index") + assert.Len(dout.TaskGroups, 1, "should have one group") + assert.Contains(dout.TaskGroups, "web", "should have web group") + assert.Equal(1, dout.TaskGroups["web"].UnhealthyAllocs, "should have one healthy") + + // Lookup the allocation + aout, err := state.AllocByID(ws, a.ID) + assert.Nil(err, "AllocByID") + assert.NotNil(aout, "alloc") + assert.NotNil(aout.DeploymentStatus, "alloc deployment status") + assert.NotNil(aout.DeploymentStatus.Healthy, "alloc deployment healthy") + assert.False(*aout.DeploymentStatus.Healthy, "alloc deployment healthy") + + // Lookup the job, its version should not have changed + jout, err := state.JobByID(ws, j.Namespace, j.ID) + assert.Nil(err, "JobByID") + assert.NotNil(jout, "job") + assert.EqualValues(1, jout.Version, "original job version") +} + func TestDeploymentEndpoint_List(t *testing.T) { t.Parallel() s1 := testServer(t, nil) diff --git a/nomad/deploymentwatcher/deployments_watcher_test.go b/nomad/deploymentwatcher/deployments_watcher_test.go index c9f9a96ed..fccc14503 100644 --- a/nomad/deploymentwatcher/deployments_watcher_test.go +++ b/nomad/deploymentwatcher/deployments_watcher_test.go @@ -319,6 +319,66 @@ func TestWatcher_SetAllocHealth_Unhealthy_Rollback(t *testing.T) { m.AssertNumberOfCalls(t, "UpdateDeploymentAllocHealth", 1) } +// Test setting allocation unhealthy on job with identical spec and there should be no rollback +func TestWatcher_SetAllocHealth_Unhealthy_NoRollback(t *testing.T) { + t.Parallel() + assert := assert.New(t) + w, m := defaultTestDeploymentWatcher(t) + + // Create a job, alloc, and a deployment + j := mock.Job() + j.TaskGroups[0].Update = structs.DefaultUpdateStrategy.Copy() + j.TaskGroups[0].Update.MaxParallel = 2 + j.TaskGroups[0].Update.AutoRevert = true + j.Stable = true + d := mock.Deployment() + d.JobID = j.ID + d.TaskGroups["web"].AutoRevert = true + a := mock.Alloc() + a.DeploymentID = d.ID + assert.Nil(m.state.UpsertJob(m.nextIndex(), j), "UpsertJob") + assert.Nil(m.state.UpsertDeployment(m.nextIndex(), d), "UpsertDeployment") + assert.Nil(m.state.UpsertAllocs(m.nextIndex(), []*structs.Allocation{a}), "UpsertAllocs") + + // Upsert the job again to get a new version + j2 := j.Copy() + j2.Stable = false + + assert.Nil(m.state.UpsertJob(m.nextIndex(), j2), "UpsertJob2") + + w.SetEnabled(true, m.state) + testutil.WaitForResult(func() (bool, error) { return 1 == len(w.watchers), nil }, + func(err error) { assert.Equal(1, len(w.watchers), "Should have 1 deployment") }) + + // Assert that we get a call to UpsertDeploymentAllocHealth + matchConfig := &matchDeploymentAllocHealthRequestConfig{ + DeploymentID: d.ID, + Unhealthy: []string{a.ID}, + Eval: true, + DeploymentUpdate: &structs.DeploymentStatusUpdate{ + DeploymentID: d.ID, + Status: structs.DeploymentStatusFailed, + StatusDescription: structs.DeploymentStatusDescriptionFailedAllocations, + }, + JobVersion: nil, + } + matcher := matchDeploymentAllocHealthRequest(matchConfig) + m.On("UpdateDeploymentAllocHealth", mocker.MatchedBy(matcher)).Return(nil) + + // Call SetAllocHealth + req := &structs.DeploymentAllocHealthRequest{ + DeploymentID: d.ID, + UnhealthyAllocationIDs: []string{a.ID}, + } + var resp structs.DeploymentUpdateResponse + err := w.SetAllocHealth(req, &resp) + assert.Nil(err, "SetAllocHealth") + + testutil.WaitForResult(func() (bool, error) { return 0 == len(w.watchers), nil }, + func(err error) { assert.Equal(0, len(w.watchers), "Should have no deployment") }) + m.AssertNumberOfCalls(t, "UpdateDeploymentAllocHealth", 1) +} + // Test promoting a deployment func TestWatcher_PromoteDeployment_HealthyCanaries(t *testing.T) { t.Parallel() diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index e58c78d8c..c04ea5f26 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4325,7 +4325,7 @@ func DeploymentStatusDescriptionRollback(baseDescription string, jobVersion uint // DeploymentStatusDescriptionRollbackNoop is used to get the status description of // a deployment when rolling back is not possible because it has the same specification func DeploymentStatusDescriptionRollbackNoop(baseDescription string, jobVersion uint64) string { - return fmt.Sprintf("%s - not rolling back to stable job version %d as current job has same specification ", baseDescription, jobVersion) + return fmt.Sprintf("%s - not rolling back to stable job version %d as current job has same specification", baseDescription, jobVersion) } // DeploymentStatusDescriptionNoRollbackTarget is used to get the status description of