From 423d27011b32d517af09e17a47c5b3b65b91b90a Mon Sep 17 00:00:00 2001 From: Robin Scher Date: Sat, 15 Aug 2026 17:09:10 -0700 Subject: [PATCH] fix(scheduler): match attr.worker.os.family macos against a darwin worker --- docs/preset-library.md | 4 +- docs/worker-capabilities.md | 8 ++ internal/scheduler/matcher.go | 37 ++++++++- internal/scheduler/matcher_test.go | 75 +++++++++++++++++++ internal/store/step.go | 3 +- .../sqi/ffmpeg-segment-transcode-bash.yaml | 3 +- test/integration/ffmpeg_presets_test.go | 18 +++-- 7 files changed, 133 insertions(+), 15 deletions(-) diff --git a/docs/preset-library.md b/docs/preset-library.md index f577c6e8..dfd6e9ed 100644 --- a/docs/preset-library.md +++ b/docs/preset-library.md @@ -193,9 +193,7 @@ of that: file. All three need Source Duration entered by hand, since nothing can measure it before submission. Pick one based on your farm: - `ffmpeg-segment-transcode-bash` — joins with a bash script; needs Linux - or macOS workers (macOS workers are not currently matchable in sqi, so - this effectively runs on Linux only today). Removes its slice files once - the join succeeds. + or macOS workers. Removes its slice files once the join succeeds. - `ffmpeg-segment-transcode-powershell` — joins with a PowerShell script; needs Windows workers. Removes its slice files once the join succeeds. - `ffmpeg-segment-transcode-expr` ("Portable") — needs no shell at all, so diff --git a/docs/worker-capabilities.md b/docs/worker-capabilities.md index 419674c7..39d36ef4 100644 --- a/docs/worker-capabilities.md +++ b/docs/worker-capabilities.md @@ -403,6 +403,14 @@ Any attribute name outside the well-known set (`attr.worker.os.family`, `attr.worker.tag.` namespace always resolves to `""` and can never match. +**`attr.worker.os.family` takes OpenJD's tokens, not Go's.** Its only accepted +values are `linux`, `windows` and `macos` — the template validator rejects +anything else — and a macOS worker satisfies `macos` even though it reports +its OS as `darwin` everywhere else in sqi (the REST API, the web UI, and +`sqi-worker capabilities` all show `darwin`, and so does the automatic `os` +capability tag). If you want the Go spelling, gate on `attr.worker.tag.os` +instead. + To gate a step on a worker tagged `maya=true` — auto-detected on any worker with a standard Maya install (see [Capability auto-detection](#capability-auto-detection-built-in-dcc-detectors) above), or diff --git a/internal/scheduler/matcher.go b/internal/scheduler/matcher.go index 5855c436..8563b954 100644 --- a/internal/scheduler/matcher.go +++ b/internal/scheduler/matcher.go @@ -25,7 +25,7 @@ package scheduler // // Attribute requirements (categorical, compared as strings): // -// attr.worker.os.family → strings.ToLower(worker.OS) +// attr.worker.os.family → osFamily(worker.OS) ("darwin" → "macos") // attr.worker.os.version → worker.OSVersion // attr.worker.computelocation → worker.ComputeLocation // attr.worker.tag. → worker.Tags[key] (empty if absent) @@ -233,7 +233,7 @@ func satisfiesAmount(req store.StepAmountRequirement, workerVal int) bool { func workerAttributeValue(worker store.Worker, name string) string { switch strings.ToLower(name) { case "attr.worker.os.family": - return strings.ToLower(worker.OS) + return osFamily(worker.OS) case "attr.worker.os.version": return worker.OSVersion case "attr.worker.computelocation": @@ -246,6 +246,39 @@ func workerAttributeValue(worker store.Worker, name string) string { } } +// osFamily translates a worker's self-reported OS into the token OpenJD's +// reserved attr.worker.os.family attribute is spelled with. +// +// The worker reports runtime.GOOS (internal/worker/capabilities, probe.OS), and +// GOOS agrees with the specification on "linux" and "windows" but not on macOS, +// where GOOS says "darwin" and the specification says "macos". The template +// validator enforces the specification's set — internal/openjd's +// reservedAttributeAllowed accepts only {linux, windows, macos} — so "macos" is +// the ONLY spelling a template can legally carry for a Mac, and comparing it +// against a raw "darwin" made every such requirement unsatisfiable: the job +// validated, was accepted, and its tasks then waited for a worker that could +// not exist. Found 2026-08-15; nothing had caught it because the matcher's +// tests exercised only linux and windows. +// +// The mapping is deliberately ONE-WAY. This attribute carries the +// specification's family, not the Go runtime's, so "darwin" must not match +// either — accepting both would leave the same two spellings disagreeing, just +// in the other direction. A template that genuinely wants the GOOS spelling has +// attr.worker.tag.os, which capabilities.Detect populates verbatim. +// +// Normalizing HERE, and not at registration, is also deliberate: store.Worker.OS +// is surfaced raw in the REST API, the web UI and `sqi-worker capabilities`, and +// is copied into the "os" capability tag. Rewriting it at the source would +// change all four, and would leave rows written by earlier workers still saying +// "darwin". Matching is the only place the specification's vocabulary applies. +func osFamily(workerOS string) string { + family := strings.ToLower(workerOS) + if family == "darwin" { + return "macos" + } + return family +} + // tagValueFold returns the value of the worker tag whose key matches key // case-insensitively, or "" if none. An exact match is preferred; otherwise keys // are compared with EqualFold. diff --git a/internal/scheduler/matcher_test.go b/internal/scheduler/matcher_test.go index 840e9b53..dead50e7 100644 --- a/internal/scheduler/matcher_test.go +++ b/internal/scheduler/matcher_test.go @@ -281,6 +281,81 @@ func TestEligible_AttrOSFamily_AnyOf_Match(t *testing.T) { } } +// A macOS worker self-reports runtime.GOOS, which is "darwin", but OpenJD's +// reserved attr.worker.os.family takes the token "macos" — and the template +// validator (internal/openjd, reservedAttributeAllowed) accepts ONLY +// {linux, windows, macos}, rejecting "darwin" outright. So "macos" is the sole +// spelling a template can legally use for a Mac, and it is the one that has to +// match here. Before this was fixed, no valid template could ever schedule on a +// Mac: the job was accepted and its tasks then waited forever. +func TestEligible_AttrOSFamily_DarwinSatisfiesMacos_AnyOf(t *testing.T) { + w := baseWorker() + w.OS = "darwin" + s := baseStep() + s.HostRequirements = &store.StepHostRequirements{ + Attributes: []store.StepAttributeRequirement{ + {Name: "attr.worker.os.family", AnyOf: []string{"macos", "windows"}}, + }, + } + if !scheduler.WorkerEligible(w, baseJob(), s, nil, nil) { + t.Error("a darwin worker should satisfy anyOf [macos, windows]") + } +} + +func TestEligible_AttrOSFamily_DarwinSatisfiesMacos_AllOf(t *testing.T) { + w := baseWorker() + w.OS = "Darwin" // case-insensitive, as every other value here is + s := baseStep() + s.HostRequirements = &store.StepHostRequirements{ + Attributes: []store.StepAttributeRequirement{ + {Name: "attr.worker.os.family", AllOf: []string{"macos"}}, + }, + } + if !scheduler.WorkerEligible(w, baseJob(), s, nil, nil) { + t.Error("a darwin worker should satisfy allOf [macos]") + } +} + +// The mapping is one-way on purpose: this attribute carries the SPEC's os +// family, not the Go runtime's. "darwin" is not a legal value for it — the +// validator rejects a template containing it — so a worker must not match it +// either, or the two halves disagree again in the opposite direction. A +// template that genuinely wants the Go spelling has attr.worker.tag.os, which +// capabilities.Detect populates with runtime.GOOS verbatim. +func TestEligible_AttrOSFamily_DarwinDoesNotSatisfyDarwin(t *testing.T) { + w := baseWorker() + w.OS = "darwin" + s := baseStep() + s.HostRequirements = &store.StepHostRequirements{ + Attributes: []store.StepAttributeRequirement{ + {Name: "attr.worker.os.family", AnyOf: []string{"darwin"}}, + }, + } + if scheduler.WorkerEligible(w, baseJob(), s, nil, nil) { + t.Error("attr.worker.os.family should carry the spec token, not runtime.GOOS") + } +} + +// Linux and Windows need no mapping — GOOS already spells them the spec's way. +// Pinned so a future normalization cannot quietly rewrite them too. +func TestEligible_AttrOSFamily_NonDarwinPassThrough(t *testing.T) { + for _, goos := range []string{"linux", "windows"} { + t.Run(goos, func(t *testing.T) { + w := baseWorker() + w.OS = goos + s := baseStep() + s.HostRequirements = &store.StepHostRequirements{ + Attributes: []store.StepAttributeRequirement{ + {Name: "attr.worker.os.family", AnyOf: []string{goos}}, + }, + } + if !scheduler.WorkerEligible(w, baseJob(), s, nil, nil) { + t.Errorf("a %s worker should satisfy anyOf [%s]", goos, goos) + } + }) + } +} + func TestEligible_AttrOSFamily_AnyOf_Mismatch(t *testing.T) { w := baseWorker() w.OS = "darwin" diff --git a/internal/store/step.go b/internal/store/step.go index 8b52a19f..bb07adf2 100644 --- a/internal/store/step.go +++ b/internal/store/step.go @@ -62,7 +62,8 @@ type Step struct { // Capability names follow the conventions established in the OpenJD spec and // documented in [matcher.go]: // -// - "attr.worker.os.family" → worker OS family ("linux", "windows", "darwin") +// - "attr.worker.os.family" → worker OS family ("linux", "windows", "macos"; +// a worker reporting GOOS "darwin" matches "macos" — see scheduler.osFamily) // - "attr.worker.os.version" → worker OS version string // - "attr.worker.computelocation" → worker compute location name // - "attr.worker.tag." → arbitrary worker tag value diff --git a/presets/sqi/ffmpeg-segment-transcode-bash.yaml b/presets/sqi/ffmpeg-segment-transcode-bash.yaml index 7a9409ee..a8a41df9 100644 --- a/presets/sqi/ffmpeg-segment-transcode-bash.yaml +++ b/presets/sqi/ffmpeg-segment-transcode-bash.yaml @@ -8,8 +8,7 @@ description: >- small silently drops the tail, and a value that is too large wastes a task transcoding an empty tail slice. The join step runs a bash script, so this variant needs Linux or macOS workers; use the Portable variant on Windows or - a mixed farm. macOS workers are not currently matchable in sqi, so this - effectively runs on Linux today. Slice files are written beside the output + a mixed farm. Slice files are written beside the output and removed once the join succeeds. Requires ffmpeg on PATH. A starting point - duplicate to customize for your pipeline. category: Transcoding diff --git a/test/integration/ffmpeg_presets_test.go b/test/integration/ffmpeg_presets_test.go index 350154b9..0973e8ac 100644 --- a/test/integration/ffmpeg_presets_test.go +++ b/test/integration/ffmpeg_presets_test.go @@ -432,15 +432,19 @@ func TestFFmpegPreset_PortableSegmentTranscodeJoins(t *testing.T) { runSegmentPreset(t, "ffmpeg-segment-transcode-expr", true) } -// TestFFmpegPreset_BashSegmentTranscodeJoins runs the bash-joined variant. +// TestFFmpegPreset_BashSegmentTranscodeJoins runs the bash-joined variant, +// whose template gates on attr.worker.os.family anyOf [linux, macos]. // -// The gate is linux only, though the template's host requirement also lists -// macos: per docs/preset-library.md a macOS worker is not currently matchable -// in sqi, so darwin would submit a job no worker can take and time out here -// rather than fail for a reason worth reporting. +// It runs on darwin as well as linux, and that is load-bearing rather than +// incidental: until scheduler.osFamily landed, a Mac worker reported GOOS +// "darwin" against a requirement that can only legally say "macos", so this +// preset validated, submitted, and then waited forever for a worker that could +// not exist. An earlier revision of this test skipped darwin FOR THAT REASON. +// Running here is the end-to-end proof that a Mac can now take the work — a +// regression would show up as this test timing out rather than failing fast. func TestFFmpegPreset_BashSegmentTranscodeJoins(t *testing.T) { - if runtime.GOOS != "linux" { - t.Skipf("ffmpeg-segment-transcode-bash requires a linux worker; GOOS=%s", runtime.GOOS) + if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.Skipf("ffmpeg-segment-transcode-bash requires a linux or macos worker; GOOS=%s", runtime.GOOS) } runSegmentPreset(t, "ffmpeg-segment-transcode-bash", false) }