From 59fac48d9249e9fb45866ace30c312dbcb8c3e08 Mon Sep 17 00:00:00 2001
From: Chris Baker <cgbaker@hashicorp.com>
Date: Thu, 20 Jun 2019 20:52:40 +0000
Subject: [PATCH] alloc lifecycle: 404 when attempting to stop non-existent
 allocation

---
 CHANGELOG.md                         |  1 +
 command/agent/alloc_endpoint.go      | 11 +++++++++--
 command/agent/alloc_endpoint_test.go | 13 ++++++-------
 nomad/alloc_endpoint.go              |  4 ++++
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2214b1488..338643196 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ IMPROVEMENTS:
 BUG FIXES:
 
  * core: Improved job spec parsing error messages for variable interpolation failures [[GH-5844](https://github.com/hashicorp/nomad/issues/5844)]
+ * core: Handle error case when attempting to stop a non-existent allocation [[GH-5865](https://github.com/hashicorp/nomad/issues/5865)]
  * client: Fixed regression that prevented registering multiple services with the same name but different ports in Consul correctly [[GH-5829](https://github.com/hashicorp/nomad/issues/5829)]
  * driver: Fixed an issue preventing external driver plugins from launching executor process [[GH-5726](https://github.com/hashicorp/nomad/issues/5726)]
  * driver/docker: Fixed a bug mounting relative paths on Windows [[GH-5811](https://github.com/hashicorp/nomad/issues/5811)]
diff --git a/command/agent/alloc_endpoint.go b/command/agent/alloc_endpoint.go
index a5a7fef24..9c78c8749 100644
--- a/command/agent/alloc_endpoint.go
+++ b/command/agent/alloc_endpoint.go
@@ -119,8 +119,15 @@ func (s *HTTPServer) allocStop(allocID string, resp http.ResponseWriter, req *ht
 	s.parseWriteRequest(req, &sr.WriteRequest)
 
 	var out structs.AllocStopResponse
-	err := s.agent.RPC("Alloc.Stop", &sr, &out)
-	return &out, err
+	rpcErr := s.agent.RPC("Alloc.Stop", &sr, &out)
+
+	if rpcErr != nil {
+		if structs.IsErrUnknownAllocation(rpcErr) {
+			rpcErr = CodedError(404, allocNotFoundErr)
+		}
+	}
+
+	return &out, rpcErr
 }
 
 func (s *HTTPServer) ClientAllocRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
diff --git a/command/agent/alloc_endpoint_test.go b/command/agent/alloc_endpoint_test.go
index ed497a671..85de625f9 100644
--- a/command/agent/alloc_endpoint_test.go
+++ b/command/agent/alloc_endpoint_test.go
@@ -424,17 +424,16 @@ func TestHTTP_AllocStop(t *testing.T) {
 		// Test that we 404 when the allocid is invalid
 		{
 			// Make the HTTP request
-			req, err := http.NewRequest("POST", "/v1/allocation/"+alloc.ID+"/stop", nil)
+			req, err := http.NewRequest("POST", "/v1/allocation/"+uuid.Generate()+"/stop", nil)
 			require.NoError(err)
 			respW := httptest.NewRecorder()
 
 			// Make the request
-			obj, err := s.Server.AllocSpecificRequest(respW, req)
-			require.NoError(err)
-
-			a := obj.(*structs.AllocStopResponse)
-			require.NotEmpty(a.EvalID, "missing eval")
-			require.NotEmpty(a.Index, "missing index")
+			_, err = s.Server.AllocSpecificRequest(respW, req)
+			require.NotNil(err)
+			if !strings.Contains(err.Error(), allocNotFoundErr) {
+				t.Fatalf("err: %v", err)
+			}
 		}
 	})
 }
diff --git a/nomad/alloc_endpoint.go b/nomad/alloc_endpoint.go
index 491541b32..995ce874a 100644
--- a/nomad/alloc_endpoint.go
+++ b/nomad/alloc_endpoint.go
@@ -231,6 +231,10 @@ func (a *Alloc) Stop(args *structs.AllocStopRequest, reply *structs.AllocStopRes
 		return err
 	}
 
+	if alloc == nil {
+		return fmt.Errorf(structs.ErrUnknownAllocationPrefix)
+	}
+
 	eval := &structs.Evaluation{
 		ID:             uuid.Generate(),
 		Namespace:      alloc.Namespace,