Return errors from Docker client.Events

[BlakeMesdag]

What does this PR do?

Logs Errors from the Docker client.Events channel and prevents hangs when the event stream will no longer be pushed into.

Motivation

Backends getting out of sync during intermittent connection issues to the Docker daemon, and not being able to force them to refresh.

More

• Added/updated tests

Additional Notes

All errors in the Docker client.Events method cause events to stop being processed, leaving the messages channel open (https://github.com/moby/moby/blob/master/client/events.go#L20-L71). Since events are used as a notification mechanism it's save to reconnect if we lose one or more events.

This area of the codebase is untested. You can test/verify behaviours with a PoC I wrote here:

package main

import (
	"context"
	"fmt"

	"github.com/docker/docker/api/types"
	"github.com/docker/docker/api/types/filters"
	"github.com/docker/docker/client"
)

func main() {
	var err error

	cli, err := client.NewEnvClient()
	if err != nil {
		panic(err)
	}

	f := filters.NewArgs()
	f.Add("type", "container")

	options := types.EventsOptions{
		Filters: f,
	}

	ctx := context.Background()
	// ctx, cancel := context.WithCancel(ctx)

	eventsc, errc := cli.Events(ctx, options)
	for {
		select {
		case event := <-eventsc:
			fmt.Println("Received event: ", event)
		case err := <-errc:
			if err == nil {
				continue
			}

			fmt.Println("Received error: ", err)
			return
		}
	}
}

[ldez]

@BlakeMesdag Thanks for the contribution 👍

Could you add the missing io import.

[BlakeMesdag]

Done, forgot to push that up after I saw the failure.

[timoreimann]

Disclaimer: I'm not familiar with the Docker provider.)

Is there a chance that the event channel gets closed (as opposed to receiving a terminating event)? If it does, then the former implementation naturally exited the loop while the new select approach doesn't do so anymore.

Just trying to make sure that we keep terminating when / if needed.

[BlakeMesdag]

There is no chance of the event channel being closed. The channel that is returned by the Events method on the Docker client returns receive-only channels and only ever closes the errors channel.

The errors channel is always pushed into before each of the return paths in the Events method, and always a single error. The only other way that method could be escaped is with a panic which would propagate up to the Traefik provider operation where it would be recovered and retried until success, or until 15 minutes had elapsed (the default exponential backoff limit).

[BlakeMesdag]

I linked it in the PR, but heres the Events method for reference: https://github.com/moby/moby/blob/master/client/events.go#L20-L71

[timoreimann]

Thanks for elaborating. 👍

[timoreimann]

Could this PR be the first to contribute some tests to the Docker provider?
If it turns out to be rather complicated, I'm also okay to postpone that. Presumably, with no tests available the Docker provider isn't very testable, but I haven't looked at it closely.

[timoreimann]

@ldez rebase to 1.5? seems like an important fix.

[BlakeMesdag]

Not that I wouldn't like to add tests, but there's a lot of overhead before I'd get to testing this new functionality.

There are a few there already for other portions of the provider, but there's a lot of tests to be added for this specific file before I'd be getting to this functionality.

[timoreimann]

LGTM. We can always add tests in subsequent PRs.

Thanks a lot for your contribution!

[mmatur]

LGTM

[ldez]

LGTM