Skip to content

Commit 7a93c73

Browse files
committed
Always delete container info when we get a 'destroy' event
Previously it would only delete if Docker said the container didn't exist, which is a race between Docker sending the event and Docker removing the info from its own records. Extract a couple of functions to make the action clearer.
1 parent e69491f commit 7a93c73

File tree

1 file changed

+37
-38
lines changed

1 file changed

+37
-38
lines changed

probe/docker/registry.go

+37-38
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ func (r *registry) updateContainers() error {
262262
}
263263

264264
for _, apiContainer := range apiContainers {
265-
r.updateContainerState(apiContainer.ID, nil)
265+
r.updateContainerState(apiContainer.ID)
266266
}
267267

268268
return nil
@@ -300,55 +300,28 @@ func (r *registry) updateNetworks() error {
300300
func (r *registry) handleEvent(event *docker_client.APIEvents) {
301301
// TODO: Send shortcut reports on networks being created/destroyed?
302302
switch event.Status {
303-
case CreateEvent, RenameEvent, StartEvent, DieEvent, DestroyEvent, PauseEvent, UnpauseEvent, NetworkConnectEvent, NetworkDisconnectEvent:
304-
r.updateContainerState(event.ID, stateAfterEvent(event.Status))
305-
}
306-
}
307-
308-
func stateAfterEvent(event string) *string {
309-
switch event {
303+
case CreateEvent, RenameEvent, StartEvent, DieEvent, PauseEvent, UnpauseEvent, NetworkConnectEvent, NetworkDisconnectEvent:
304+
r.updateContainerState(event.ID)
310305
case DestroyEvent:
311-
return &StateDeleted
312-
default:
313-
return nil
306+
r.Lock()
307+
r.deleteContainer(event.ID)
308+
r.Unlock()
309+
r.sendDeletedUpdate(event.ID)
314310
}
315311
}
316312

317-
func (r *registry) updateContainerState(containerID string, intendedState *string) {
313+
func (r *registry) updateContainerState(containerID string) {
318314
r.Lock()
319315
defer r.Unlock()
320316

321317
dockerContainer, err := r.client.InspectContainer(containerID)
322318
if err != nil {
323-
// Don't spam the logs if the container was short lived
324319
if _, ok := err.(*docker_client.NoSuchContainer); !ok {
325-
log.Errorf("Error processing event for container %s: %v", containerID, err)
320+
log.Errorf("Unable to get status for container %s: %v", containerID, err)
326321
return
327322
}
328-
329-
// Container doesn't exist anymore, so lets stop and remove it
330-
c, ok := r.containers.Get(containerID)
331-
if !ok {
332-
return
333-
}
334-
container := c.(Container)
335-
336-
r.containers.Delete(containerID)
337-
delete(r.containersByPID, container.PID())
338-
if r.collectStats {
339-
container.StopGatheringStats()
340-
}
341-
342-
if intendedState != nil {
343-
node := report.MakeNodeWith(report.MakeContainerNodeID(containerID), map[string]string{
344-
ContainerID: containerID,
345-
ContainerState: *intendedState,
346-
})
347-
// Trigger anyone watching for updates
348-
for _, f := range r.watchers {
349-
f(node)
350-
}
351-
}
323+
// Docker says the container doesn't exist - remove it from our data
324+
r.deleteContainer(containerID)
352325
return
353326
}
354327

@@ -389,6 +362,32 @@ func (r *registry) updateContainerState(containerID string, intendedState *strin
389362
}
390363
}
391364

365+
func (r *registry) deleteContainer(containerID string) {
366+
// Container doesn't exist anymore, so lets stop and remove it
367+
c, ok := r.containers.Get(containerID)
368+
if !ok {
369+
return
370+
}
371+
container := c.(Container)
372+
373+
r.containers.Delete(containerID)
374+
delete(r.containersByPID, container.PID())
375+
if r.collectStats {
376+
container.StopGatheringStats()
377+
}
378+
}
379+
380+
func (r *registry) sendDeletedUpdate(containerID string) {
381+
node := report.MakeNodeWith(report.MakeContainerNodeID(containerID), map[string]string{
382+
ContainerID: containerID,
383+
ContainerState: StateDeleted,
384+
})
385+
// Trigger anyone watching for updates
386+
for _, f := range r.watchers {
387+
f(node)
388+
}
389+
}
390+
392391
// LockedPIDLookup runs f under a read lock, and gives f a function for
393392
// use doing pid->container lookups.
394393
func (r *registry) LockedPIDLookup(f func(func(int) Container)) {

0 commit comments

Comments
 (0)