Uptodate list of running docker containers stated in an exported golang variable

Issue

I am trying to use the Golang SDK of Docker in order to maintain a slice variable with currently running containers on the local Docker instance. This slice is exported from a package and I want to use it to feed a web page.

I am not really used to goroutines and channels and that’s why I am wondering if I have spotted a good solution for my problem.

I have a docker package as follows.

https://play.golang.org/p/eMmqkMezXZn

It has a Running variable containing the current state of running containers.

var Running []types.Container

I use a reload function to load the running containers in the Running variable.

// Reload the list of running containers
func reload() error {
    ...
    Running, err = cli.ContainerList(context.Background(), types.ContainerListOptions{
        All: false,
    })
    ...
}

And then I start a goroutine from the init function to listen to Docker events and trigger the reload function accordingly.

func init() {
    ...
    // Listen for docker events
    go listen()
    ...
}

// Listen for docker events
func listen() {
    filter := filters.NewArgs()
    filter.Add("type", "container")
    filter.Add("event", "start")
    filter.Add("event", "die")

    msg, errChan := cli.Events(context.Background(), types.EventsOptions{
        Filters: filter,
    })

    for {
        select {
        case err := <-errChan:
            panic(err)
        case <-msg:
            fmt.Println("reloading")
            reload()
        }
    }
}

My question is, is it proper to update a variable from inside a goroutine (in terms of sync)? Maybe there is a cleaner way to achieve what I am trying to build?

Update

My concern here is not really about caching. It is more about hiding the “complexity” of the process of listening and update from the Docker SDK. I wanted to provide something like an index to easily let the end user loop and display currently running containers.

I was aware of data-races problems in threaded programs but I did not realize I was as actually in a context of concurrence here (I never wrote concurrent programs in Go before).

I effectively need to re-think the solution to be more idiomatic. As far as I can see, I have two options here: either protecting the variable with a mutex or re-thinking the design to integrate channels.

What means the most to me is to hide or encapsulate the method of synchronization used so the package users need not concern of how the shared state is protected.

Would you have any recommendations?

Thanks a lot for your help,
Loric

Solution

No, it is not idiomatic Go to share the Running variable between two goroutines. You do this by sharing it between the routine that runs your main function, and the listen function which is started with go—which spawns another goroutine.

Why, is because it breaks with

Do not communicate by sharing memory; instead, share memory by
communicating. ¹

So the design of the API needs to change in order to be idiomatic; you need to remove the Running variable and replace it with what? It depends on what you are trying to achieve. If you are trying to cache the cli.ContainerList because you need to call it often, and it might be expensive, you should implement a cache which is invalidated on each cli.Events.

What is your motivation?

Answered By – Jonas G. Drange

Answer Checked By – Robin (GoLangFix Admin)

Leave a Reply

Your email address will not be published.