Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

Use container-level flag to determine when logs should be consumed #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

seubert
Copy link

@seubert seubert commented Feb 26, 2019

This fixes a bug described here:

Here you can see newDriver being called https://github.com/solarwinds/docker-plugin-papertrail/blob/master/main.go#L31 which sets a boolean on the newly initialized driver to true. This boolean determines whether or not consumeLog continues to consume logs (seen here https://github.com/solarwinds/docker-plugin-papertrail/blob/master/driver.go#L112). Now that works when you’re just dealing with one container and not stopping or starting anything. However, when you stop a container and StopLogging gets called (https://github.com/solarwinds/docker-plugin-papertrail/blob/master/http.go#L55 and https://github.com/solarwinds/docker-plugin-papertrail/blob/master/driver.go#L94) that boolean gets set to false which stops log consumption. Nothing calls newDriver again to reset that variable to true so logs never start getting consumed again.

Since the driver is being shared amongst the handlers for the lifetime of the plugin, it can't be holding onto file/container-specific logging state, which it is prior to this patch. The ReadLogs function still depends on loopFactor and I haven't determined whether or not that needs to change but likely so. If so, it will be in a separate PR.

@trevrosen
Copy link

This seems to solve the essential problem of putting the state in the right place, and is along the lines I envisioned when I read the bug yesterday.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants