From 54bde4db1094946196aa04e46bc6337881d0c4ca Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Wed, 20 May 2020 14:25:04 -0400
Subject: [PATCH] resolve comments from code review

---
 nomad/csi_batch.go     |  8 +++++---
 nomad/job_endpoint.go  | 11 ++++++++---
 nomad/node_endpoint.go | 11 ++++++++---
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/nomad/csi_batch.go b/nomad/csi_batch.go
index 089b5aff4fe..99419bac2c8 100644
--- a/nomad/csi_batch.go
+++ b/nomad/csi_batch.go
@@ -2,6 +2,7 @@ package nomad
 
 import (
 	log "github.com/hashicorp/go-hclog"
+	multierror "github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
@@ -30,7 +31,7 @@ func newCSIBatchRelease(srv *Server, logger log.Logger, max int) *csiBatchReleas
 
 // add the volume ID + namespace to the deduplicated batches
 func (c *csiBatchRelease) add(vol, namespace string) {
-	id := vol + namespace
+	id := vol + "\x00" + namespace
 
 	// ignore duplicates
 	_, seen := c.seen[id]
@@ -60,14 +61,15 @@ func (c *csiBatchRelease) add(vol, namespace string) {
 
 // apply flushes the batches to raft
 func (c *csiBatchRelease) apply() error {
+	var result *multierror.Error
 	for _, batch := range c.batches {
 		if len(batch.Claims) > 0 {
 			_, _, err := c.srv.raftApply(structs.CSIVolumeClaimBatchRequestType, batch)
 			if err != nil {
 				c.logger.Error("csi raft apply failed", "error", err, "method", "claim")
-				return err
+				result = multierror.Append(result, err)
 			}
 		}
 	}
-	return nil
+	return result.ErrorOrNil()
 }
diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go
index cbc3d2016fc..ce18222b69c 100644
--- a/nomad/job_endpoint.go
+++ b/nomad/job_endpoint.go
@@ -731,7 +731,11 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
 	reply.JobModifyIndex = index
 
 	// Make a raft apply to release the CSI volume claims of terminal allocs.
-	volumesToGC.apply()
+	var result *multierror.Error
+	err = volumesToGC.apply()
+	if err != nil {
+		result = multierror.Append(result, err)
+	}
 
 	// If the job is periodic or parameterized, we don't create an eval.
 	if job != nil && (job.IsPeriodic() || job.IsParameterized()) {
@@ -762,15 +766,16 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
 	// Commit this evaluation via Raft
 	_, evalIndex, err := j.srv.raftApply(structs.EvalUpdateRequestType, update)
 	if err != nil {
+		result = multierror.Append(result, err)
 		j.logger.Error("eval create failed", "error", err, "method", "deregister")
-		return err
+		return result.ErrorOrNil()
 	}
 
 	// Populate the reply with eval information
 	reply.EvalID = eval.ID
 	reply.EvalCreateIndex = evalIndex
 	reply.Index = evalIndex
-	return nil
+	return result.ErrorOrNil()
 }
 
 // BatchDeregister is used to remove a set of jobs from the cluster.
diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go
index 26fd0633c18..d613f62b002 100644
--- a/nomad/node_endpoint.go
+++ b/nomad/node_endpoint.go
@@ -1139,7 +1139,11 @@ func (n *Node) UpdateAlloc(args *structs.AllocUpdateRequest, reply *structs.Gene
 	}
 
 	// Make a raft apply to release the CSI volume claims of terminal allocs.
-	volumesToGC.apply()
+	var result *multierror.Error
+	err := volumesToGC.apply()
+	if err != nil {
+		result = multierror.Append(result, err)
+	}
 
 	// Add this to the batch
 	n.updatesLock.Lock()
@@ -1171,12 +1175,13 @@ func (n *Node) UpdateAlloc(args *structs.AllocUpdateRequest, reply *structs.Gene
 
 	// Wait for the future
 	if err := future.Wait(); err != nil {
-		return err
+		result = multierror.Append(result, err)
+		return result.ErrorOrNil()
 	}
 
 	// Setup the response
 	reply.Index = future.Index()
-	return nil
+	return result.ErrorOrNil()
 }
 
 // batchUpdate is used to update all the allocations