reconnect log stream unless container is dead #191

bobzoller · 2016-05-19T23:40:54Z

I believe this fixes #168 (at least my own root cause, which seems to be moby/moby#21220).

tested manually; will report back when I've tested it more widely.

johanndt · 2016-05-31T05:26:45Z

Hi @bobzoller - any more results from your testing? Do you have an image up on docker hub I can also try?

johanndt · 2016-05-31T23:29:36Z

For info, I searched around a bit and found an image at https://hub.docker.com/r/goodeggs/logspout/tags/ with the reconnect-logs tag. We've been running it and while some containers seem to be faring better, we are still losing some in papertrail after about half a day.

rdbaron · 2016-06-01T14:04:39Z

router/pump.go

+				if !four04 {
+					assert(err, "pump")
+				}
+			} else if container.State.Running {


I found a problem here when testing this fix. My container restarts so quickly that this check always returns true. This causes the deletion of the pump below to be skipped and each time my container restarts I get a new one. This leads to LOTS of duplication.

I haven't run into this problem, but it makes perfect sense. Off the cuff it seems like we could add a check around line 153 and only create a new pump if one does not exist for that container. eg if _, ok := p.pumps[id]; !ok { }. Happy to be corrected.

That should work, mind modifying and testing out that solution?

bobzoller · 2016-06-09T22:51:38Z

I've pushed a new version of docker image goodeggs/logspout:reconnect-logs with the fix in 13d1129. It's running fine on my dev cluster, but perhaps @rdbaron could attempt to reproduce the issue and let us know if it is fixed?

rdbaron · 2016-06-10T14:11:22Z

@bobzoller: will do, I'll let you know once I have.

rdbaron · 2016-06-13T18:22:57Z

@bobzoller; I can confirm that this fixes the issue with duplicating the pump when the container restarts quickly.

bobzoller · 2016-06-13T18:34:32Z

great, thanks! @josegonzalez seems like this might be ready to merge.

josegonzalez · 2016-06-13T19:43:05Z

Works for me :)

josegonzalez · 2016-06-13T19:43:34Z

Thanks to everyone for being diligent about this PR. Please direct me to any other ones we need to shepherd to greatness :)

mattatcha · 2016-06-13T19:43:47Z

I am working on a solution for #187 which may also take care of this.

I also think there are some edge cases this doesn't take care of.

josegonzalez · 2016-06-13T19:44:53Z

@MattAitchison Feel free to override me on this PR, I consider logspout your baby :)

bobzoller · 2016-06-13T20:10:14Z

yes @MattAitchison , and I was fairly certain there were some edge cases this didn't take care of ;)

The following commits are included from gliderlabs/master: commit a415f5d Merge: d56f003 324db6e Author: Jose Diaz-Gonzalez <josegonzalez@users.noreply.github.com> Date: Fri Jul 1 12:14:40 2016 -0400 Merge pull request gliderlabs#203 from udacity/master ignore containers with unsupported log drivers commit 324db6e Author: Paul M Bauer <paul@bauer.codes> Date: Fri Jul 1 08:22:56 2016 -0700 ignore containers with unsupported log drivers trying to hit /logs for a container that does not support that endpoint results in the following error in that container's stdout "Error running logs job: configured logging reader does not support reading" commit d56f003 Merge: 685c7bc 00147aa Author: Jose Diaz-Gonzalez <josegonzalez@users.noreply.github.com> Date: Thu Jun 30 18:13:11 2016 -0400 Merge pull request gliderlabs#171 from selimekizoglu/exclude-by-label Exclude containers by label. commit 00147aa Author: Selim Ekizoglu <selimeki@gmail.com> Date: Fri Jul 1 00:31:01 2016 +0300 Ignore empty EXCLUDE_LABEL values commit e92c2a0 Author: Selim Ekizoglu <selimeki@gmail.com> Date: Fri Jul 1 00:00:16 2016 +0300 Fixed documentation commit 079e8ae Author: Selim Ekizoglu <selimeki@gmail.com> Date: Thu Jun 30 23:33:44 2016 +0300 Renamed environment variable defining the label commit 685c7bc Merge: 5cad24a ff57c7d Author: Jose Diaz-Gonzalez <josegonzalez@users.noreply.github.com> Date: Thu Jun 30 12:06:54 2016 -0400 Merge pull request gliderlabs#200 from digitalkaoz/master Make Build Script Extentable commit 7ed5fb4 Author: d-sekizoglu <d-sekizoglu@ziraatteknoloji.com> Date: Thu Jun 30 11:56:06 2016 +0300 Documentation added commit 5cad24a Merge: 97c8c67 3ffcd8e Author: Jose Diaz-Gonzalez <josegonzalez@users.noreply.github.com> Date: Wed Jun 29 16:30:02 2016 -0400 Merge pull request gliderlabs#199 from iron-io/module-help Some instructions and helper scripts for working on custom modules. commit 97c8c67 Merge: a923387 13d1129 Author: Jose Diaz-Gonzalez <josegonzalez@users.noreply.github.com> Date: Mon Jun 13 15:43:10 2016 -0400 Merge pull request gliderlabs#191 from goodeggs/reconnect-logs reconnect log stream unless container is dead commit 13d1129 Author: Bob Zoller <bob@zoller.us> Date: Wed Jun 8 12:07:34 2016 -0700 avoid duplicate pumps with mutex lock commit ff57c7d Author: Robert Schönthal <caziel@gmx.net> Date: Mon Jun 6 23:01:51 2016 +0200 Update Dockerfile commit 3ffcd8e Author: Travis Reeder <treeder@gmail.com> Date: Tue May 31 20:04:53 2016 -0700 Some help for working on custom modules. commit a923387 Author: Matt Aitchison <Matt@lanciv.com> Date: Thu May 26 15:41:51 2016 -0500 Fix for gliderlabs#193 commit 38fa4b7 Author: Matt Aitchison <Matt@lanciv.com> Date: Mon May 23 16:38:58 2016 -0500 bump commit 19fa188 Author: Matt Aitchison <Matt@lanciv.com> Date: Mon May 23 16:26:22 2016 -0500 fixed version and date... commit 72dada2 Author: Matt Aitchison <Matt@lanciv.com> Date: Mon May 23 16:24:53 2016 -0500 release prep commit a86151b Author: Matt Aitchison <Matt@lanciv.com> Date: Mon May 23 16:21:24 2016 -0500 Update chnagelog commit 118f322 Author: Bob Zoller <bob@zoller.us> Date: Thu May 19 13:22:11 2016 -0700 reconnect log stream unless container is dead commit 23d3c9f Author: Matt Aitchison <Matt@lanciv.com> Date: Thu May 19 11:34:37 2016 -0500 Quick fix for gliderlabs#185 commit 231ac0e Author: Dylan Meissner <dylanmei@gmail.com> Date: Tue May 17 13:41:02 2016 -0700 Add logspout-kafka module to README (gliderlabs#77) commit 457941d Author: Tony Nuzzi <nuzzis@sbcglobal.net> Date: Tue May 17 15:37:58 2016 -0500 Add support text and command to stream logs to Loggly (gliderlabs#186) commit 0175188 Author: R. Toma <rtoma@bol.com> Date: Tue May 17 22:35:07 2016 +0200 added link to the logspout-redis-logstash module (gliderlabs#172) Change-Id: I69df9d76f472e84116a8e1b5b9e0c3cb255fe663 commit 8e54dea Author: R. Toma <rtoma@bol.com> Date: Tue May 17 22:22:26 2016 +0200 Now using Alpine Linux 3.3 and GO 1.5.3, removed the "edge" package repo for building the official Docker image (gliderlabs#174) Change-Id: I62a7960748906c1f9f7609dfc849f5ce0ce436fb commit 1bfbc27 Author: Matt Aitchison <Matt@lanciv.com> Date: Tue May 17 15:07:47 2016 -0500 Fixes issue gliderlabs#183 commit b6acebc Author: Matt Aitchison <Matt@lanciv.com> Date: Thu May 12 23:20:35 2016 -0500 cleaned up setup method commit 67eaa00 Author: Matt Aitchison <Matt@lanciv.com> Date: Wed May 11 18:45:21 2016 -0500 fix RouteManager.Name() commit 90302f0 Author: Michael Hobbs <michaelshobbs@users.noreply.github.com> Date: Wed Apr 20 19:46:57 2016 -0700 update container name if we get a rename event. closes gliderlabs#144 (gliderlabs#180) * update container name if we get a rename event. closes gliderlabs#144 * revert to using event.ID commit 113e3b7 Author: Selim Ekizoglu <sekizoglu@thoughtworks.com> Date: Thu Mar 24 17:57:49 2016 +0200 Exclude containers by label. Closes gliderlabs#159

reconnect log stream unless container is dead

118f322

rdbaron reviewed Jun 1, 2016
View reviewed changes

avoid duplicate pumps with mutex lock

13d1129

josegonzalez merged commit 97c8c67 into gliderlabs:master Jun 13, 2016

This was referenced Jul 11, 2016

WIP: Now using event timestamp to retrieve relevant logs. #209

Open

Is there a health check of any kind on logspout?? #207

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reconnect log stream unless container is dead #191

reconnect log stream unless container is dead #191

bobzoller commented May 19, 2016

johanndt commented May 31, 2016

johanndt commented May 31, 2016

rdbaron Jun 1, 2016

bobzoller Jun 8, 2016

josegonzalez Jun 8, 2016

bobzoller commented Jun 9, 2016

rdbaron commented Jun 10, 2016

rdbaron commented Jun 13, 2016

bobzoller commented Jun 13, 2016

josegonzalez commented Jun 13, 2016

josegonzalez commented Jun 13, 2016

mattatcha commented Jun 13, 2016

josegonzalez commented Jun 13, 2016

bobzoller commented Jun 13, 2016

reconnect log stream unless container is dead #191

reconnect log stream unless container is dead #191

Conversation

bobzoller commented May 19, 2016

johanndt commented May 31, 2016

johanndt commented May 31, 2016

rdbaron Jun 1, 2016

Choose a reason for hiding this comment

bobzoller Jun 8, 2016

Choose a reason for hiding this comment

josegonzalez Jun 8, 2016

Choose a reason for hiding this comment

bobzoller commented Jun 9, 2016

rdbaron commented Jun 10, 2016

rdbaron commented Jun 13, 2016

bobzoller commented Jun 13, 2016

josegonzalez commented Jun 13, 2016

josegonzalez commented Jun 13, 2016

mattatcha commented Jun 13, 2016

josegonzalez commented Jun 13, 2016

bobzoller commented Jun 13, 2016