From 7fb40c802a0989dac206cb18aa5a9694c0056281 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 14 Jun 2016 16:53:23 +0000 Subject: [PATCH] Review feedback --- probe/docker/registry.go | 28 +++++++++++----------------- probe/docker/registry_test.go | 16 ++++++++-------- probe/docker/reporter.go | 4 ++-- probe/docker/reporter_test.go | 14 +++++++------- 4 files changed, 28 insertions(+), 34 deletions(-) diff --git a/probe/docker/registry.go b/probe/docker/registry.go index 645c7f9ce..2f4d111a6 100644 --- a/probe/docker/registry.go +++ b/probe/docker/registry.go @@ -39,8 +39,8 @@ type Registry interface { Stop() LockedPIDLookup(f func(func(int) Container)) WalkContainers(f func(Container)) - WalkImages(f func(*docker_client.APIImages)) - WalkNetworks(f func(*docker_client.Network)) + WalkImages(f func(docker_client.APIImages)) + WalkNetworks(f func(docker_client.Network)) WatchContainerUpdates(ContainerUpdateWatcher) GetContainer(string) (Container, bool) GetContainerByPrefix(string) (Container, bool) @@ -61,8 +61,8 @@ type registry struct { watchers []ContainerUpdateWatcher containers *radix.Tree containersByPID map[int]Container - images map[string]*docker_client.APIImages - networks []*docker_client.Network + images map[string]docker_client.APIImages + networks []docker_client.Network } // Client interface for mocking. @@ -99,7 +99,7 @@ func NewRegistry(interval time.Duration, pipes controls.PipeClient, collectStats r := ®istry{ containers: radix.New(), containersByPID: map[int]Container{}, - images: map[string]*docker_client.APIImages{}, + images: map[string]docker_client.APIImages{}, client: client, pipes: pipes, @@ -228,7 +228,7 @@ func (r *registry) reset() { r.containers = radix.New() r.containersByPID = map[int]Container{} - r.images = map[string]*docker_client.APIImages{} + r.images = map[string]docker_client.APIImages{} r.networks = r.networks[:0] } @@ -254,8 +254,7 @@ func (r *registry) updateImages() error { r.Lock() defer r.Unlock() - for i := range images { - image := &images[i] + for _, image := range images { r.images[trimImageID(image.ID)] = image } @@ -269,13 +268,8 @@ func (r *registry) updateNetworks() error { } r.Lock() - defer r.Unlock() - - // reset - r.networks = r.networks[:0] - for i := range networks { - r.networks = append(r.networks, &networks[i]) - } + r.networks = networks + r.Unlock() return nil } @@ -424,7 +418,7 @@ func (r *registry) GetContainerByPrefix(prefix string) (Container, bool) { // WalkImages runs f on every image of running containers the registry // knows of. f may be run on the same image more than once. -func (r *registry) WalkImages(f func(*docker_client.APIImages)) { +func (r *registry) WalkImages(f func(docker_client.APIImages)) { r.RLock() defer r.RUnlock() @@ -439,7 +433,7 @@ func (r *registry) WalkImages(f func(*docker_client.APIImages)) { } // WalkNetworks runs f on every network the registry knows of. -func (r *registry) WalkNetworks(f func(*docker_client.Network)) { +func (r *registry) WalkNetworks(f func(docker_client.Network)) { r.RLock() defer r.RUnlock() diff --git a/probe/docker/registry_test.go b/probe/docker/registry_test.go index 4fb8d0de7..cec900644 100644 --- a/probe/docker/registry_test.go +++ b/probe/docker/registry_test.go @@ -300,17 +300,17 @@ func allContainers(r docker.Registry) []docker.Container { return result } -func allImages(r docker.Registry) []*client.APIImages { - result := []*client.APIImages{} - r.WalkImages(func(i *client.APIImages) { +func allImages(r docker.Registry) []client.APIImages { + result := []client.APIImages{} + r.WalkImages(func(i client.APIImages) { result = append(result, i) }) return result } -func allNetworks(r docker.Registry) []*client.Network { - result := []*client.Network{} - r.WalkNetworks(func(i *client.Network) { +func allNetworks(r docker.Registry) []client.Network { + result := []client.Network{} + r.WalkNetworks(func(i client.Network) { result = append(result, i) }) return result @@ -331,14 +331,14 @@ func TestRegistry(t *testing.T) { } { - want := []*client.APIImages{&apiImage1} + want := []client.APIImages{apiImage1} test.Poll(t, 100*time.Millisecond, want, func() interface{} { return allImages(registry) }) } { - want := []*client.Network{&network1} + want := []client.Network{network1} test.Poll(t, 100*time.Millisecond, want, func() interface{} { return allNetworks(registry) }) diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go index d7186d96c..04593f0ff 100644 --- a/probe/docker/reporter.go +++ b/probe/docker/reporter.go @@ -226,7 +226,7 @@ func (r *Reporter) containerImageTopology() report.Topology { WithMetadataTemplates(ContainerImageMetadataTemplates). WithTableTemplates(ContainerImageTableTemplates) - r.registry.WalkImages(func(image *docker_client.APIImages) { + r.registry.WalkImages(func(image docker_client.APIImages) { imageID := trimImageID(image.ID) nodeID := report.MakeContainerImageNodeID(imageID) node := report.MakeNodeWith(nodeID, map[string]string{ @@ -246,7 +246,7 @@ func (r *Reporter) containerImageTopology() report.Topology { func (r *Reporter) overlayTopology() report.Topology { localSubnets := []string{} - r.registry.WalkNetworks(func(network *docker_client.Network) { + r.registry.WalkNetworks(func(network docker_client.Network) { if network.Scope == "local" { for _, config := range network.IPAM.Config { localSubnets = append(localSubnets, config.Subnet) diff --git a/probe/docker/reporter_test.go b/probe/docker/reporter_test.go index 009eb3123..5a8f9df0f 100644 --- a/probe/docker/reporter_test.go +++ b/probe/docker/reporter_test.go @@ -12,8 +12,8 @@ import ( type mockRegistry struct { containersByPID map[int]docker.Container - images map[string]*client.APIImages - networks []*client.Network + images map[string]client.APIImages + networks []client.Network } func (r *mockRegistry) Stop() {} @@ -30,13 +30,13 @@ func (r *mockRegistry) WalkContainers(f func(docker.Container)) { } } -func (r *mockRegistry) WalkImages(f func(*client.APIImages)) { +func (r *mockRegistry) WalkImages(f func(client.APIImages)) { for _, i := range r.images { f(i) } } -func (r *mockRegistry) WalkNetworks(f func(*client.Network)) { +func (r *mockRegistry) WalkNetworks(f func(client.Network)) { for _, i := range r.networks { f(i) } @@ -54,10 +54,10 @@ var ( containersByPID: map[int]docker.Container{ 2: &mockContainer{container1}, }, - images: map[string]*client.APIImages{ - imageID: &apiImage1, + images: map[string]client.APIImages{ + imageID: apiImage1, }, - networks: []*client.Network{&network1}, + networks: []client.Network{network1}, } )