host, router: safe bin-overrides for merged-usr + controller re-resolve

host/libcontainer_backend.go

@@ -538,7 +538,7 @@ func (l *LibcontainerBackend) Run(job *host.Job, runConfig *RunConfig, rateLimit
 			return err
 		}
 	}
-	rootMount, diffDir, err := l.rootOverlayMount(job)
+	rootMount, diffDir, mergedUsr, err := l.rootOverlayMount(job)
 	if err != nil {
 		log.Error("error setting up rootfs", "err", err)
 		return err
@@ -699,8 +699,17 @@ func (l *LibcontainerBackend) Run(job *host.Job, runConfig *RunConfig, rateLimit
 	binOverridesDir := "/etc/flynn/bin-overrides"
 	if entries, err := os.ReadDir(binOverridesDir); err == nil {
 		containerBinDir := filepath.Join(diffDir, "bin")
+		if mergedUsr {
+			// Debian merged-usr images ship /bin as a symlink to /usr/bin.
+			// Materializing a real /bin directory in the overlay upperdir
+			// would shadow that symlink and hide /usr/bin from every process
+			// spawned in the container (e.g. "fork/exec /runner/init: no such
+			// file or directory"), crash-looping the app. Inject overrides
+			// into usr/bin instead so they're still reached via the symlink.
+			containerBinDir = filepath.Join(diffDir, "usr", "bin")
+		}
 		if err := os.MkdirAll(containerBinDir, 0755); err != nil {
-			log.Error("error creating container /bin in overlay upperdir", "err", err)
+			log.Error("error creating container bin dir in overlay upperdir", "err", err)
 			return err
 		}
 		for _, entry := range entries {
@@ -905,24 +914,24 @@ func (l *LibcontainerBackend) Run(job *host.Job, runConfig *RunConfig, rateLimit
 	return nil
 }
 
-func (l *LibcontainerBackend) rootOverlayMount(job *host.Job) (*configs.Mount, string, error) {
+func (l *LibcontainerBackend) rootOverlayMount(job *host.Job) (*configs.Mount, string, bool, error) {
 	log := l.Logger.New("fn", "rootOverlayMount", "job.id", job.ID)
 	layers := make([]string, 0, len(job.Mountspecs)+1)
 	for _, spec := range job.Mountspecs {
 		if spec.Type != host.MountspecTypeSquashfs {
-			return nil, "", fmt.Errorf("unknown mountspec type: %q", spec.Type)
+			return nil, "", false, fmt.Errorf("unknown mountspec type: %q", spec.Type)
 		}
 		log.Info("mounting squashfs layer", "id", spec.ID)
 		path, err := l.mountSquashfs(spec)
 		if err != nil {
-			return nil, "", err
+			return nil, "", false, err
 		}
 		layers = append(layers, path)
 	}
 	log.Info("mounting ext2 layer")
 	tmpfs, err := l.mountTmpfs(job)
 	if err != nil {
-		return nil, "", err
+		return nil, "", false, err
 	}
 	layers = append(layers, tmpfs)
 	dirs := make([]string, len(layers))
@@ -935,15 +944,31 @@ func (l *LibcontainerBackend) rootOverlayMount(job *host.Job) (*configs.Mount, s
 	workDir := filepath.Join(tmpfs, "overlay-workdir")
 	for _, dir := range []string{upperDir, workDir} {
 		if err := os.Mkdir(dir, 0755); err != nil {
-			return nil, "", err
+			return nil, "", false, err
 		}
 	}
+	mergedUsr := len(layers) > 0 && layerMergedUsr(layers[0])
 	return &configs.Mount{
 		Source:      "overlay",
 		Destination: "/",
 		Device:      "overlay",
 		Data:        fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", strings.Join(dirs[1:], ":"), upperDir, workDir),
-	}, upperDir, nil
+	}, upperDir, mergedUsr, nil
+}
+
+// layerMergedUsr reports whether the given (base image) layer ships /bin
+// as a symlink to /usr/bin, i.e. it is a Debian merged-usr image. This
+// matters when injecting files into the overlay upperdir: materializing a
+// real /bin directory there would shadow the symlink and hide /usr/bin
+// from every process in the container.
+func layerMergedUsr(layerPath string) bool {
+	binPath := filepath.Join(layerPath, "bin")
+	fi, err := os.Lstat(binPath)
+	if err != nil || fi.Mode()&os.ModeSymlink == 0 {
+		return false
+	}
+	target, err := filepath.EvalSymlinks(binPath)
+	return err == nil && target == filepath.Join(layerPath, "usr", "bin")
 }
 
 func (l *LibcontainerBackend) mountSquashfs(m *host.Mountspec) (string, error) {

host/libcontainer_backend_test.go

@@ -0,0 +1,53 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestLayerMergedUsr(t *testing.T) {
+	dir := t.TempDir()
+
+	t.Run("merged-usr image", func(t *testing.T) {
+		usrBin := filepath.Join(dir, "usr", "bin")
+		if err := os.MkdirAll(usrBin, 0755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Symlink("usr/bin", filepath.Join(dir, "bin")); err != nil {
+			t.Fatal(err)
+		}
+		if !layerMergedUsr(dir) {
+			t.Fatal("expected merged-usr detection to succeed for /bin -> usr/bin symlink")
+		}
+	})
+
+	t.Run("legacy image with real bin dir", func(t *testing.T) {
+		binDir := filepath.Join(dir, "legacy", "bin")
+		if err := os.MkdirAll(binDir, 0755); err != nil {
+			t.Fatal(err)
+		}
+		if layerMergedUsr(filepath.Join(dir, "legacy")) {
+			t.Fatal("expected plain bin directory not to be treated as merged-usr")
+		}
+	})
+
+	t.Run("bin symlink to something else", func(t *testing.T) {
+		base := filepath.Join(dir, "other")
+		if err := os.MkdirAll(base, 0755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Symlink("sbin", filepath.Join(base, "bin")); err != nil {
+			t.Fatal(err)
+		}
+		if layerMergedUsr(base) {
+			t.Fatal("expected unknown symlink target not to be treated as merged-usr")
+		}
+	})
+
+	t.Run("no bin at all", func(t *testing.T) {
+		if layerMergedUsr(filepath.Join(dir, "empty")) {
+			t.Fatal("expected missing bin not to be treated as merged-usr")
+		}
+	})
+}

router/store.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"encoding/json"
+	"errors"
 
 	controller "github.com/flynn/flynn/controller/client"
 	ct "github.com/flynn/flynn/controller/types"
@@ -16,34 +17,73 @@ type Store interface {
 }
 
 func NewControllerStore() (*ControllerStore, error) {
-	instances, err := discoverd.NewService("controller").Instances()
-	if err != nil {
-		return nil, err
-	}
-	inst := instances[0]
-	client, err := controller.NewClient("http://"+inst.Addr, inst.Meta["AUTH_KEY"])
-	if err != nil {
-		return nil, err
-	}
-	return &ControllerStore{client}, nil
+	return &ControllerStore{service: discoverd.NewService("controller")}, nil
 }
 
 type ControllerStore struct {
-	client controller.Client
+	service discoverd.Service
+}
+
+// call resolves the live controller instances for each invocation and
+// invokes fn with a client for the first instance that accepts the
+// request. Unlike pinning to the first instance at startup, this allows
+// the router to keep syncing when the previously used controller is
+// replaced or dies (the controllers and the router are restarted
+// independently, so a restarted router must not assume the original
+// instance is still in discoverd).
+func (c *ControllerStore) call(fn func(controller.Client) error) error {
+	instances, err := c.service.Instances()
+	if err != nil {
+		return err
+	}
+	if len(instances) == 0 {
+		return errors.New("router: no controller instances available")
+	}
+	var lastErr error
+	for _, inst := range instances {
+		client, err := controller.NewClient("http://"+inst.Addr, inst.Meta["AUTH_KEY"])
+		if err != nil {
+			lastErr = err
+			continue
+		}
+		if err := fn(client); err != nil {
+			lastErr = err
+			continue
+		}
+		return nil
+	}
+	if lastErr == nil {
+		return errors.New("router: no reachable controller instances")
+	}
+	return lastErr
 }
 
 func (c *ControllerStore) List() ([]*router.Route, error) {
-	return c.client.RouteList()
+	var routes []*router.Route
+	err := c.call(func(client controller.Client) error {
+		var err error
+		routes, err = client.RouteList()
+		return err
+	})
+	return routes, err
 }
 
 func (c *ControllerStore) Watch(ch chan *router.Event) (stream.Stream, error) {
-	events := make(chan *ct.Event)
-	eventStream, err := c.client.StreamEvents(ct.StreamEventsOptions{
-		ObjectTypes: []ct.EventType{
-			ct.EventTypeRoute,
-			ct.EventTypeRouteDeletion,
-		},
-	}, events)
+	var (
+		events      chan *ct.Event
+		eventStream stream.Stream
+	)
+	err := c.call(func(client controller.Client) error {
+		events = make(chan *ct.Event)
+		var err error
+		eventStream, err = client.StreamEvents(ct.StreamEventsOptions{
+			ObjectTypes: []ct.EventType{
+				ct.EventTypeRoute,
+				ct.EventTypeRouteDeletion,
+			},
+		}, events)
+		return err
+	})
 	if err != nil {
 		return nil, err
 	}