Skip to content

Fix nil-interface partialFailure bug #1294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 19, 2016
Merged

Fix nil-interface partialFailure bug #1294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions manager/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
14 changes: 10 additions & 4 deletions manager/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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]
Expand Down