Corrected docker/swarm discovery from previous refactoring (#446)

Author: itzg

.devcontainer/devcontainer.json

@@ -0,0 +1,35 @@
+// For format details, see https://aka.ms/devcontainer.json. For config options, see the
+// README at: https://github.com/devcontainers/templates/tree/main/src/go
+{
+  "name": "Go",
+  // Or use a Dockerfile or Docker Compose file. More info: https://containers.dev/guide/dockerfile
+  "image": "golang:1.24-bookworm",
+  // Features to add to the dev container. More info: https://containers.dev/features.
+  "features": {
+    // For in-docker discovery testing
+    "ghcr.io/devcontainers/features/docker-outside-of-docker:1": {}
+  },
+  // Use 'forwardPorts' to make a list of ports inside the container available locally.
+  "forwardPorts": [
+    25565
+  ],
+
+  containerEnv: {
+    "GOROOT": "/usr/local/go"
+  },
+
+  // Configure tool-specific properties.
+  "customizations": {
+    "jetbrains": {
+      "backend": "IntelliJ",
+      "plugins": [
+        "org.jetbrains.plugins.go"
+      ]
+    },
+    "vscode": {
+      "extensions": [
+        "golang.go"
+      ]
+    }
+  }
+}

DEVELOPMENT.md

@@ -0,0 +1,8 @@
+## Developing Docker discovery on non-Linux
+
+This works best with the included devcontaner setup, which includes attaching the host's docker socket to the dev container at `/var/run/docker.sock`.
+
+On Windows, can create the devcontainer using:
+
+![image.png](docs/create-dev-container.png)
+

README.md

@@ -103,8 +103,6 @@ The [multi-architecture image published at Docker Hub](https://hub.docker.com/re
 The diagram below shows how this `docker-compose.yml` configures two Minecraft server services named `vanilla` and `forge`, which also become the internal network aliases. _Notice those services don't need their ports exposed since the internal networking allows for the inter-container access._
 
 ```yaml
-version: "3.8"
-
 services:
   vanilla:
     image: itzg/minecraft-server
@@ -141,20 +139,21 @@ To test out this example, add these two entries to my "hosts" file:
 
 ### Using Docker auto-discovery
 
-When running `mc-router` in a Docker environment you can pass the `--in-docker` or `--in-docker-swarm` 
-command-line argument and it will poll the Docker API periodically to find all the running
-containers/services for Minecraft instances. To enable discovery you have to set the `mc-router.host`
-label on the container. These are the labels scanned:
-
-- `mc-router.host`: Used to configure the hostname the Minecraft clients would use to 
-  connect to the server. The container/service endpoint will be used as the routed backend. You can 
-  use more than one hostname by splitting it with a comma.
-- `mc-router.port`: This value must be set to the port the Minecraft server is listening on.
-  The default value is 25565.
-- `mc-router.default`: Set this to a truthy value to make this server the default backend.
-  Please note that `mc-router.host` is still required to be set.
-- `mc-router.network`: Specify the network you are using for the router if multiple are 
-  present in the container/service. You can either use the network ID, it's full name or an alias.
+When running `mc-router` in a Docker environment you can pass the `--in-docker` or `--in-docker-swarm` command-line argument or set the environment variables `IN_DOCKER` or `IN_DOCKER_SWARM` to "true". With that, it will poll the Docker API periodically to find all the running containers/services for Minecraft instances. To enable discovery, you have to set the `mc-router.host` label on the container. 
+
+When using in Docker, make sure to volume mount the Docker socket into the container, such as
+
+```yaml
+    volumes:
+      - /var/run/docker.sock:/var/run/docker.sock:ro
+```
+
+These are the labels scanned:
+
+- `mc-router.host`: Used to configure the hostname the Minecraft clients would use to connect to the server. The container/service endpoint will be used as the routed backend. You can use more than one hostname by splitting it with a comma.
+- `mc-router.port`: This value must be set to the port the Minecraft server is listening on. The default value is 25565.
+- `mc-router.default`: Set this to a truthy value to make this server the default backend. Please note that `mc-router.host` is still required to be set.
+- `mc-router.network`: Specify the network you are using for the router if multiple are present in the container/service. You can either use the network ID, it's full name or an alias.
 
 #### Example Docker deployment
 

cmd/mc-router/main.go

@@ -24,6 +24,7 @@ func showVersion() {
 type CliConfig struct {
 	Version bool `usage:"Output version and exit"`
 	Debug   bool `usage:"Enable debug logs"`
+	Trace   bool `usage:"Enable trace logs"`
 
 	ServerConfig server.Config `flatten:"true"`
 }
@@ -40,7 +41,10 @@ func main() {
 		os.Exit(0)
 	}
 
-	if cliConfig.Debug {
+	if cliConfig.Trace {
+		logrus.SetLevel(logrus.TraceLevel)
+		logrus.Trace("Trace logs enabled")
+	} else if cliConfig.Debug {
 		logrus.SetLevel(logrus.DebugLevel)
 		logrus.Debug("Debug logs enabled")
 	}

docs/create-dev-container.png

@@ -0,0 +1,23 @@
+services:
+  router:
+    image: itzg/mc-router
+    environment:
+      IN_DOCKER: true
+    ports:
+      - "25565:25565"
+    volumes:
+      - /var/run/docker.sock:/var/run/docker.sock:ro
+  vanilla:
+    image: itzg/minecraft-server
+    environment:
+      EULA: "TRUE"
+    labels:
+      mc-router.host: "vanilla.example.com"
+  paper:
+    image: itzg/minecraft-server
+    environment:
+      EULA: "TRUE"
+      TYPE: PAPER
+    labels:
+      mc-router.host: "paper.example.com"
+

examples/docker-discovery/compose.yml

@@ -15,8 +15,7 @@ import (
 )
 
 type IDockerWatcher interface {
-	Start(socket string, timeoutSeconds int, refreshIntervalSeconds int, autoScaleUp bool, autoScaleDown bool) error
-	Stop()
+	Start(ctx context.Context, socket string, timeoutSeconds int, refreshIntervalSeconds int, autoScaleUp bool, autoScaleDown bool) error
 }
 
 const (
@@ -34,7 +33,6 @@ type dockerWatcherImpl struct {
 	autoScaleUp   bool
 	autoScaleDown bool
 	client        *client.Client
-	contextCancel context.CancelFunc
 }
 
 func (w *dockerWatcherImpl) makeWakerFunc(_ *routableContainer) ScalerFunc {
@@ -57,7 +55,7 @@ func (w *dockerWatcherImpl) makeSleeperFunc(_ *routableContainer) ScalerFunc {
 	}
 }
 
-func (w *dockerWatcherImpl) Start(socket string, timeoutSeconds int, refreshIntervalSeconds int, autoScaleUp bool, autoScaleDown bool) error {
+func (w *dockerWatcherImpl) Start(ctx context.Context, socket string, timeoutSeconds int, refreshIntervalSeconds int, autoScaleUp bool, autoScaleDown bool) error {
 	var err error
 
 	w.autoScaleUp = autoScaleUp
@@ -83,9 +81,7 @@ func (w *dockerWatcherImpl) Start(socket string, timeoutSeconds int, refreshInte
 	ticker := time.NewTicker(refreshInterval)
 	containerMap := map[string]*routableContainer{}
 
-	var ctx context.Context
-	ctx, w.contextCancel = context.WithCancel(context.Background())
-
+	logrus.Trace("Performing initial listing of Docker containers")
 	initialContainers, err := w.listContainers(ctx)
 	if err != nil {
 		return err
@@ -104,6 +100,7 @@ func (w *dockerWatcherImpl) Start(socket string, timeoutSeconds int, refreshInte
 		for {
 			select {
 			case <-ticker.C:
+				logrus.Trace("Listing Docker containers")
 				containers, err := w.listContainers(ctx)
 				if err != nil {
 					logrus.WithError(err).Error("Docker failed to list containers")
@@ -145,6 +142,7 @@ func (w *dockerWatcherImpl) Start(socket string, timeoutSeconds int, refreshInte
 				}
 
 			case <-ctx.Done():
+				logrus.Debug("Stopping Docker monitoring")
 				ticker.Stop()
 				return
 			}
@@ -303,12 +301,6 @@ func (w *dockerWatcherImpl) parseContainerData(container *dockertypes.Container)
 	return
 }
 
-func (w *dockerWatcherImpl) Stop() {
-	if w.contextCancel != nil {
-		w.contextCancel()
-	}
-}
-
 type routableContainer struct {
 	externalContainerName string
 	containerEndpoint     string

server/docker.go

@@ -26,7 +26,6 @@ type dockerSwarmWatcherImpl struct {
 	autoScaleUp   bool
 	autoScaleDown bool
 	client        *client.Client
-	contextCancel context.CancelFunc
 }
 
 func (w *dockerSwarmWatcherImpl) makeWakerFunc(_ *routableService) ScalerFunc {
@@ -49,7 +48,7 @@ func (w *dockerSwarmWatcherImpl) makeSleeperFunc(_ *routableService) ScalerFunc
 	}
 }
 
-func (w *dockerSwarmWatcherImpl) Start(socket string, timeoutSeconds int, refreshIntervalSeconds int, autoScaleUp bool, autoScaleDown bool) error {
+func (w *dockerSwarmWatcherImpl) Start(ctx context.Context, socket string, timeoutSeconds int, refreshIntervalSeconds int, autoScaleUp bool, autoScaleDown bool) error {
 	var err error
 
 	w.autoScaleUp = autoScaleUp
@@ -75,9 +74,7 @@ func (w *dockerSwarmWatcherImpl) Start(socket string, timeoutSeconds int, refres
 	ticker := time.NewTicker(refreshInterval)
 	serviceMap := map[string]*routableService{}
 
-	var ctx context.Context
-	ctx, w.contextCancel = context.WithCancel(context.Background())
-
+	logrus.Trace("Performing initial listing of Docker containers")
 	initialServices, err := w.listServices(ctx)
 	if err != nil {
 		return err
@@ -99,7 +96,7 @@ func (w *dockerSwarmWatcherImpl) Start(socket string, timeoutSeconds int, refres
 				services, err := w.listServices(ctx)
 				if err != nil {
 					logrus.WithError(err).Error("Docker failed to list services")
-					return
+					continue
 				}
 
 				visited := map[string]struct{}{}
@@ -332,9 +329,3 @@ func (w *dockerSwarmWatcherImpl) parseServiceData(service *swarm.Service, networ
 	ok = true
 	return
 }
-
-func (w *dockerSwarmWatcherImpl) Stop() {
-	if w.contextCancel != nil {
-		w.contextCancel()
-	}
-}

server/docker_swarm.go

@@ -142,21 +142,17 @@ func NewServer(ctx context.Context, config *Config) (*Server, error) {
 
 	// TODO convert to RouteFinder
 	if config.InDocker {
-		err = DockerWatcher.Start(config.DockerSocket, config.DockerTimeout, config.DockerRefreshInterval, config.AutoScale.Up, config.AutoScale.Down)
+		err = DockerWatcher.Start(ctx, config.DockerSocket, config.DockerTimeout, config.DockerRefreshInterval, config.AutoScale.Up, config.AutoScale.Down)
 		if err != nil {
 			return nil, fmt.Errorf("could not start docker integration: %w", err)
-		} else {
-			defer DockerWatcher.Stop()
 		}
 	}
 
 	// TODO convert to RouteFinder
 	if config.InDockerSwarm {
-		err = DockerSwarmWatcher.Start(config.DockerSocket, config.DockerTimeout, config.DockerRefreshInterval, config.AutoScale.Up, config.AutoScale.Down)
+		err = DockerSwarmWatcher.Start(ctx, config.DockerSocket, config.DockerTimeout, config.DockerRefreshInterval, config.AutoScale.Up, config.AutoScale.Down)
 		if err != nil {
 			return nil, fmt.Errorf("could not start docker swarm integration: %w", err)
-		} else {
-			defer DockerSwarmWatcher.Stop()
 		}
 	}