From 018c86679275cd1f0e0446e820424ccf5167d38f Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Wed, 25 Oct 2017 15:55:18 +0100
Subject: [PATCH 1/7] datastore: errcheck: Ensure all errors are correctly
 handled

Make ciao-controller/internal/datastore pass errcheck. In most cases the
error can be safely swallowed as they are on an error path.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 .../internal/datastore/datastore_test.go      |  5 +-
 .../internal/datastore/sqlite3db.go           | 92 +++++++++----------
 .../internal/datastore/sqlite3db_test.go      |  2 +-
 3 files changed, 51 insertions(+), 48 deletions(-)

diff --git a/ciao-controller/internal/datastore/datastore_test.go b/ciao-controller/internal/datastore/datastore_test.go
index d0d7631a1..41685db2b 100644
--- a/ciao-controller/internal/datastore/datastore_test.go
+++ b/ciao-controller/internal/datastore/datastore_test.go
@@ -1246,7 +1246,10 @@ func TestAttachVolumeFailure(t *testing.T) {
 	}
 
 	// pretend we got a failure to attach.
-	ds.AttachVolumeFailure(instance.ID, data.ID, payloads.AttachVolumeAlreadyAttached)
+	err = ds.AttachVolumeFailure(instance.ID, data.ID, payloads.AttachVolumeAlreadyAttached)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	// make sure state has been switched to Available again.
 	bd, err := ds.GetBlockDevice(data.ID)
diff --git a/ciao-controller/internal/datastore/sqlite3db.go b/ciao-controller/internal/datastore/sqlite3db.go
index e9fb06bb3..0a2b9da02 100644
--- a/ciao-controller/internal/datastore/sqlite3db.go
+++ b/ciao-controller/internal/datastore/sqlite3db.go
@@ -593,7 +593,7 @@ func (ds *sqliteDB) Connect(persistentURI string) error {
 
 // Disconnect is used to close the connection to the sql database
 func (ds *sqliteDB) disconnect() {
-	ds.db.Close()
+	_ = ds.db.Close()
 }
 
 func (ds *sqliteDB) logEvent(event types.LogEntry) error {
@@ -651,7 +651,7 @@ func (ds *sqliteDB) getWorkloadDefaults(ID string) ([]payloads.RequestedResource
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	var defaults []payloads.RequestedResource
 
@@ -713,7 +713,7 @@ func (ds *sqliteDB) getWorkloadStorage(ID string) ([]types.StorageResource, erro
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	res := []types.StorageResource{}
 	var sourceType string
@@ -807,7 +807,7 @@ func (ds *sqliteDB) getTenantWorkloads(tenantID string) ([]types.Workload, error
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var wl types.Workload
@@ -872,7 +872,7 @@ func (ds *sqliteDB) updateWorkload(w types.Workload) error {
 		for _, d := range w.Defaults {
 			err := ds.createWorkloadDefault(tx, w.ID, d)
 			if err != nil {
-				tx.Rollback()
+				_ = tx.Rollback()
 				return err
 			}
 		}
@@ -882,7 +882,7 @@ func (ds *sqliteDB) updateWorkload(w types.Workload) error {
 			for i := range w.Storage {
 				err := ds.createWorkloadStorage(tx, w.ID, &w.Storage[i])
 				if err != nil {
-					tx.Rollback()
+					_ = tx.Rollback()
 					return err
 				}
 			}
@@ -893,18 +893,18 @@ func (ds *sqliteDB) updateWorkload(w types.Workload) error {
 		path := fmt.Sprintf("%s/%s", ds.workloadsPath, filename)
 		err := ioutil.WriteFile(path, []byte(w.Config), 0644)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 
 		_, err = tx.Exec("INSERT INTO workload_template (id, tenant_id, description, filename, fw_type, vm_type, image_name, internal) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", w.ID, w.TenantID, w.Description, filename, w.FWType, string(w.VMType), w.ImageName, false)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 	} else {
 		// update not supported yet.
-		tx.Rollback()
+		_ = tx.Rollback()
 		return errors.New("Workload Update not supported yet")
 	}
 
@@ -925,19 +925,19 @@ func (ds *sqliteDB) deleteWorkload(ID string) error {
 
 	err = ds.deleteWorkloadDefault(tx, ID)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
 	err = ds.deleteWorkloadStorage(tx, ID)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
 	_, err = tx.Exec("DELETE FROM workload_template WHERE id = ?", ID)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
@@ -945,7 +945,7 @@ func (ds *sqliteDB) deleteWorkload(ID string) error {
 	path := filepath.Join(ds.workloadsPath, filename)
 	err = os.Remove(path)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
@@ -967,7 +967,7 @@ func (ds *sqliteDB) getTenants() ([]*tenant, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var id sql.NullString
@@ -1055,7 +1055,7 @@ func (ds *sqliteDB) getTenantNetwork(tenant *tenant) error {
 	if err != nil {
 		return err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var subnetInt uint16
@@ -1108,13 +1108,13 @@ func (ds *sqliteDB) deleteTenant(tenantID string) error {
 	// first delete any quotas associated with this tenant
 	_, err = tx.Exec("DELETE FROM quotas WHERE tenant_id = ?", tenantID)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
 	_, err = tx.Exec("DELETE FROM tenants WHERE id = ?", tenantID)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
@@ -1165,7 +1165,7 @@ func (ds *sqliteDB) getInstances() ([]*types.Instance, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var i types.Instance
@@ -1232,7 +1232,7 @@ func (ds *sqliteDB) getTenantInstances(tenantID string) (map[string]*types.Insta
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	instances := make(map[string]*types.Instance)
 	for rows.Next() {
@@ -1329,11 +1329,11 @@ func (ds *sqliteDB) addInstanceStats(stats []payloads.InstanceStat, nodeID strin
 
 	stmt, err := tx.Prepare(cmd)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
-	defer stmt.Close()
+	defer func() { _ = stmt.Close() }()
 
 	for index := range stats {
 		stat := stats[index]
@@ -1366,7 +1366,7 @@ func (ds *sqliteDB) addFrameStat(stat payloads.FrameTrace) error {
 
 	_, err = tx.Exec(query, stat.Label, stat.Type, stat.Operand, stat.StartTimestamp, stat.EndTimestamp)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
@@ -1374,7 +1374,7 @@ func (ds *sqliteDB) addFrameStat(stat payloads.FrameTrace) error {
 
 	err = tx.QueryRow("SELECT last_insert_rowid();").Scan(&id)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
@@ -1386,7 +1386,7 @@ func (ds *sqliteDB) addFrameStat(stat payloads.FrameTrace) error {
 
 		_, err = tx.Exec(cmd, id, t.SSNTPUUID, t.TxTimestamp, t.RxTimestamp)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 	}
@@ -1409,7 +1409,7 @@ func (ds *sqliteDB) getEventLog() ([]*types.LogEntry, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	logEntries = make([]*types.LogEntry, 0)
 	for rows.Next() {
@@ -1441,7 +1441,7 @@ func (ds *sqliteDB) getBatchFrameSummary() ([]types.BatchFrameSummary, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	stats = make([]types.BatchFrameSummary, 0)
 
@@ -1556,7 +1556,7 @@ func (ds *sqliteDB) getBatchFrameStatistics(label string) ([]types.BatchFrameSta
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	stats = make([]types.BatchFrameStat, 0)
 
@@ -1642,7 +1642,7 @@ func (ds *sqliteDB) getTenantDevices(tenantID string) (map[string]types.Volume,
 	if err != nil {
 		return devices, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var state string
@@ -1683,7 +1683,7 @@ func (ds *sqliteDB) getAllBlockData() (map[string]types.Volume, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var data types.Volume
@@ -1763,7 +1763,7 @@ func (ds *sqliteDB) getAllStorageAttachments() (map[string]types.StorageAttachme
 	if err != nil {
 		return attachments, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var a types.StorageAttachment
@@ -1859,7 +1859,7 @@ func (ds *sqliteDB) updateAddresses(tx *sql.Tx, pool types.Pool) error {
 		if !ok {
 			_, err = tx.Exec("DELETE FROM address_pool WHERE id = ?", addr.ID)
 			if err != nil {
-				tx.Rollback()
+				_ = tx.Rollback()
 				return err
 			}
 		}
@@ -1870,7 +1870,7 @@ func (ds *sqliteDB) updateAddresses(tx *sql.Tx, pool types.Pool) error {
 	for _, IP := range pool.IPs {
 		_, err = tx.Exec("INSERT OR IGNORE INTO address_pool (id, pool_id, address) VALUES (?, ?, ?)", IP.ID, pool.ID, IP.Address)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 	}
@@ -1896,13 +1896,13 @@ func (ds *sqliteDB) updatePool(pool types.Pool) error {
 
 	err = ds.updateSubnets(tx, pool)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
 	err = ds.updateAddresses(tx, pool)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
@@ -1911,14 +1911,14 @@ func (ds *sqliteDB) updatePool(pool types.Pool) error {
 	if !ok {
 		_, err = tx.Exec("INSERT INTO pools (id, name, free, total) VALUES (?, ?, ?, ?)", pool.ID, pool.Name, pool.Free, pool.TotalIPs)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 	} else {
 		// update free and total counts.
 		_, err = tx.Exec("UPDATE pools SET free = ?, total = ? WHERE id = ?", pool.Free, pool.TotalIPs, pool.ID)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 	}
@@ -1943,7 +1943,7 @@ func (ds *sqliteDB) getAllPools() map[string]types.Pool {
 	if err != nil {
 		return nil
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var pool types.Pool
@@ -1999,7 +1999,7 @@ func (ds *sqliteDB) deletePool(ID string) error {
 	for _, subnet := range subnets {
 		_, err = tx.Exec("DELETE FROM subnet_pool WHERE id = ?", subnet.ID)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 	}
@@ -2007,14 +2007,14 @@ func (ds *sqliteDB) deletePool(ID string) error {
 	for _, addr := range IPs {
 		_, err = tx.Exec("DELETE FROM address_pool WHERE id = ?", addr.ID)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return err
 		}
 	}
 
 	_, err = tx.Exec("DELETE FROM pools WHERE id = ?", ID)
 	if err != nil {
-		tx.Rollback()
+		_ = tx.Rollback()
 		return err
 	}
 
@@ -2037,7 +2037,7 @@ func (ds *sqliteDB) getPoolSubnets(poolID string) ([]types.ExternalSubnet, error
 	if err != nil {
 		return subnets, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var subnet types.ExternalSubnet
@@ -2071,7 +2071,7 @@ func (ds *sqliteDB) getPoolAddresses(poolID string) ([]types.ExternalIP, error)
 	if err != nil {
 		return IPs, err
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var IP types.ExternalIP
@@ -2136,7 +2136,7 @@ func (ds *sqliteDB) getMappedIPs() map[string]types.MappedIP {
 		fmt.Println(err)
 		return IPs
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		var IP types.MappedIP
@@ -2170,7 +2170,7 @@ func (ds *sqliteDB) updateQuotas(tenantID string, qds []types.QuotaDetails) erro
 	for i := range qds {
 		_, err = tx.Exec("REPLACE INTO quotas (tenant_id, name, value) VALUES (?, ?, ?)", tenantID, qds[i].Name, qds[i].Value)
 		if err != nil {
-			tx.Rollback()
+			_ = tx.Rollback()
 			return errors.Wrap(err, "error executing query for quota update")
 		}
 	}
@@ -2189,7 +2189,7 @@ func (ds *sqliteDB) getQuotas(tenantID string) ([]types.QuotaDetails, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, "error getting quotas from database")
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	results := []types.QuotaDetails{}
 	for rows.Next() {
@@ -2221,7 +2221,7 @@ func (ds *sqliteDB) getImages() ([]types.Image, error) {
 	if err != nil {
 		return images, errors.Wrap(err, "error getting images from database")
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
 		i := types.Image{}
diff --git a/ciao-controller/internal/datastore/sqlite3db_test.go b/ciao-controller/internal/datastore/sqlite3db_test.go
index 4ea2d1817..5feb08b8d 100644
--- a/ciao-controller/internal/datastore/sqlite3db_test.go
+++ b/ciao-controller/internal/datastore/sqlite3db_test.go
@@ -974,7 +974,7 @@ users:
 
 	// file will be added, so we will want to remove it.
 	filename := fmt.Sprintf("%s/%s_config.yaml", *workloadsPath, wl.ID)
-	defer os.Remove(filename)
+	defer func() { _ = os.Remove(filename) }()
 
 	err = db.updateWorkload(wl)
 	if err != nil {

From 77e8c8e24b630307aec95bbf83e941a4506ac47c Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Wed, 25 Oct 2017 16:24:30 +0100
Subject: [PATCH 2/7] ciao-controller: Ensure that instance errors are logged

Unfortunately instance launch errors were not being logged as the code
could never be executed because if errors occured the function would
have been returned from earlier.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 ciao-controller/compute.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ciao-controller/compute.go b/ciao-controller/compute.go
index 9ad692f8c..933c30441 100644
--- a/ciao-controller/compute.go
+++ b/ciao-controller/compute.go
@@ -384,6 +384,10 @@ func (c *controller) CreateServer(tenant string, server api.CreateServerRequest)
 		servers.Servers = append(servers.Servers, server)
 	}
 
+	if e != nil {
+		_ = c.ds.LogError(tenant, fmt.Sprintf("Error launching instance(s): %v", e))
+	}
+
 	// If no instances launcher or if none converted bail early
 	if e != nil && len(servers.Servers) == 0 {
 		return server, e
@@ -409,9 +413,6 @@ func (c *controller) CreateServer(tenant string, server api.CreateServerRequest)
 		},
 	}
 
-	if e != nil {
-		c.ds.LogError(tenant, fmt.Sprintf("Error launching instance(s): %v", e))
-	}
 	return builtServers, nil
 }
 

From 0493ced17fc5e4102d5049d4e32306f1dfdc3269 Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Wed, 25 Oct 2017 16:39:43 +0100
Subject: [PATCH 3/7] ciao-controller: errcheck: Ensure all errors are handled
 or swallowed

Ensure that all errors generated within ciao-controller are either
propagated up where possible, logged or swallowed if already on an error
path or where logging is not necessary.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 ciao-controller/api.go        | 10 +++++--
 ciao-controller/api/api.go    |  2 +-
 ciao-controller/client.go     | 42 +++++++++++++++++++++------
 ciao-controller/cnci.go       |  9 ++++--
 ciao-controller/command.go    | 54 ++++++++++++++++++++++++++---------
 ciao-controller/image.go      |  6 ++--
 ciao-controller/instance.go   | 18 ++++++++----
 ciao-controller/legacy_api.go |  2 +-
 ciao-controller/main.go       |  9 ++++--
 ciao-controller/node.go       | 14 +++++++--
 ciao-controller/volume.go     |  4 +--
 11 files changed, 127 insertions(+), 43 deletions(-)

diff --git a/ciao-controller/api.go b/ciao-controller/api.go
index 05c8ee283..39bbb9bf7 100644
--- a/ciao-controller/api.go
+++ b/ciao-controller/api.go
@@ -352,7 +352,10 @@ func serversAction(c *controller, w http.ResponseWriter, r *http.Request) (APIRe
 				return errorResponse(err), err
 			}
 
-			actionFunc(instanceID)
+			err = actionFunc(instanceID)
+			if err != nil {
+				return errorResponse(err), err
+			}
 		}
 	} else {
 		/* We want to act on all relevant instances */
@@ -367,7 +370,10 @@ func serversAction(c *controller, w http.ResponseWriter, r *http.Request) (APIRe
 				continue
 			}
 
-			actionFunc(instance.ID)
+			err = actionFunc(instance.ID)
+			if err != nil {
+				return errorResponse(err), err
+			}
 		}
 	}
 
diff --git a/ciao-controller/api/api.go b/ciao-controller/api/api.go
index f8346a760..258e29c70 100644
--- a/ciao-controller/api/api.go
+++ b/ciao-controller/api/api.go
@@ -296,7 +296,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Content-Type", contentType)
 	w.WriteHeader(resp.status)
-	w.Write(b)
+	_, _ = w.Write(b)
 }
 
 func listResources(c *Context, w http.ResponseWriter, r *http.Request) (Response, error) {
diff --git a/ciao-controller/client.go b/ciao-controller/client.go
index 324a21874..419b4d0cb 100644
--- a/ciao-controller/client.go
+++ b/ciao-controller/client.go
@@ -147,7 +147,10 @@ func (client *ssntpClient) RemoveInstance(instanceID string) {
 	}
 
 	// notify anyone is listening for a state change
-	transitionInstanceState(i, payloads.Deleted)
+	err = transitionInstanceState(i, payloads.Deleted)
+	if err != nil {
+		glog.Warningf("Error transitioning CNCI to deleted: %v", err)
+	}
 }
 
 func (client *ssntpClient) instanceDeleted(payload []byte) {
@@ -222,7 +225,10 @@ func (client *ssntpClient) concentratorInstanceAdded(payload []byte) {
 		return
 	}
 
-	tenant.CNCIctrl.CNCIAdded(newCNCI.InstanceUUID)
+	err = tenant.CNCIctrl.CNCIAdded(newCNCI.InstanceUUID)
+	if err != nil {
+		glog.Warningf("Error adding CNCI: %v", err)
+	}
 }
 
 func (client *ssntpClient) traceReport(payload []byte) {
@@ -259,7 +265,10 @@ func (client *ssntpClient) nodeDisconnected(payload []byte) {
 	}
 
 	glog.Infof("Node %s disconnected", nodeDisconnected.Disconnected.NodeUUID)
-	client.ctl.ds.DeleteNode(nodeDisconnected.Disconnected.NodeUUID)
+	err = client.ctl.ds.DeleteNode(nodeDisconnected.Disconnected.NodeUUID)
+	if err != nil {
+		glog.Warningf("Error marking node as deleted in datastore: %v", err)
+	}
 }
 
 func (client *ssntpClient) unassignEvent(payload []byte) {
@@ -285,7 +294,10 @@ func (client *ssntpClient) unassignEvent(payload []byte) {
 	client.ctl.qs.Release(i.TenantID, payloads.RequestedResource{Type: payloads.ExternalIP, Value: 1})
 
 	msg := fmt.Sprintf("Unmapped %s from %s", event.UnassignedIP.PublicIP, event.UnassignedIP.PrivateIP)
-	client.ctl.ds.LogEvent(i.TenantID, msg)
+	err = client.ctl.ds.LogEvent(i.TenantID, msg)
+	if err != nil {
+		glog.Warningf("Error logging event: %v", err)
+	}
 }
 
 func (client *ssntpClient) assignEvent(payload []byte) {
@@ -303,7 +315,10 @@ func (client *ssntpClient) assignEvent(payload []byte) {
 	}
 
 	msg := fmt.Sprintf("Mapped %s to %s", event.AssignedIP.PublicIP, event.AssignedIP.PrivateIP)
-	client.ctl.ds.LogEvent(i.TenantID, msg)
+	err = client.ctl.ds.LogEvent(i.TenantID, msg)
+	if err != nil {
+		glog.Warningf("Error logging event: %v", err)
+	}
 }
 
 func (client *ssntpClient) EventNotify(event ssntp.Event, frame *ssntp.Frame) {
@@ -377,7 +392,10 @@ func (client *ssntpClient) startFailure(payload []byte) {
 			return
 		}
 
-		tenant.CNCIctrl.StartFailure(failure.InstanceUUID)
+		err = tenant.CNCIctrl.StartFailure(failure.InstanceUUID)
+		if err != nil {
+			glog.Warningf("Error adding StartFailure to datastore: %v", err)
+		}
 	}
 }
 
@@ -388,7 +406,7 @@ func (client *ssntpClient) stopFailure(payload []byte) {
 		glog.Warningf("Error unmarshalling StopFailure: %v", err)
 		return
 	}
-	client.ctl.ds.StopFailure(failure.InstanceUUID, failure.Reason)
+	err = client.ctl.ds.StopFailure(failure.InstanceUUID, failure.Reason)
 	if err != nil {
 		glog.Warningf("Error adding StopFailure to datastore: %v", err)
 	}
@@ -436,7 +454,10 @@ func (client *ssntpClient) assignError(payload []byte) {
 	client.ctl.qs.Release(failure.TenantUUID, payloads.RequestedResource{Type: payloads.ExternalIP, Value: 1})
 
 	msg := fmt.Sprintf("Failed to map %s to %s: %s", failure.PublicIP, failure.InstanceUUID, failure.Reason.String())
-	client.ctl.ds.LogError(failure.TenantUUID, msg)
+	err = client.ctl.ds.LogError(failure.TenantUUID, msg)
+	if err != nil {
+		glog.Warningf("Error logging error: %v", err)
+	}
 }
 
 func (client *ssntpClient) unassignError(payload []byte) {
@@ -449,7 +470,10 @@ func (client *ssntpClient) unassignError(payload []byte) {
 
 	// we can't unmap the IP - all we can do is log.
 	msg := fmt.Sprintf("Failed to unmap %s from %s: %s", failure.PublicIP, failure.InstanceUUID, failure.Reason.String())
-	client.ctl.ds.LogError(failure.TenantUUID, msg)
+	err = client.ctl.ds.LogError(failure.TenantUUID, msg)
+	if err != nil {
+		glog.Warningf("Error logging error: %v", err)
+	}
 }
 
 func (client *ssntpClient) ErrorNotify(err ssntp.Error, frame *ssntp.Frame) {
diff --git a/ciao-controller/cnci.go b/ciao-controller/cnci.go
index 77fb34741..0e73f70bf 100644
--- a/ciao-controller/cnci.go
+++ b/ciao-controller/cnci.go
@@ -102,7 +102,10 @@ func waitForEventTimeout(ch chan event, e event, timeout time.Duration) error {
 func (c *CNCI) transitionState(to CNCIState) {
 	glog.Infof("State transition to %s received for %s", to, c.instance.ID)
 
-	transitionInstanceState(c.instance, (string(to)))
+	err := transitionInstanceState(c.instance, (string(to)))
+	if err != nil {
+		glog.Warningf("Error transitioning instance %s to %s state", c.instance.ID, string(to))
+	}
 
 	// some state changes cause events
 	ch := c.eventCh
@@ -346,9 +349,9 @@ func (c *CNCIManager) CNCIStopped(id string) error {
 	}
 
 	cnci.transitionState(exited)
-	c.ctrl.restartInstance(cnci.instance.ID)
+	err := c.ctrl.restartInstance(cnci.instance.ID)
 
-	return nil
+	return errors.Wrap(err, "Error restarting instance")
 }
 
 // CNCIAdded will move the CNCI into the active state
diff --git a/ciao-controller/command.go b/ciao-controller/command.go
index 6a1c5cf13..204402ae9 100644
--- a/ciao-controller/command.go
+++ b/ciao-controller/command.go
@@ -48,10 +48,18 @@ func (c *controller) restartInstance(instanceID string) error {
 	}
 
 	if !i.CNCI {
-		t.CNCIctrl.WaitForActiveSubnetString(i.Subnet)
+		err = t.CNCIctrl.WaitForActiveSubnetString(i.Subnet)
+		if err != nil {
+			return errors.Wrap(err, "Error waiting for active subnet")
+		}
 	}
 
-	go c.client.RestartInstance(i, &w, t)
+	go func() {
+		if err := c.client.RestartInstance(i, &w, t); err != nil {
+			glog.Warningf("Error restarting instance: %v", err)
+		}
+	}()
+
 	return nil
 }
 
@@ -70,7 +78,12 @@ func (c *controller) stopInstance(instanceID string) error {
 		return errors.New("You may not stop a pending instance")
 	}
 
-	go c.client.StopInstance(instanceID, i.NodeID)
+	go func() {
+		if err := c.client.StopInstance(instanceID, i.NodeID); err != nil {
+			glog.Warningf("Error stopping instance: %v", err)
+		}
+	}()
+
 	return nil
 }
 
@@ -111,7 +124,10 @@ func (c *controller) deleteInstanceSync(instanceID string) error {
 	case <-wait:
 		return nil
 	case <-time.After(2 * time.Minute):
-		transitionInstanceState(i, payloads.Hung)
+		err = transitionInstanceState(i, payloads.Hung)
+		if err != nil {
+			glog.Warningf("Error transitioning instance to hung state: %v", err)
+		}
 		return fmt.Errorf("timeout waiting for delete")
 	}
 }
@@ -136,7 +152,12 @@ func (c *controller) deleteInstance(instanceID string) error {
 		}
 	}
 
-	go c.client.DeleteInstance(instanceID, i.NodeID)
+	go func() {
+		if err := c.client.DeleteInstance(instanceID, i.NodeID); err != nil {
+			glog.Warningf("Error deleting instance: %v", err)
+		}
+	}()
+
 	return nil
 }
 
@@ -243,7 +264,7 @@ func (c *controller) startWorkload(w types.WorkloadRequest) ([]*types.Instance,
 
 		ok, err := instance.Allowed()
 		if err != nil {
-			instance.Clean()
+			_ = instance.Clean()
 			e = errors.Wrap(err, "Error checking if instance allowed")
 			continue
 		}
@@ -251,19 +272,26 @@ func (c *controller) startWorkload(w types.WorkloadRequest) ([]*types.Instance,
 		if ok {
 			err = instance.Add()
 			if err != nil {
-				instance.Clean()
+				_ = instance.Clean()
 				e = errors.Wrap(err, "Error adding instance")
 				continue
 			}
 
 			newInstances = append(newInstances, instance.Instance)
-			if w.TraceLabel == "" {
-				go c.client.StartWorkload(instance.newConfig.config)
-			} else {
-				go c.client.StartTracedWorkload(instance.newConfig.config, instance.startTime, w.TraceLabel)
-			}
+			go func(label string) {
+				var err error
+				if label == "" {
+					err = c.client.StartWorkload(instance.newConfig.config)
+				} else {
+					err = c.client.StartTracedWorkload(instance.newConfig.config, instance.startTime, w.TraceLabel)
+				}
+
+				if err != nil {
+					glog.Warningf("Error starting workload: %v", err)
+				}
+			}(w.TraceLabel)
 		} else {
-			instance.Clean()
+			_ = instance.Clean()
 			// stop if we are over limits
 			e = errors.New("Over quota")
 			continue
diff --git a/ciao-controller/image.go b/ciao-controller/image.go
index 52e2ed88b..b8f7cd374 100644
--- a/ciao-controller/image.go
+++ b/ciao-controller/image.go
@@ -86,12 +86,12 @@ func (c *controller) uploadImage(imageID string, body io.Reader) error {
 	if err != nil {
 		return fmt.Errorf("Error creating temporary image file: %v", err)
 	}
-	defer os.Remove(f.Name())
+	defer func() { _ = os.Remove(f.Name()) }()
 
 	buf := make([]byte, 1<<16)
 	_, err = io.CopyBuffer(f, body, buf)
 	if err != nil {
-		f.Close()
+		_ = f.Close()
 		return fmt.Errorf("Error writing to temporary image file: %v", err)
 	}
 
@@ -107,7 +107,7 @@ func (c *controller) uploadImage(imageID string, body io.Reader) error {
 
 	err = c.CreateBlockDeviceSnapshot(imageID, "ciao-image")
 	if err != nil {
-		c.DeleteBlockDevice(imageID)
+		_ = c.DeleteBlockDevice(imageID)
 		return fmt.Errorf("Unable to create snapshot: %v", err)
 	}
 
diff --git a/ciao-controller/instance.go b/ciao-controller/instance.go
index 977eb7f7a..8b7af8308 100644
--- a/ciao-controller/instance.go
+++ b/ciao-controller/instance.go
@@ -144,7 +144,10 @@ func (i *instance) Clean() error {
 		return nil
 	}
 
-	i.ctl.ds.ReleaseTenantIP(i.TenantID, i.IPAddress)
+	err := i.ctl.ds.ReleaseTenantIP(i.TenantID, i.IPAddress)
+	if err != nil {
+		return errors.Wrap(err, "error releasing tenant IP")
+	}
 
 	wl, err := i.ctl.ds.GetWorkload(i.TenantID, i.WorkloadID)
 	if err != nil {
@@ -153,7 +156,12 @@ func (i *instance) Clean() error {
 	resources := []payloads.RequestedResource{{Type: payloads.Instance, Value: 1}}
 	resources = append(resources, wl.Defaults...)
 	i.ctl.qs.Release(i.TenantID, resources...)
-	i.ctl.deleteEphemeralStorage(i.ID)
+
+	err = i.ctl.deleteEphemeralStorage(i.ID)
+	if err != nil {
+		return errors.Wrap(err, "error deleting ephemeral strorage")
+	}
+
 	return nil
 }
 
@@ -232,7 +240,7 @@ func addBlockDevice(c *controller, tenant string, instanceID string, device stor
 			payloads.RequestedResource{Type: payloads.SharedDiskGiB, Value: device.Size})
 
 		if !res.Allowed() {
-			c.DeleteBlockDevice(device.ID)
+			_ = c.DeleteBlockDevice(device.ID)
 			c.qs.Release(tenant, res.Resources()...)
 			return payloads.StorageResource{}, fmt.Errorf("Error creating volume: %s", res.Reason())
 		}
@@ -240,7 +248,7 @@ func addBlockDevice(c *controller, tenant string, instanceID string, device stor
 
 	err := c.ds.AddBlockDevice(data)
 	if err != nil {
-		c.DeleteBlockDevice(device.ID)
+		_ = c.DeleteBlockDevice(device.ID)
 		return payloads.StorageResource{}, err
 	}
 
@@ -309,7 +317,7 @@ func getStorage(c *controller, s types.StorageResource, tenant string, instanceI
 	}
 
 	if err != nil {
-		c.DeleteBlockDevice(device.ID)
+		_ = c.DeleteBlockDevice(device.ID)
 		return payloads.StorageResource{}, errors.Wrap(err, "error resizing volume")
 	}
 
diff --git a/ciao-controller/legacy_api.go b/ciao-controller/legacy_api.go
index c33b53042..42539b635 100644
--- a/ciao-controller/legacy_api.go
+++ b/ciao-controller/legacy_api.go
@@ -74,7 +74,7 @@ func (h legacyAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(resp.status)
-	w.Write(b)
+	_, _ = w.Write(b)
 }
 
 func listTenantQuotas(c *controller, w http.ResponseWriter, r *http.Request) (APIResponse, error) {
diff --git a/ciao-controller/main.go b/ciao-controller/main.go
index a8fe2e389..f839deec5 100644
--- a/ciao-controller/main.go
+++ b/ciao-controller/main.go
@@ -85,7 +85,8 @@ func init() {
 	}
 
 	if logDirFlag.Value.String() == "" {
-		logDirFlag.Value.Set(logDir)
+		err := logDirFlag.Value.Set(logDir)
+		glog.Errorf("Error setting log directory: %v", err)
 	}
 
 	if err := os.MkdirAll(logDirFlag.Value.String(), 0755); err != nil {
@@ -135,7 +136,11 @@ func main() {
 	}
 
 	ctl.qs.Init()
-	populateQuotasFromDatastore(ctl.qs, ctl.ds)
+	err = populateQuotasFromDatastore(ctl.qs, ctl.ds)
+	if err != nil {
+		glog.Fatalf("Error populating quotas from datastore: %v", err)
+		return
+	}
 
 	config := &ssntp.Config{
 		URI:    *serverURL,
diff --git a/ciao-controller/node.go b/ciao-controller/node.go
index 39608d980..efef7cc46 100644
--- a/ciao-controller/node.go
+++ b/ciao-controller/node.go
@@ -14,13 +14,23 @@
 
 package main
 
+import "github.com/golang/glog"
+
 func (c *controller) EvacuateNode(nodeID string) error {
 	// should I bother to see if nodeID is valid?
-	go c.client.EvacuateNode(nodeID)
+	go func() {
+		if err := c.client.EvacuateNode(nodeID); err != nil {
+			glog.Warningf("Error evacuating node")
+		}
+	}()
 	return nil
 }
 
 func (c *controller) RestoreNode(nodeID string) error {
-	go c.client.RestoreNode(nodeID)
+	go func() {
+		if err := c.client.RestoreNode(nodeID); err != nil {
+			glog.Warning("Error restoring node")
+		}
+	}()
 	return nil
 }
diff --git a/ciao-controller/volume.go b/ciao-controller/volume.go
index 871c378a1..d412438b1 100644
--- a/ciao-controller/volume.go
+++ b/ciao-controller/volume.go
@@ -76,14 +76,14 @@ func (c *controller) CreateVolume(tenant string, req api.RequestedVolume) (types
 		payloads.RequestedResource{Type: payloads.SharedDiskGiB, Value: bd.Size})
 
 	if !res.Allowed() {
-		c.DeleteBlockDevice(bd.ID)
+		_ = c.DeleteBlockDevice(bd.ID)
 		c.qs.Release(tenant, res.Resources()...)
 		return types.Volume{}, api.ErrQuota
 	}
 
 	err = c.ds.AddBlockDevice(data)
 	if err != nil {
-		c.DeleteBlockDevice(bd.ID)
+		_ = c.DeleteBlockDevice(bd.ID)
 		c.qs.Release(tenant, res.Resources()...)
 		return types.Volume{}, err
 	}

From eb83201549d90c4aef27a35297e22cc3c63d0bb9 Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Wed, 25 Oct 2017 16:45:20 +0100
Subject: [PATCH 4/7] ciao-controller: Log whenever a request generates an
 error

When a request generates an error log this via glog. Now that we are
better at handling errors thoughout controller this change means that
all errors generated from requests will be logged into the controller
log. This should provide more details than the error reported to the
user as that is the result of conversion to a HTTP error code.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 ciao-controller/api/api.go    | 3 +++
 ciao-controller/legacy_api.go | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/ciao-controller/api/api.go b/ciao-controller/api/api.go
index 258e29c70..69b596120 100644
--- a/ciao-controller/api/api.go
+++ b/ciao-controller/api/api.go
@@ -27,6 +27,7 @@ import (
 	"github.com/ciao-project/ciao/ciao-controller/types"
 	"github.com/ciao-project/ciao/service"
 	"github.com/ciao-project/ciao/uuid"
+	"github.com/golang/glog"
 	"github.com/gorilla/mux"
 )
 
@@ -277,6 +278,8 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			Error: data,
 		}
 
+		glog.Warningf("Returning error response to request: %s: %v", r.URL.String(), err)
+
 		b, err := json.Marshal(code)
 		if err != nil {
 			http.Error(w, http.StatusText(resp.status), resp.status)
diff --git a/ciao-controller/legacy_api.go b/ciao-controller/legacy_api.go
index 42539b635..6eecd0b8d 100644
--- a/ciao-controller/legacy_api.go
+++ b/ciao-controller/legacy_api.go
@@ -21,6 +21,7 @@ import (
 	"net/http"
 
 	"github.com/ciao-project/ciao/service"
+	"github.com/golang/glog"
 	"github.com/gorilla/mux"
 )
 
@@ -55,6 +56,8 @@ func (h legacyAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			Error: data,
 		}
 
+		glog.Warningf("Returning error response to request: %s: %v", r.URL.String(), err)
+
 		b, err := json.Marshal(code)
 		if err != nil {
 			http.Error(w, http.StatusText(resp.status), resp.status)

From 68b0dd71f64572d47506b81aad3a8df7144d303e Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Wed, 25 Oct 2017 17:25:50 +0100
Subject: [PATCH 5/7] ciao-controller: errcheck: modify unit tests to handle
 errors

Ensure all errors generated by calls inside the unit test are either
handled or explicitly ignored.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 ciao-controller/compute_test.go    |  8 +++--
 ciao-controller/controller_test.go | 49 +++++++++++++++++++-----------
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/ciao-controller/compute_test.go b/ciao-controller/compute_test.go
index f2a3b0670..704b5cb60 100644
--- a/ciao-controller/compute_test.go
+++ b/ciao-controller/compute_test.go
@@ -34,6 +34,7 @@ import (
 	"github.com/ciao-project/ciao/payloads"
 	"github.com/ciao-project/ciao/ssntp"
 	"github.com/ciao-project/ciao/testutil"
+	"github.com/pkg/errors"
 )
 
 // ByTenantID is used to sort CNCI instances by Tenant ID.
@@ -79,7 +80,7 @@ func testHTTPRequest(t *testing.T, method string, URL string, expectedResponse i
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != expectedResponse {
 		var msg string
@@ -465,7 +466,10 @@ func sendStopEvent(client *testutil.SsntpTestClient, instanceUUID string) error
 		return fmt.Errorf("Unable to create InstanceStopped payload : %v", err)
 	}
 	clientEvtCh := wrappedClient.addEventChan(ssntp.InstanceStopped)
-	client.Ssntp.SendEvent(ssntp.InstanceStopped, y)
+	_, err = client.Ssntp.SendEvent(ssntp.InstanceStopped, y)
+	if err != nil {
+		return errors.Wrap(err, "Error sending instance stopped")
+	}
 	err = wrappedClient.getEventChan(clientEvtCh, ssntp.InstanceStopped)
 	if err != nil {
 		return fmt.Errorf("InstanceStopped event not received: %v", err)
diff --git a/ciao-controller/controller_test.go b/ciao-controller/controller_test.go
index 30de443d2..7d83a3ff7 100644
--- a/ciao-controller/controller_test.go
+++ b/ciao-controller/controller_test.go
@@ -602,7 +602,7 @@ func addTestBlockDevice(t *testing.T, tenantID string) types.Volume {
 
 	err = ctl.ds.AddBlockDevice(data)
 	if err != nil {
-		ctl.DeleteBlockDevice(bd.ID)
+		_ = ctl.DeleteBlockDevice(bd.ID)
 		t.Fatal(err)
 	}
 
@@ -851,7 +851,10 @@ func TestStartFailure(t *testing.T) {
 }
 
 func TestStopFailure(t *testing.T) {
-	ctl.ds.ClearLog()
+	err := ctl.ds.ClearLog()
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	var reason payloads.StartFailureReason
 
@@ -866,7 +869,7 @@ func TestStopFailure(t *testing.T) {
 	serverCh := server.AddCmdChan(ssntp.DELETE)
 	controllerCh := wrappedClient.addErrorChan(ssntp.DeleteFailure)
 
-	err := ctl.stopInstance(instances[0].ID)
+	err = ctl.stopInstance(instances[0].ID)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -885,7 +888,10 @@ func TestStopFailure(t *testing.T) {
 }
 
 func TestRestartFailure(t *testing.T) {
-	ctl.ds.ClearLog()
+	err := ctl.ds.ClearLog()
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	var reason payloads.StartFailureReason
 
@@ -900,7 +906,7 @@ func TestRestartFailure(t *testing.T) {
 	serverCh := server.AddCmdChan(ssntp.DELETE)
 	clientCh := client.AddCmdChan(ssntp.DELETE)
 
-	err := ctl.stopInstance(instances[0].ID)
+	err = ctl.stopInstance(instances[0].ID)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1207,7 +1213,7 @@ func TestGetStorageForVolume(t *testing.T) {
 	}
 
 	sourceVolume := addTestBlockDevice(t, tenant.ID)
-	defer ctl.DeleteBlockDevice(sourceVolume.ID)
+	defer func() { _ = ctl.DeleteBlockDevice(sourceVolume.ID) }()
 
 	// a temporary in memory filesystem?
 	s := types.StorageResource{
@@ -1258,7 +1264,7 @@ func TestGetStorageForImage(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer os.Remove(tmpfile.Name())
+	defer func() { _ = os.Remove(tmpfile.Name()) }()
 
 	// a temporary in memory filesystem?
 	s := types.StorageResource{
@@ -1315,7 +1321,7 @@ func TestStorageConfig(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer os.Remove(tmpfile.Name())
+	defer func() { _ = os.Remove(tmpfile.Name()) }()
 
 	info, err := tmpfile.Stat()
 	if err != nil {
@@ -1566,18 +1572,27 @@ func deletePool(name string) error {
 func TestAddPoolWithSubnet(t *testing.T) {
 	subnet := "192.168.0.0/16"
 	testAddPool(t, "test1", &subnet, []string{})
-	deletePool("test1")
+	err := deletePool("test1")
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestAddPoolWithIPs(t *testing.T) {
 	ips := []string{"10.10.0.1", "10.10.0.2"}
 	testAddPool(t, "test2", nil, ips)
-	deletePool("test2")
+	err := deletePool("test2")
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestAddPool(t *testing.T) {
 	testAddPool(t, "test3", nil, []string{})
-	deletePool("test3")
+	err := deletePool("test3")
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestListPools(t *testing.T) {
@@ -2069,7 +2084,7 @@ func TestMain(m *testing.M) {
 
 	f, err := os.Create(fakeImage)
 	if err != nil {
-		os.RemoveAll(dir)
+		_ = os.RemoveAll(dir)
 		os.Exit(1)
 	}
 
@@ -2080,8 +2095,8 @@ func TestMain(m *testing.M) {
 
 	err = ctl.ds.Init(dsConfig)
 	if err != nil {
-		f.Close()
-		os.RemoveAll(dir)
+		_ = f.Close()
+		_ = os.RemoveAll(dir)
 		os.Exit(1)
 	}
 
@@ -2109,7 +2124,7 @@ func TestMain(m *testing.M) {
 		os.Exit(1)
 	}
 
-	go s.ListenAndServeTLS(httpsCAcert, httpsKey)
+	go func() { _ = s.ListenAndServeTLS(httpsCAcert, httpsKey) }()
 	time.Sleep(1 * time.Second)
 
 	code := m.Run()
@@ -2118,8 +2133,8 @@ func TestMain(m *testing.M) {
 	ctl.ds.Exit()
 	ctl.qs.Shutdown()
 	server.Shutdown()
-	f.Close()
-	os.RemoveAll(dir)
+	_ = f.Close()
+	_ = os.RemoveAll(dir)
 
 	os.Exit(code)
 }

From 731e77f3b980c19810bb878fa25d2562c10bf5a0 Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Thu, 26 Oct 2017 15:20:22 +0100
Subject: [PATCH 6/7] ciao-controller: Fix testServersActionStart to wait for
 DELETE event

Fix unit test that started to fail now that we are actually checking
errors which was caused by failing to wait for a DELETE event as a
result of an instance being stopped.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 ciao-controller/compute_test.go | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/ciao-controller/compute_test.go b/ciao-controller/compute_test.go
index 704b5cb60..a1828a011 100644
--- a/ciao-controller/compute_test.go
+++ b/ciao-controller/compute_test.go
@@ -296,16 +296,22 @@ func testServersActionStart(t *testing.T, httpExpectedStatus int, validToken boo
 
 	time.Sleep(1 * time.Second)
 
+	serverCh := server.AddCmdChan(ssntp.DELETE)
+
 	err = ctl.stopInstance(servers.Servers[0].ID)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	time.Sleep(1 * time.Second)
-
-	sendStatsCmd(client, t)
+	err = sendStopEvent(client, servers.Servers[0].ID)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	time.Sleep(1 * time.Second)
+	_, err = server.GetCmdChanResult(serverCh, ssntp.DELETE)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	var ids []string
 	ids = append(ids, servers.Servers[0].ID)

From a9e2cffc1c1d4b16d6ea0a48ff945d8e0ac0d975 Mon Sep 17 00:00:00 2001
From: Rob Bradford <robert.bradford@intel.com>
Date: Thu, 26 Oct 2017 16:26:19 +0100
Subject: [PATCH 7/7] ciao-controller: Remove incorrect closure of request body

The request body should be not be closed by the HTTP handling code that
is done by the standard library if necessary.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
---
 ciao-controller/api/api.go | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/ciao-controller/api/api.go b/ciao-controller/api/api.go
index 69b596120..8256e4191 100644
--- a/ciao-controller/api/api.go
+++ b/ciao-controller/api/api.go
@@ -922,8 +922,6 @@ func validPrivilege(visibility types.Visibility, privileged bool) bool {
 // createImage creates information about an image, but doesn't contain
 // any actual image.
 func createImage(context *Context, w http.ResponseWriter, r *http.Request) (Response, error) {
-	defer r.Body.Close()
-
 	vars := mux.Vars(r)
 	tenantID := vars["tenant"]
 
@@ -1028,8 +1026,6 @@ func createVolume(bc *Context, w http.ResponseWriter, r *http.Request) (Response
 	vars := mux.Vars(r)
 	tenant := vars["tenant"]
 
-	defer r.Body.Close()
-
 	body, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		return Response{http.StatusBadRequest, nil}, err
@@ -1142,8 +1138,6 @@ func volumeAction(bc *Context, w http.ResponseWriter, r *http.Request) (Response
 
 	var req interface{}
 
-	defer r.Body.Close()
-
 	body, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		return Response{http.StatusBadRequest, nil}, err
@@ -1173,8 +1167,6 @@ func createInstance(c *Context, w http.ResponseWriter, r *http.Request) (Respons
 	vars := mux.Vars(r)
 	tenant := vars["tenant"]
 
-	defer r.Body.Close()
-
 	body, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		return Response{http.StatusBadRequest, nil}, err
@@ -1265,8 +1257,6 @@ func instanceAction(c *Context, w http.ResponseWriter, r *http.Request) (Respons
 	tenant := vars["tenant"]
 	server := vars["instance_id"]
 
-	defer r.Body.Close()
-
 	body, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		return Response{http.StatusBadRequest, nil}, err