Merge pull request #1294 from timstclair/partial-failure

Fix nil-interface partialFailure bug
google · May 19, 2016 · 7ddf6eb · 7ddf6eb
2 parents fb5fb83 + 81786ec
commit 7ddf6eb
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 8 deletions.
diff --git a/manager/manager.go b/manager/manager.go
@@ -382,7 +382,7 @@ func (self *manager) GetDerivedStats(containerName string, options v2.RequestOpt
 		}
 		stats[name] = d
 	}
-	return stats, errs
+	return stats, errs.OrNil()
 }
 
 func (self *manager) GetContainerSpec(containerName string, options v2.RequestOptions) (map[string]v2.ContainerSpec, error) {
@@ -400,7 +400,7 @@ func (self *manager) GetContainerSpec(containerName string, options v2.RequestOp
 		spec := self.getV2Spec(cinfo)
 		specs[name] = spec
 	}
-	return specs, errs
+	return specs, errs.OrNil()
 }
 
 // Get V2 container spec from v1 container info.
@@ -461,7 +461,7 @@ func (self *manager) GetContainerInfoV2(containerName string, options v2.Request
 		infos[name] = result
 	}
 
-	return infos, errs
+	return infos, errs.OrNil()
 }
 
 func (self *manager) containerDataToContainerInfo(cont *containerData, query *info.ContainerInfoRequest) (*info.ContainerInfo, error) {
@@ -614,7 +614,7 @@ func (self *manager) GetRequestedContainersInfo(containerName string, options v2
 		}
 		containersMap[name] = info
 	}
-	return containersMap, errs
+	return containersMap, errs.OrNil()
 }
 
 func (self *manager) getRequestedContainers(containerName string, options v2.RequestOptions) (map[string]*containerData, error) {
@@ -1241,3 +1241,10 @@ func (f *partialFailure) append(id, operation string, err error) {
 func (f partialFailure) Error() string {
 	return fmt.Sprintf("partial failures: %s", strings.Join(f, ", "))
 }
+
+func (f partialFailure) OrNil() error {
+	if len(f) == 0 {
+		return nil
+	}
+	return f
+}
diff --git a/manager/manager_test.go b/manager/manager_test.go
@@ -32,7 +32,6 @@ import (
 	"github.com/google/cadvisor/info/v2"
 	"github.com/google/cadvisor/utils/sysfs/fakesysfs"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 // TODO(vmarmol): Refactor these tests.
@@ -174,7 +173,9 @@ func TestGetContainerInfoV2(t *testing.T) {
 	m, _, handlerMap := expectManagerWithContainers(containers, query, t)
 
 	infos, err := m.GetContainerInfoV2("/", options)
-	require.NoError(t, err, "Error calling GetContainerInfoV2")
+	if err != nil {
+		t.Fatalf("GetContainerInfoV2 failed: %v", err)
+	}
 
 	for container, handler := range handlerMap {
 		handler.AssertExpectations(t)
@@ -205,7 +206,10 @@ func TestGetContainerInfoV2Failure(t *testing.T) {
 	m, _, handlerMap := expectManagerWithContainers(containers, query, t)
 
 	// Remove /c1 stats
-	require.NoError(t, m.memoryCache.RemoveContainer(statless))
+	err := m.memoryCache.RemoveContainer(statless)
+	if err != nil {
+		t.Fatalf("RemoveContainer failed: %v", err)
+	}
 
 	// Make GetSpec fail on /c2
 	mockErr := fmt.Errorf("intentional GetSpec failure")
@@ -216,7 +220,9 @@ func TestGetContainerInfoV2Failure(t *testing.T) {
 	m.containers[namespacedContainerName{Name: failing}].lastUpdatedTime = time.Time{} // Force GetSpec.
 
 	infos, err := m.GetContainerInfoV2("/", options)
-	assert.Error(t, err, "Expected error calling GetContainerInfoV2")
+	if err == nil {
+		t.Error("Expected error calling GetContainerInfoV2")
+	}
 
 	// Successful containers still successful.
 	info, ok := infos[successful]