fix: improve executor portability and configuration defaults

compute/scheduler/util.go

@@ -1,16 +1,29 @@
 package scheduler
 
 import (
+	"errors"
 	"fmt"
 	"math"
 
 	"github.com/google/uuid"
 	"github.com/ohsu-comp-bio/funnel/config"
-	pscpu "github.com/shirou/gopsutil/cpu"
-	psdisk "github.com/shirou/gopsutil/disk"
-	psmem "github.com/shirou/gopsutil/mem"
+	pscpu "github.com/shirou/gopsutil/v4/cpu"
+	psdisk "github.com/shirou/gopsutil/v4/disk"
+	psmem "github.com/shirou/gopsutil/v4/mem"
 )
 
+type resourceProbe struct {
+	cpuInfo       func() ([]pscpu.InfoStat, error)
+	virtualMemory func() (*psmem.VirtualMemoryStat, error)
+	diskUsage     func(string) (*psdisk.UsageStat, error)
+}
+
+var hostResourceProbe = resourceProbe{
+	cpuInfo:       pscpu.Info,
+	virtualMemory: psmem.VirtualMemory,
+	diskUsage:     psdisk.Usage,
+}
+
 // GenNodeID returns a UUID string.
 func GenNodeID() string {
 	u, _ := uuid.NewV7()
@@ -24,42 +37,50 @@ func GenNodeID() string {
 // Upon error, detectResources will return the resources given by the config
 // with the error.
 func detectResources(conf *config.Node, workdir string) (*Resources, error) {
+	return detectResourcesWithProbe(conf, workdir, hostResourceProbe)
+}
+
+func detectResourcesWithProbe(conf *config.Node, workdir string, probe resourceProbe) (*Resources, error) {
 	res := &Resources{
 		Cpus:   conf.Resources.Cpus,
 		RamGb:  conf.Resources.RamGb,
 		DiskGb: conf.Resources.DiskGb,
 	}
 
-	cpuinfo, err := pscpu.Info()
-	if err != nil {
-		return res, fmt.Errorf("Error detecting cpu cores: %s", err)
-	}
-	vmeminfo, err := psmem.VirtualMemory()
-	if err != nil {
-		return res, fmt.Errorf("Error detecting memory: %s", err)
-	}
-	diskinfo, err := psdisk.Usage(workdir)
-	if err != nil {
-		return res, fmt.Errorf("Error detecting available disk: %s", err)
-	}
+	var detectionErrors []error
 
 	if conf.Resources.Cpus == 0 {
-		// TODO is cores the best metric? with hyperthreading,
-		//      runtime.NumCPU() and pscpu.Counts() return 8
-		//      on my 4-core mac laptop
-		for _, cpu := range cpuinfo {
-			res.Cpus += uint32(cpu.Cores)
+		cpuinfo, err := probe.cpuInfo()
+		if err != nil {
+			detectionErrors = append(detectionErrors, fmt.Errorf("detecting CPU cores: %w", err))
+		} else {
+			// TODO is cores the best metric? with hyperthreading,
+			//      runtime.NumCPU() and pscpu.Counts() return 8
+			//      on my 4-core mac laptop
+			for _, cpu := range cpuinfo {
+				res.Cpus += uint32(cpu.Cores)
+			}
 		}
 	}
 
 	gb := math.Pow(1000, 3)
 	if conf.Resources.RamGb == 0.0 {
-		res.RamGb = float64(vmeminfo.Total) / float64(gb)
+		vmeminfo, err := probe.virtualMemory()
+		if err != nil {
+			detectionErrors = append(detectionErrors, fmt.Errorf("detecting memory: %w", err))
+		} else {
+			res.RamGb = float64(vmeminfo.Total) / gb
+		}
 	}
 
 	if conf.Resources.DiskGb == 0.0 {
-		res.DiskGb = float64(diskinfo.Free) / float64(gb)
+		diskinfo, err := probe.diskUsage(workdir)
+		if err != nil {
+			detectionErrors = append(detectionErrors, fmt.Errorf("detecting available disk: %w", err))
+		} else {
+			res.DiskGb = float64(diskinfo.Free) / gb
+		}
 	}
 
-	return res, nil
+	return res, errors.Join(detectionErrors...)
 }

compute/scheduler/util_test.go

@@ -0,0 +1,89 @@
+package scheduler
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/ohsu-comp-bio/funnel/config"
+	pscpu "github.com/shirou/gopsutil/v4/cpu"
+	psdisk "github.com/shirou/gopsutil/v4/disk"
+	psmem "github.com/shirou/gopsutil/v4/mem"
+)
+
+func TestDetectResourcesSkipsConfiguredValues(t *testing.T) {
+	probe := resourceProbe{
+		cpuInfo: func() ([]pscpu.InfoStat, error) {
+			t.Fatal("CPU probe should not run for a configured value")
+			return nil, nil
+		},
+		virtualMemory: func() (*psmem.VirtualMemoryStat, error) {
+			t.Fatal("memory probe should not run for a configured value")
+			return nil, nil
+		},
+		diskUsage: func(string) (*psdisk.UsageStat, error) {
+			t.Fatal("disk probe should not run for a configured value")
+			return nil, nil
+		},
+	}
+	conf := &config.Node{Resources: &config.Resources{Cpus: 4, RamGb: 8, DiskGb: 12}}
+
+	got, err := detectResourcesWithProbe(conf, "/unused", probe)
+	if err != nil {
+		t.Fatal("unexpected detection error:", err)
+	}
+	if got.Cpus != 4 || got.RamGb != 8 || got.DiskGb != 12 {
+		t.Fatalf("configured resources changed: %+v", got)
+	}
+}
+
+func TestDetectResourcesContinuesAfterProbeFailure(t *testing.T) {
+	cpuErr := errors.New("CPU unavailable")
+	probe := resourceProbe{
+		cpuInfo: func() ([]pscpu.InfoStat, error) {
+			return nil, cpuErr
+		},
+		virtualMemory: func() (*psmem.VirtualMemoryStat, error) {
+			return &psmem.VirtualMemoryStat{Total: 2_000_000_000}, nil
+		},
+		diskUsage: func(workdir string) (*psdisk.UsageStat, error) {
+			if workdir != "/work" {
+				t.Fatalf("unexpected workdir %q", workdir)
+			}
+			return &psdisk.UsageStat{Free: 3_000_000_000}, nil
+		},
+	}
+	conf := &config.Node{Resources: &config.Resources{}}
+
+	got, err := detectResourcesWithProbe(conf, "/work", probe)
+	if !errors.Is(err, cpuErr) {
+		t.Fatalf("expected CPU error, got %v", err)
+	}
+	if got.Cpus != 0 || got.RamGb != 2 || got.DiskGb != 3 {
+		t.Fatalf("expected successful probes to be retained, got %+v", got)
+	}
+}
+
+func TestDetectResourcesJoinsProbeFailures(t *testing.T) {
+	memoryErr := errors.New("memory unavailable")
+	diskErr := errors.New("disk unavailable")
+	probe := resourceProbe{
+		cpuInfo: func() ([]pscpu.InfoStat, error) {
+			return []pscpu.InfoStat{{Cores: 10}}, nil
+		},
+		virtualMemory: func() (*psmem.VirtualMemoryStat, error) {
+			return nil, memoryErr
+		},
+		diskUsage: func(string) (*psdisk.UsageStat, error) {
+			return nil, diskErr
+		},
+	}
+	conf := &config.Node{Resources: &config.Resources{}}
+
+	got, err := detectResourcesWithProbe(conf, "/work", probe)
+	if !errors.Is(err, memoryErr) || !errors.Is(err, diskErr) {
+		t.Fatalf("expected joined memory and disk errors, got %v", err)
+	}
+	if got.Cpus != 10 {
+		t.Fatalf("expected successful CPU detection to be retained, got %+v", got)
+	}
+}

config/config_test.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -68,3 +69,35 @@ func TestEmbeddedDefaultConfigForbiddenPaths(t *testing.T) {
 		t.Fatalf("expected embedded forbidden paths %v, got %v", want, got)
 	}
 }
+
+func TestRPCClientCredentialParsing(t *testing.T) {
+	conf := EmptyConfig()
+	raw := []byte(`
+RPCClient:
+  Credential:
+    User: funnel
+    Password: abc123
+`)
+	if err := Parse(raw, conf); err != nil {
+		t.Fatal("parsing nested RPC credential:", err)
+	}
+	if got := conf.RPCClient.Credential; got.User != "funnel" || got.Password != "abc123" {
+		t.Fatalf("unexpected RPC credential: %+v", got)
+	}
+}
+
+func TestRPCClientLegacyCredentialFieldsRejected(t *testing.T) {
+	conf := EmptyConfig()
+	raw := []byte(`
+RPCClient:
+  User: funnel
+  Password: abc123
+`)
+	err := Parse(raw, conf)
+	if err == nil {
+		t.Fatal("expected legacy RPC credential fields to be rejected")
+	}
+	if !strings.Contains(err.Error(), `unknown field "User"`) {
+		t.Fatalf("expected unknown User field error, got %v", err)
+	}
+}

config/default-config.yaml

@@ -80,8 +80,9 @@ RPCClient:
   # Credentials for Basic authentication for the server APIs using a password.
   # If used, make sure to properly restrict access to the config file
   # (e.g. chmod 600 funnel.config.yml)
-  # User: funnel
-  # Password: abc123
+  # Credential:
+  #   User: funnel
+  #   Password: abc123
 
   # connection timeout.
   Timeout:
@@ -158,6 +159,7 @@ Worker:
 
     RunCommand: |
       run -i --read-only
+      {{if .NeedsTmpfs}}--tmpfs /tmp{{end}}
       {{if .RemoveContainer}}--rm{{end}}
       {{.GetEnvArgs}}
       {{range $k, $v := .Tags}}--label "{{$k}}={{$v}}" {{end}}

config/default.go

@@ -90,6 +90,8 @@ func DefaultConfig() *Config {
 			Container: &ContainerConfig{
 				DriverCommand: "docker",
 				RunCommand: "run -i --read-only " +
+					// Writable scratch space scoped to this executor container.
+					"{{if .NeedsTmpfs}}--tmpfs /tmp{{end}} " +
 					// Remove container after it exits
 					"{{if .RemoveContainer}}--rm{{end}} " +
 

docs/funnel-config-examples/default-config.yaml

@@ -80,8 +80,9 @@ RPCClient:
   # Credentials for Basic authentication for the server APIs using a password.
   # If used, make sure to properly restrict access to the config file
   # (e.g. chmod 600 funnel.config.yml)
-  # User: funnel
-  # Password: abc123
+  # Credential:
+  #   User: funnel
+  #   Password: abc123
 
   # connection timeout.
   Timeout:

go.mod

@@ -40,7 +40,6 @@ require (
 	github.com/prometheus/client_golang v1.23.2
 	github.com/prometheus/common v0.67.5
 	github.com/rs/xid v1.6.0
-	github.com/shirou/gopsutil v3.21.11+incompatible
 	github.com/sirupsen/logrus v1.9.4
 	github.com/spf13/cobra v1.10.2
 	github.com/spf13/pflag v1.0.10
@@ -74,6 +73,7 @@ require (
 	github.com/jackc/pgx/v5 v5.9.1
 	github.com/lestrrat-go/jwx/v2 v2.1.6
 	github.com/minio/minio-go/v7 v7.1.0
+	github.com/shirou/gopsutil/v4 v4.26.3
 	github.com/testcontainers/testcontainers-go v0.42.0
 	github.com/testcontainers/testcontainers-go/modules/postgres v0.42.0
 	go.mongodb.org/mongo-driver/v2 v2.5.1
@@ -109,7 +109,6 @@ require (
 	github.com/moby/sys/userns v0.1.0 // indirect
 	github.com/morikuni/aec v1.1.0 // indirect
 	github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect
-	github.com/shirou/gopsutil/v4 v4.26.3 // indirect
 	github.com/shopspring/decimal v1.4.0 // indirect
 	github.com/zeebo/xxh3 v1.1.0 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.43.0 // indirect

go.sum

@@ -596,8 +596,6 @@ github.com/segmentio/asm v1.2.1 h1:DTNbBqs57ioxAD4PrArqftgypG4/qNpXoJx8TVXxPR0=
 github.com/segmentio/asm v1.2.1/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr5aAcs=
 github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN3Uc8sB6B/s6Z4t2xvBgU1htSHuq8=
 github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4=
-github.com/shirou/gopsutil v3.21.11+incompatible h1:+1+c1VGhc88SSonWP6foOcLhvnKlUeu/erjjvaPEYiI=
-github.com/shirou/gopsutil v3.21.11+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA=
 github.com/shirou/gopsutil/v4 v4.26.3 h1:2ESdQt90yU3oXF/CdOlRCJxrP+Am1aBYubTMTfxJ1qc=
 github.com/shirou/gopsutil/v4 v4.26.3/go.mod h1:LZ6ewCSkBqUpvSOf+LsTGnRinC6iaNUNMGBtDkJBaLQ=
 github.com/shopspring/decimal v0.0.0-20180709203117-cd690d0c9e24/go.mod h1:M+9NzErvs504Cn4c5DxATwIqPbtswREoFCre64PpcG4=

website/content/docs/security/basic.md

@@ -40,10 +40,15 @@ so you will also need to configure the RPC client.
 
 ```yaml
 RPCClient:
-  User: funnel
-  Password: abc123
+  Credential:
+    User: funnel
+    Password: abc123
 ```
 
+Beginning with Funnel v0.11.8, `User` and `Password` must be nested under
+`RPCClient.Credential`. Configurations created for earlier releases need to be
+updated to use this structure.
+
 Make sure to properly protect the configuration file so that it's not readable
 by everyone:
 

website/static/funnel-config-examples/default-config.yaml

@@ -80,8 +80,9 @@ RPCClient:
   # Credentials for Basic authentication for the server APIs using a password.
   # If used, make sure to properly restrict access to the config file
   # (e.g. chmod 600 funnel.config.yml)
-  # User: funnel
-  # Password: abc123
+  # Credential:
+  #   User: funnel
+  #   Password: abc123
 
   # connection timeout.
   Timeout:

worker/docker.go

@@ -9,6 +9,7 @@ import (
 	"math"
 	"os"
 	"os/exec"
+	"path"
 	"strings"
 	"text/template"
 	"time"
@@ -46,6 +47,20 @@ func (docker DockerCommand) MemoryMB() int64 {
 	return int64(math.Round(docker.Resources.RamGb * 1024))
 }
 
+// NeedsTmpfs reports whether the container needs Funnel's executor-local /tmp
+// mount. A task-provided mount at /tmp or one of its ancestors takes precedence
+// so explicitly requested storage can persist between executors.
+func (docker DockerCommand) NeedsTmpfs() bool {
+	const tmpDir = "/tmp"
+	for _, volume := range docker.Volumes {
+		containerPath := path.Clean(volume.ContainerPath)
+		if containerPath == "/" || containerPath == tmpDir || strings.HasPrefix(tmpDir, containerPath+"/") {
+			return false
+		}
+	}
+	return true
+}
+
 type DockerVersion struct {
 	Client string
 	Server string