novox/mesh-catalog
6
0
Fork 0
Code Issues Pull Requests 5 Actions Packages Projects Releases Wiki Activity

Merge pull request 'docker: log rotation is a preference, 100m × 10 by default, judged before daemon.json takes it (hq issue 452)' (#205) from fix/452-log-rotation-is-a-preference into main

Browse Source
This commit was merged in pull request #205.
This commit is contained in:
mesh-admin
2026-10-11 10:21:53 +00:00
parent 0557be174a 9a1a97cc43
commit e2d25b2248
5 changed files with 379 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/docker/README.md
+26 -3
View File
@@ -14,7 +14,9 @@ tools below are the module's own.
| `socket` | `docker.socket` running, enabled at boot | given back as found when the module goes (ADR 0118) |
| `prune-service`, `prune-timer` | `/etc/systemd/system/docker-prune.{service,timer}`, written whole | removed with the module |
| `prune` | `docker-prune.timer` running, enabled at boot; restarted when either file changes | stopped and disabled with the module (the mesh made the unit) |
| `daemon` | `live-restore` and the mesh's registry in `insecure-registries` written into `/etc/docker/daemon.json`, beside other keys | each key given back as found when the module goes; only the member this module added leaves the list |
| `state`, `log-rotation` | the module's own directory, and in it `log-rotation.conf`: the settings `log-max-size` and `log-max-file` as the step judges them (0600) | removed with the module |
| `log-rotation-check` | a step, `docker-tools check-log-rotation`, run as root when `log-rotation` changes; it refuses a bad value, and the rest of the module (`daemon` with it) is then skipped | removed with the module |
| `daemon` | `live-restore`, the mesh's registry in `insecure-registries`, `log-driver` and `log-opts` written into `/etc/docker/daemon.json`, beside other keys | each key given back as found when the module goes; only the member this module added leaves the list |
| `runtime` | `docker.service` running, enabled at boot; reloaded, never restarted, when `daemon` changes | given back as found (ADR 0118) |
The weekly prune takes **dangling images and build cache unused for a week, and nothing else**. It
@@ -64,8 +66,27 @@ once, for `daemon`. The address is the same one, so the runtime's trust does not
Still elsewhere:
- **Nobody** writes log rotation. One machine has `log-driver` and `log-opts` by hand; they are left
as they are until a size is chosen for every machine.
## Log rotation (novox/hq issue 452)
`daemon` writes `"log-driver": "json-file"` and `"log-opts": {"max-size": …, "max-file": …}` from two
preferences: `log-max-size` (default `100m`) and `log-max-file` (default `10`), about 1 GB of log per
container. The defaults are what the one machine with rotation set by hand already had, so that machine
does not change; the others gain rotation. Both settings are terminal-only (`"trusted": true`).
**A bad value never reaches `daemon.json`.** The controller pastes a setting into a file as text and judges
only that it is one line (novox/hq issue 346), and the runtime is the only reader of `daemon.json`: a bad
size would be found as a runtime that does not start, and a quote could add any key. So the same two values
are written into `log-rotation.conf` first, and the step `log-rotation-check` judges them before `daemon`
is applied: a size is a whole number from 1 with `k`, `m` or `g` after it, a count is 1 to 99. Refused, the
step fails, the host skips the rest of this module (ADR 0136), `daemon.json` keeps what it had, and the
apply's report names the setting.
**Nothing restarts.** The runtime reads `log-driver` and `log-opts` only when it starts. A change to
`daemon` reloads it, as before; the reload takes neither key, and stops no container. The new rotation
holds from the runtime's next start (the machine's next boot, for one), and then only for
containers created after it: a container keeps the log options it was created with until it is created
again. `docker_daemon_config` lists the keys the daemon has not taken yet.
## What it does not declare yet, and why
@@ -242,6 +263,8 @@ The tests run against a fake runner and cover:
- a printed secret found by name and never answered by value, in the scan and in `docker_logs`;
- size parsing;
- what the daemon has not yet taken;
- the log rotation: the defaults `daemon.json` takes, the step declared before `daemon`, a bad size or
count refused, a good one read, and the runtime never restarted for it;
- event filtering;
- volume ownership;
- that the tools served are exactly the manifest's `tools`.
modules/docker/cmd/docker-tools/logrotation.go
+80
View File
@@ -0,0 +1,80 @@
package main
// The container runtime's log rotation, checked before it reaches daemon.json (novox/hq issue 452).
//
// **Why a step and not the file alone.** daemon.json's log-driver and log-opts are filled by the controller
// from this module's settings log-max-size and log-max-file, pasted as text (novox/hq issue 346): the
// controller judges nothing but that a value is one line. The runtime is the only reader of daemon.json and
// reads log-opts only when it starts, so a bad value would be found at the runtime's next start, as a runtime
// that does not start, and a value with a quote in it could add any key to the file. So the module's
// manifest writes the same two values into log-rotation.conf first, and runs this as a step before daemon.json:
// a value refused here fails the step, and the host skips the rest of the module (novox/hq ADR 0136), so
// daemon.json keeps what it had, and the refusal is in the apply's report.
import (
"fmt"
"os"
"regexp"
"strings"
)
// logRotation is what log-rotation.conf says, both values as the runtime takes them: text.
type logRotation struct{ Size, Files string }
var (
// A size as the json-file driver reads it, with the unit required and in lower case, as decided on issue
// 452: a whole number from 1 with k, m or g, at most nine digits, so no value overflows the runtime's count.
logSize = regexp.MustCompile(`^[1-9][0-9]{0,8}[kmg]$`)
// A count of files from 1 to 99, without a leading zero.
logFiles = regexp.MustCompile(`^[1-9][0-9]?$`)
)
// parseLogRotation reads `size=` and `files=` once each; a comment or an empty line is skipped, and any other
// line is refused, so a value that brought a line of its own is never read past.
func parseLogRotation(text string) (logRotation, error) {
var got logRotation
seen := map[string]bool{}
for _, line := range strings.Split(text, "\n") {
if line == "" || strings.HasPrefix(line, "#") {
continue
}
key, value, ok := strings.Cut(line, "=")
if !ok || (key != "size" && key != "files") {
return logRotation{}, fmt.Errorf("log-rotation.conf has a line that is not size= or files=: %q", line)
}
if seen[key] {
return logRotation{}, fmt.Errorf("log-rotation.conf says %s twice", key)
}
seen[key] = true
switch key {
case "size":
if !logSize.MatchString(value) {
return logRotation{}, fmt.Errorf("log-max-size %q is not a size: a whole number from 1 with k, m or g "+
"after it, such as 100m; daemon.json was left as it is", value)
}
got.Size = value
case "files":
if !logFiles.MatchString(value) {
return logRotation{}, fmt.Errorf("log-max-file %q is not a count from 1 to 99; daemon.json was left "+
"as it is", value)
}
got.Files = value
}
}
for _, key := range []string{"size", "files"} {
if !seen[key] {
return logRotation{}, fmt.Errorf("log-rotation.conf does not say %s", key)
}
}
return got, nil
}
// checkLogRotationFile is the step: the file the manifest wrote, judged.
func checkLogRotationFile(path string) error {
raw, err := os.ReadFile(path)
if err != nil {
return err
}
_, err = parseLogRotation(string(raw))
return err
}
modules/docker/cmd/docker-tools/logrotation_test.go
+214
View File
@@ -0,0 +1,214 @@
package main
import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"reflect"
"regexp"
"strings"
"testing"
)
// The container runtime's log rotation is a preference (novox/hq issue 452).
type manifestForLogs struct {
Settings map[string]struct {
Kind string `json:"kind"`
Default any `json:"default"`
Why string `json:"why"`
} `json:"settings"`
Resources []map[string]any `json:"resources"`
}
func readManifestForLogs(t *testing.T) manifestForLogs {
t.Helper()
raw, err := os.ReadFile("../../module.json")
if err != nil {
t.Fatal(err)
}
var m manifestForLogs
if err := json.Unmarshal(raw, &m); err != nil {
t.Fatal(err)
}
return m
}
func resourceByID(m manifestForLogs, id string) (map[string]any, int) {
for i, r := range m.Resources {
if r["id"] == id {
return r, i
}
}
return nil, -1
}
// filled is a file's content as the controller fills it from the settings' defaults alone: each
// ${setting:<key>} replaced by the default as text, the way the controller writes a value into a file.
func filled(t *testing.T, m manifestForLogs, content string) string {
t.Helper()
ref := regexp.MustCompile(`\$\{setting:([a-z0-9][a-z0-9_.-]*)\}`)
var missing []string
out := ref.ReplaceAllStringFunc(content, func(s string) string {
key := ref.FindStringSubmatch(s)[1]
d, ok := m.Settings[key]
if !ok || d.Kind != "preference" {
missing = append(missing, key)
return s
}
switch v := d.Default.(type) {
case float64:
return fmt.Sprintf("%d", int64(v))
default:
return fmt.Sprint(v)
}
})
if len(missing) > 0 {
t.Fatalf("the content asks for %v, which the manifest gives no preference default", missing)
}
return strings.ReplaceAll(out, "${seat:mesh-artifact-store:reach}", "registry.example:5100")
}
func TestTheDefaultLogRotationIsTheOneTheHandSetMachineHas(t *testing.T) {
m := readManifestForLogs(t)
daemon, _ := resourceByID(m, "daemon")
if daemon == nil {
t.Fatal("no daemon resource")
}
var keys map[string]any
if err := json.Unmarshal([]byte(filled(t, m, daemon["content"].(string))), &keys); err != nil {
t.Fatalf("daemon.json's keys with the defaults are not JSON: %v", err)
}
if keys["log-driver"] != "json-file" {
t.Errorf("log-driver is %v, want json-file (the driver every machine runs today)", keys["log-driver"])
}
want := map[string]any{"max-size": "100m", "max-file": "10"}
if !reflect.DeepEqual(keys["log-opts"], want) {
t.Errorf("log-opts is %v, want %v", keys["log-opts"], want)
}
if keys["live-restore"] != true {
t.Errorf("live-restore is %v", keys["live-restore"])
}
// The values a person set are the terminal's only (novox/hq issue 346): the file says so.
if daemon["trusted"] != true {
t.Errorf("daemon asks for settings and says trusted %v, want true", daemon["trusted"])
}
}
func TestTheLogRotationIsCheckedBeforeDaemonJSONIsWritten(t *testing.T) {
m := readManifestForLogs(t)
conf, confAt := resourceByID(m, "log-rotation")
check, checkAt := resourceByID(m, "log-rotation-check")
_, daemonAt := resourceByID(m, "daemon")
if conf == nil || check == nil {
t.Fatal("no log-rotation file or log-rotation-check step")
}
// The host applies a module's resources in declared order, and a step that fails skips the rest
// of its module (novox/hq ADR 0136): declared before daemon, a refused value never reaches it.
if !(confAt < checkAt && checkAt < daemonAt) {
t.Errorf("order: log-rotation %d, log-rotation-check %d, daemon %d", confAt, checkAt, daemonAt)
}
if check["type"] != "process" || check["run-once"] != true || check["artifact"] != "tools" {
t.Errorf("the check is not a step of the tools bundle: %v", check)
}
if !reflect.DeepEqual(check["run"], []any{"./docker-tools", "check-log-rotation"}) {
t.Errorf("run %v", check["run"])
}
if !reflect.DeepEqual(check["restart-on"], []any{"log-rotation"}) {
t.Errorf("restart-on %v", check["restart-on"])
}
if _, asRoot := check["user"]; asRoot {
t.Errorf("the check reads a 0600 file and must run as root")
}
if conf["trusted"] != true {
t.Errorf("log-rotation says trusted %v, want true", conf["trusted"])
}
// The step reads the very values daemon.json takes: the same keys, filled the same way.
values, err := parseLogRotation(filled(t, m, conf["content"].(string)))
if err != nil {
t.Fatalf("the defaults are refused: %v", err)
}
if values != (logRotation{Size: "100m", Files: "10"}) {
t.Errorf("defaults read as %+v", values)
}
daemon, _ := resourceByID(m, "daemon")
for _, key := range []string{"${setting:log-max-size}", "${setting:log-max-file}"} {
if !strings.Contains(daemon["content"].(string), key) || !strings.Contains(conf["content"].(string), key) {
t.Errorf("%s is not in both daemon and log-rotation", key)
}
}
}
func TestTheRuntimeIsNeverRestartedForTheLogRotation(t *testing.T) {
m := readManifestForLogs(t)
runtime, _ := resourceByID(m, "runtime")
if _, restarts := runtime["restart-on"]; restarts {
t.Errorf("docker.service restarts on %v: a restart stops every container", runtime["restart-on"])
}
if !reflect.DeepEqual(runtime["reload-on"], []any{"daemon"}) {
t.Errorf("reload-on %v", runtime["reload-on"])
}
}
func TestABadLogRotationIsRefused(t *testing.T) {
good := map[string]string{"size": "100m", "files": "10"}
conf := func(over map[string]string) string {
lines := []string{"# a comment"}
for _, k := range []string{"size", "files"} {
v := good[k]
if o, ok := over[k]; ok {
v = o
}
lines = append(lines, k+"="+v)
}
return strings.Join(lines, "\n") + "\n"
}
for _, bad := range []map[string]string{
{"size": "100"}, {"size": "10x"}, {"size": "0m"}, {"size": "-1"}, {"size": ""}, {"size": "m"},
{"size": "1.5m"}, {"size": "10M"}, {"size": "100m\""}, {"size": `100m", "data-root": "/tmp`},
{"size": "100m\nfiles=3"}, {"size": " 100m"}, {"size": "99999999999m"},
{"files": "0"}, {"files": "100"}, {"files": "-1"}, {"files": ""}, {"files": "01"}, {"files": "1\""},
{"files": "1\nfiles=2"}, {"files": "3.0"}, {"files": "ten"},
} {
if v, err := parseLogRotation(conf(bad)); err == nil {
t.Errorf("%q accepted as %+v", bad, v)
}
}
for _, broken := range []string{"size=100m\n", "files=10\n", "size=100m\nfiles=10\nfiles=3\n",
"size=100m\nfiles=10\nmode=x\n", "size=100m\nfiles\n"} {
if v, err := parseLogRotation(broken); err == nil {
t.Errorf("%q accepted as %+v", broken, v)
}
}
}
func TestAGoodLogRotationIsAccepted(t *testing.T) {
for _, c := range []struct{ size, files string }{{"100m", "10"}, {"512k", "1"}, {"2g", "99"}, {"50m", "3"}} {
got, err := parseLogRotation("# comment\nsize=" + c.size + "\nfiles=" + c.files + "\n")
if err != nil {
t.Errorf("%v refused: %v", c, err)
continue
}
if got != (logRotation{Size: c.size, Files: c.files}) {
t.Errorf("%v read as %+v", c, got)
}
}
// And from the file the step is handed.
path := filepath.Join(t.TempDir(), "log-rotation.conf")
if err := os.WriteFile(path, []byte("size=50m\nfiles=3\n"), 0o600); err != nil {
t.Fatal(err)
}
if err := checkLogRotationFile(path); err != nil {
t.Errorf("a good file refused: %v", err)
}
if err := os.WriteFile(path, []byte("size=50\nfiles=3\n"), 0o600); err != nil {
t.Fatal(err)
}
if err := checkLogRotationFile(path); err == nil || !strings.Contains(err.Error(), "log-max-size") {
t.Errorf("a bad size was not refused by its setting's name: %v", err)
}
if err := checkLogRotationFile(filepath.Join(t.TempDir(), "absent")); err == nil {
t.Error("an absent file was accepted")
}
}
modules/docker/cmd/docker-tools/main.go
+14
View File
@@ -16,6 +16,20 @@ import (
)
func main() {
// Started as `docker-tools check-log-rotation` it is the module's step that judges the log rotation before
// daemon.json takes it (logrotation.go, novox/hq issue 452): run as root by the host, reading the file
// DOCKER_LOG_ROTATION names. With no argument, the runtime's tools bundle.
if len(os.Args) > 1 {
if os.Args[1] != "check-log-rotation" || len(os.Args) != 2 {
fmt.Fprintln(os.Stderr, "usage: docker-tools [check-log-rotation]")
os.Exit(2)
}
if err := checkLogRotationFile(os.Getenv("DOCKER_LOG_ROTATION")); err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
return
}
// An empty name serves as the module the runtime names (MESH_SERVED_MODULE): docker.
if err := stdio.Serve("", tools(NewClient())); err != nil {
fmt.Fprintln(os.Stderr, err)
modules/docker/module.json
+45 -1
View File
@@ -44,6 +44,18 @@
"docker exec cat /etc/resolv.conf"
]
},
"settings": {
"log-max-size": {
"kind": "preference",
"default": "100m",
"why": "each container's log is cut at this size and the runtime keeps log-max-file of them, so 100m and 10 hold about 1 GB per container: what the one machine with rotation set by hand has (novox/hq issue 452). A whole number from 1 with k, m or g after it, refused before daemon.json is written. The runtime reads it only at its next start, which the mesh never forces, and a container created before that keeps the options it was created with until it is created again"
},
"log-max-file": {
"kind": "preference",
"default": 10,
"why": "how many of a container's log files of log-max-size the runtime keeps, the oldest dropped first; a count from 1 to 99, refused before daemon.json is written, taken at the runtime's next start and by containers created after it, like log-max-size (novox/hq issue 452)"
}
},
"resources": [
{
"id": "package",
@@ -62,13 +74,45 @@
"state": "running",
"boot": "enabled"
},
{
"id": "state",
"type": "directory",
"mode": "0700",
"place": "."
},
{
"id": "log-rotation",
"type": "file",
"path": "${dir:state}/log-rotation.conf",
"mode": "0600",
"trusted": true,
"content": "# The log rotation daemon.json takes, written by the mesh from this machine's settings log-max-size and\n# log-max-file, and judged by the step log-rotation-check before daemon.json is written (novox/hq issue 452).\n# Replaced on every push; change the settings, never this file.\nsize=${setting:log-max-size}\nfiles=${setting:log-max-file}\n"
},
{
"id": "log-rotation-check",
"type": "process",
"name": "docker-log-rotation-check",
"artifact": "tools",
"run": [
"./docker-tools",
"check-log-rotation"
],
"run-once": true,
"env": {
"DOCKER_LOG_ROTATION": "${dir:state}/log-rotation.conf"
},
"restart-on": [
"log-rotation"
]
},
{
"id": "daemon",
"type": "file",
"path": "/etc/docker/daemon.json",
"mode": "0644",
"into": "json",
"content": "{\"live-restore\": true, \"insecure-registries\": [\"${seat:mesh-artifact-store:reach}\"]}\n"
"trusted": true,
"content": "{\"live-restore\": true, \"insecure-registries\": [\"${seat:mesh-artifact-store:reach}\"], \"log-driver\": \"json-file\", \"log-opts\": {\"max-size\": \"${setting:log-max-size}\", \"max-file\": \"${setting:log-max-file}\"}}\n"
},
{
"id": "runtime",