Ilker ManapandClaude Opus 5
774e134ed1
Two halves of the same problem: nothing stopped a model being placed on a node that could not hold it, and nothing ever removed the weights of a model that had been deleted from the catalog. Placement now has a disk check, shaped exactly like the memory one it sits beside, with one addition: it is skipped on a node that already holds the weights. A node holding the file needs no room for it, and without that the node best placed to run a model — because it already has it — is the one most likely to be rejected, since holding the file is what used the space. Reclaiming is the dangerous half, so the whole design is about what it will not do. llmgw deletes only files it fetched itself, and that is a calculation rather than a heuristic: the cache name is a hash of the source, so the set of wanted files can be computed from the catalog exactly. Anything without that name was put there by an operator — scp, a shared mount, a copy by hand — and is none of this system's business however unreferenced it looks. A model that took a week to copy must not be removable by a command someone ran to tidy up. The node checks every path again before unlinking it, and that duplication is the point. The control plane knows the catalog; the node knows its disk. A control plane that is wrong, newer than the agent, or tampered with must not be able to talk a node into deleting something, so the agent refuses anything outside its own cache directory or without the cache name — including a path that tries to traverse out of it. Listing is the default and deleting takes --collect. The summary says "asked to remove", because each node decides, and one that refuses a path it cannot prove it fetched is this working rather than failing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
94 lines
3.6 KiB
Go
94 lines
3.6 KiB
Go
package placement
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
|
|
"gitea.mynodes.xyz/ilker/llmgw/internal/agent/discovery"
|
|
"gitea.mynodes.xyz/ilker/llmgw/internal/control/inventory"
|
|
)
|
|
|
|
// withDisk gives a node a local filesystem of the stated size.
|
|
func withDisk(n inventory.Node, freeGB, totalGB int64) inventory.Node {
|
|
n.Storage = []discovery.Storage{{
|
|
Path: "/var/lib/llmgw-models",
|
|
FreeBytes: freeGB << 30,
|
|
TotalBytes: totalGB << 30,
|
|
}}
|
|
return n
|
|
}
|
|
|
|
// weightsProfile is a model that would have to be fetched: it names a source
|
|
// and says how much room that source needs.
|
|
func weightsProfile(source string, diskGB int) Profile {
|
|
return Profile{Name: "cpu", Engine: "llamacpp", Source: source, MinDiskGB: diskGB}
|
|
}
|
|
|
|
func TestANodeWithoutRoomForTheWeightsIsNotACandidate(t *testing.T) {
|
|
// The failure this replaces: the node is picked, the agent starts
|
|
// downloading, and it dies partway through with a disk-full error that
|
|
// names neither the model nor the scheduler decision that caused it.
|
|
s := New(store(withDisk(node("full", "", 0), 2, 60)))
|
|
|
|
res := s.Place([]Profile{weightsProfile("https://models/x.gguf", 20)}, nil)
|
|
if res.Placement != nil {
|
|
t.Fatalf("placed on a node with 2 GB free for a 20 GB model")
|
|
}
|
|
if !strings.Contains(res.Explain(), "needs 20") {
|
|
t.Errorf("the reason does not say what was needed:\n%s", res.Explain())
|
|
}
|
|
}
|
|
|
|
func TestRoomIsNotAskedForTwiceOnANodeThatAlreadyHasTheWeights(t *testing.T) {
|
|
// A node holding the file needs no space to hold it again. Without this,
|
|
// the one node best placed to run a model — because it already has it — is
|
|
// the one most likely to be rejected, since holding the file is what used
|
|
// the space up.
|
|
const source = "/var/lib/llmgw-models/big.gguf"
|
|
n := withDisk(node("holder", "", 0), 2, 60)
|
|
n.Weights = []discovery.LocalWeights{{Path: source, SizeBytes: 40 << 30}}
|
|
s := New(store(n))
|
|
|
|
res := s.Place([]Profile{weightsProfile(source, 20)}, nil)
|
|
if res.Placement == nil {
|
|
t.Fatalf("a node already holding the weights was rejected for lack of room:\n%s", res.Explain())
|
|
}
|
|
}
|
|
|
|
func TestANodeThatReportsNoDiskIsStillACandidate(t *testing.T) {
|
|
// An agent too old to report storage, and a node with no model directories,
|
|
// both report none. Excluding them would mean a control plane upgrade
|
|
// quietly empties every node in a fleet that has not upgraded yet — a worse
|
|
// failure than the one this check prevents.
|
|
s := New(store(node("silent", "", 0)))
|
|
|
|
res := s.Place([]Profile{weightsProfile("https://models/x.gguf", 20)}, nil)
|
|
if res.Placement == nil {
|
|
t.Fatalf("a node that reported no storage was treated as a full one:\n%s", res.Explain())
|
|
}
|
|
}
|
|
|
|
func TestWithoutADeclaredSizeDiskIsNotConsidered(t *testing.T) {
|
|
// Every profile that existed before this field has no number, and must
|
|
// keep being placed exactly as it was.
|
|
s := New(store(withDisk(node("full", "", 0), 0, 60)))
|
|
|
|
res := s.Place([]Profile{{Name: "cpu", Engine: "llamacpp", Source: "https://models/x.gguf"}}, nil)
|
|
if res.Placement == nil {
|
|
t.Fatalf("a profile with no declared size was rejected on disk:\n%s", res.Explain())
|
|
}
|
|
}
|
|
|
|
func TestSpaceOnASharedFilerDoesNotCountAsRoom(t *testing.T) {
|
|
// The node cannot spend it: it belongs to everyone mounting the filer. A
|
|
// node whose only storage is shared therefore reports no local disk, which
|
|
// lands in the same place as reporting nothing — unknown, so not excluded.
|
|
n := node("filer-only", "", 0)
|
|
n.Storage = []discovery.Storage{{
|
|
Path: "/mnt/filer", FreeBytes: 500 << 30, TotalBytes: 900 << 30, Shared: true,
|
|
}}
|
|
if free, total := n.LocalDisk(); free != 0 || total != 0 {
|
|
t.Fatalf("shared space counted as local: %d/%d", free, total)
|
|
}
|
|
}
|