diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 6a5b233..96fde11 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -12,7 +12,31 @@ env: IMAGE_NAME: ${{ github.repository }} jobs: + test: + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: go vet + run: go vet ./... + + - name: go build + run: go build ./... + + - name: go test + run: go test ./... + build: + needs: test runs-on: ubuntu-latest permissions: contents: read @@ -22,9 +46,6 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Set up QEMU - uses: docker/setup-qemu-action@v3 - - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 @@ -48,7 +69,7 @@ jobs: type=sha,prefix= - name: Build and push - uses: docker/build-push-action@v5 + uses: docker/build-push-action@v6 with: context: . platforms: linux/amd64 diff --git a/.gitignore b/.gitignore index 2573a32..ad1bf5a 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,10 @@ .vscode/ *.swp *.swo +*.sketch + +# dex task state +.dex/ # OS .DS_Store diff --git a/Dockerfile b/Dockerfile index bf5ec0c..2906f75 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build stage -FROM golang:1.25-bookworm AS builder +FROM golang:1.25.12-bookworm AS builder RUN apt-get update && apt-get install -y gcc libsqlite3-dev && rm -rf /var/lib/apt/lists/* diff --git a/README.md b/README.md index e84df26..b47dad1 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,50 @@ Shows: - Temperature history graph with threshold lines - Active workload hints +## Safety + +While running, this controller puts the iDRAC into **manual fan mode** via raw +IPMI OEM commands and is the **sole thing standing between your hardware and +the BMC's fans idling**. It is designed so that as many failure modes as +possible fail toward "the BMC keeps cooling the box," not toward "the fans +stay wherever they were and the machine cooks": + +- **Abnormal exit restores auto mode.** SIGINT/SIGTERM, a fatal API server + error, and even a panic in the control loop all go through the same + shutdown path, which calls `RestoreAutoMode` before the process exits. A + config that exists but fails safety validation makes the process refuse to + start at all (see [Upgrading](#upgrading)) rather than run with unsafe + defaults — since manual mode is never enabled in that case, the BMC simply + keeps its own automatic control. +- **Sensor loss fails up, not down.** A failed temperature read is never + treated as 0°C (which would ramp fans down). The controller holds the last + fan speed and, after `sensor_failure_limit` consecutive failures, hands + cooling back to BMC auto mode. This is recoverable: once sensor reads + succeed again, manual control is reclaimed automatically. +- **Fan-write failure is a sticky fail-safe.** After `write_failure_limit` + consecutive failures to *write* a fan speed, control is handed back to BMC + auto mode and stays there until the process restarts — the controller does + not probe the write channel again mid-run, since that would flap the BMC + between manual and auto. +- **Restore is retried until confirmed.** If the `RestoreAutoMode` call itself + fails (e.g. the BMC is unreachable), the controller keeps retrying on every + control-loop tick until it succeeds; `restore_pending` in `/api/status` + reflects this. Nothing re-enables manual mode while a restore is + unconfirmed. +- **Critical temperatures bypass everything.** If `critical_cpu_temp` or + `critical_gpu_temp` is reached, fans jump straight to `max_speed`, + bypassing step ramping and any active manual override. + +**What this does *not* protect against:** a `SIGKILL` (`kill -9`) or a power +loss bypasses the shutdown path entirely and cannot trigger a restore — the +BMC's own thermal protections are the last line of defense in that case, same +as on any other server. This controller is a cooling *policy* on top of the +BMC, not a replacement for its built-in safety logic. + +Also note that the raw IPMI commands used here (`ipmitool raw 0x30 0x30 ...`) +are **Dell iDRAC-specific OEM commands** — they are not portable to other +vendors' BMCs. + ## API Security The API binds `0.0.0.0` by default (required for container/bridge networking), so @@ -124,10 +168,24 @@ Returns current system state including thresholds: "zone": "idle", "cpu_threshold": 65, "gpu_threshold": 60, - "idle_speed": 20 + "idle_speed": 20, + "failsafe_active": false, + "failsafe_reason": "none", + "restore_pending": false, + "last_write_failed": false } ``` +The fail-safe fields (see [Safety](#safety) above): + +- `failsafe_active` — `true` once cooling has been handed back to BMC auto + mode (sensor loss or repeated fan-write failures). +- `failsafe_reason` — `"none"`, `"sensor-loss"`, or `"write-failure"`. +- `restore_pending` — `true` if fail-safe is active but the hand-back to BMC + auto mode has not yet been confirmed (the BMC may still be in manual mode); + the controller keeps retrying until this clears. +- `last_write_failed` — `true` if the most recent fan-speed write failed. + ### POST /api/hint Register a workload hint for proactive cooling: @@ -180,6 +238,11 @@ Get temperature/fan history for graphing. See [config.example.yaml](config.example.yaml) for all options. +**Logging**: the controller logs to stderr only — there is no `logging.level` +or `logging.file` config. Under Docker, use `docker logs` and your Docker log +driver's rotation settings (e.g. `json-file` with `max-size`/`max-file`) to +capture and rotate logs. + ### Key Settings ```yaml @@ -189,6 +252,17 @@ fan_control: gpu_threshold: 60 # Increase fans when GPU exceeds this (°C) step_size: 10 # Fan increase per 5°C over threshold (%) cooldown_delay: 60 # Seconds below threshold before ramping down + # Emergency ramp trigger — required, must exceed the (effective) threshold + # above it or the service refuses to start. See Safety and Upgrading above. + critical_cpu_temp: 85 + critical_gpu_temp: 90 + # Fail-safe: consecutive failures before handing cooling back to BMC auto. + sensor_failure_limit: 3 + write_failure_limit: 3 + +storage: + path: "/var/lib/only-fan-controller/history.db" + retention_days: 30 # History readings older than this are pruned daily ``` ### Example: Quiet Home Server @@ -261,6 +335,37 @@ All config options can be overridden via environment variables: 2. Configure via the Unraid Docker UI 3. Or use Community Applications (search "Only Fan Controller") +## Upgrading + +**smart-fan-controller → only-fan-controller rename.** The project (and the +Docker image) were renamed from `smart-fan-controller` to +`only-fan-controller`. If you have an existing deployment: + +- The container name changed from `smart-fan-controller` to + `only-fan-controller` — update any `docker stop`/`docker logs`/etc. scripts + that reference the old name. +- The GHCR image path changed accordingly (`ghcr.io//only-fan-controller` + instead of `.../smart-fan-controller`); update your `docker-compose.yml` / + `docker run` image reference. +- `scripts/hint-client.sh` now reads `FAN_URL` instead of `SMART_FAN_URL` for + the controller base URL, but still honors `SMART_FAN_URL` as a fallback if + set — no forced script changes. + +**Config validation now refuses to start on unsafe configs.** If your config +sets `cpu_threshold` (or `gpu_threshold`) at or above the corresponding +critical temperature's default — most notably `cpu_threshold >= 85` without +also setting `critical_cpu_temp` explicitly above it — the service now exits +at startup with a clear error instead of silently falling back to default +thresholds. Set `critical_cpu_temp`/`critical_gpu_temp` explicitly, above your +configured thresholds, to fix this. + +**API token auth is a config-only addition.** Existing deployments keep +working unchanged; see [API Security](#api-security) above. + +**History retention now defaults to 30 days.** The `readings` table is +pruned automatically (see `storage.retention_days` in +[config.example.yaml](config.example.yaml)); previously it grew unbounded. + ## Requirements - Dell PowerEdge server with iDRAC (tested on R730) diff --git a/SPEC.md b/SPEC.md index e4858aa..8ddbed6 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1,5 +1,34 @@ # iDRAC Smart Fan Controller +> **This is a historical design document, not current documentation.** It +> describes the original plan and has been superseded by the actual code and +> [README.md](README.md), which describe what was actually built and is kept +> up to date. Where this document conflicts with the code, the code wins. +> See "What was actually built" below for the most significant drift; the +> rest of this document (decision-engine pseudocode, phase checklists, file +> layout, etc.) is left as originally written and should be read as design +> history, not a spec of current behavior. + +## What was actually built (vs. this document) + +- **API routes are `/api/...`, and there is no `PUT /config`.** e.g. + `GET /api/status`, `POST /api/hint`, `POST /api/override`, not the bare + `/status`, `/hint`, etc. shown below. Configuration is read-only via + `GET /api/config`; there is no endpoint to update it at runtime. +- **Zone-based fan curves were never the control mechanism.** Fan control is + simple threshold + hysteresis (`cpu_threshold`/`gpu_threshold`, + `cooldown_delay`), not the zone lookup described in "Temperature Zones & + Fan Curves" below. `zones` still exist in config, but only for dashboard + display. The emergency ramp added later (`critical_cpu_temp`/ + `critical_gpu_temp`, see [README.md](README.md#safety)) is a separate, + explicitly-validated trigger — it is not part of the zone system either. +- **Trend calculation is telemetry only.** `cpu_trend`/`gpu_trend` are + reported in `/api/status` for visibility, but they do not feed back into + the fan-speed decision the way the "Decision Engine Logic" pseudocode below + implies. +- **The dashboard is embedded static HTML served on the same port as the + API**, not a separate Vue/Svelte SPA on port 8087. See `internal/api/static/`. + ## Overview An intelligent fan controller for Dell PowerEdge servers that goes beyond simple threshold-based control. Monitors CPU and GPU temperatures, accepts hints from external applications about upcoming workloads, and makes predictive decisions about fan speeds to maintain optimal cooling while minimizing noise. diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 2f156bc..59ccf80 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -11,6 +11,7 @@ import ( "strings" "sync" "syscall" + "time" "github.com/sethpjohnson/only-fan-controller/internal/api" "github.com/sethpjohnson/only-fan-controller/internal/config" @@ -128,6 +129,39 @@ func restoreOnce(restore func() error) func() error { } } +// cleanupShutdownTimeout bounds how long shutdown waits for the history +// cleanup goroutine to stop. Thermal safety (restore()) always runs before +// this wait starts, so a hung Cleanup query must not stall process exit. +const cleanupShutdownTimeout = 5 * time.Second + +// runHistoryCleanup prunes readings older than retention on a fixed daily +// interval until stopCh is closed. The initial cleanup (so a fresh start +// doesn't wait a full day before its first prune) is run by the caller before +// this goroutine is started; this loop only handles the recurring runs. +func runHistoryCleanup(store *storage.Store, retention time.Duration, retentionDays int, stopCh <-chan struct{}, done chan<- struct{}) { + defer close(done) + ticker := time.NewTicker(24 * time.Hour) + defer ticker.Stop() + for { + select { + case <-ticker.C: + logCleanupResult(store, retention, retentionDays) + case <-stopCh: + return + } + } +} + +// logCleanupResult runs a single history cleanup pass and logs the outcome. +func logCleanupResult(store *storage.Store, retention time.Duration, retentionDays int) { + deleted, err := store.Cleanup(retention) + if err != nil { + log.Printf("History cleanup failed: %v", err) + return + } + log.Printf("History cleanup: removed %d old reading(s) (retention %d days)", deleted, retentionDays) +} + // resolveConfig loads the config, distinguishing a genuinely absent file (safe // to fall back to defaults + env overrides) from a present-but-invalid file // (parse or safety-validation failure). For the latter we REFUSE to start rather @@ -189,6 +223,17 @@ func run() int { } defer store.Close() + // Prune history once at startup, then keep pruning daily for as long as the + // process runs. This is not on the safety-critical restore path, so its + // goroutine is stopped (via cleanupStop/cleanupDone) before the deferred + // store.Close() above runs, but is otherwise independent of the + // control-loop/API shutdown below. + retention := time.Duration(cfg.Storage.RetentionDays) * 24 * time.Hour + logCleanupResult(store, retention, cfg.Storage.RetentionDays) + cleanupStop := make(chan struct{}) + cleanupDone := make(chan struct{}) + go runHistoryCleanup(store, retention, cfg.Storage.RetentionDays, cleanupStop, cleanupDone) + // errCh carries any fatal error (API server failure, control-loop panic) // back to the shutdown path so cleanup + auto-mode restore always run. errCh := make(chan error, 1) @@ -249,6 +294,20 @@ func run() int { log.Printf("Warning: Failed to restore auto fan mode: %v", err) } + // Stop the history cleanup goroutine before the deferred store.Close() runs. + // Bounded: if a Cleanup query happens to be in flight, don't let it stall + // exit indefinitely (thermal safety already happened above, in restore()). + close(cleanupStop) + select { + case <-cleanupDone: + case <-time.After(cleanupShutdownTimeout): + // Abandoning it is safe: an in-flight Cleanup Exec racing the deferred + // store.Close() is benign — database/sql won't force-interrupt a + // checked-out connection, an interrupted DELETE rolls back via SQLite's + // journal, and the process exits immediately after this anyway. + log.Printf("Warning: history cleanup goroutine did not stop within %s; abandoning it", cleanupShutdownTimeout) + } + log.Println("Shutdown complete") return exitCode } diff --git a/config.example.yaml b/config.example.yaml index ba25dc1..d86dc29 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -25,6 +25,18 @@ fan_control: gpu_threshold: 60 # Bump fans when GPU exceeds this (°C) step_size: 10 # Fan speed increment per 5°C over threshold (%) cooldown_delay: 60 # Seconds below threshold before ramping down + # Critical temperatures force an emergency ramp straight to max_speed, + # bypassing step ramping and any manual override. Required, and must be + # greater than the (effective) threshold above it — the service refuses to + # start otherwise rather than silently falling back to defaults. If you + # raise cpu_threshold/gpu_threshold, raise these too. + critical_cpu_temp: 85 # Emergency ramp at/above this CPU temp (°C) + critical_gpu_temp: 90 # Emergency ramp at/above this GPU temp (°C) + # Fail-safe thresholds: after this many consecutive failures, cooling is + # handed back to the BMC's automatic fan control until reads/writes succeed + # again. See the README's Safety section for the full fail-safe design. + sensor_failure_limit: 3 # Consecutive sensor read failures before restoring auto mode + write_failure_limit: 3 # Consecutive fan-write failures before restoring auto mode api: # host 0.0.0.0 binds every interface — REQUIRED for container/bridge @@ -51,7 +63,10 @@ dashboard: storage: path: "/var/lib/only-fan-controller/history.db" + # How long history readings are kept before being pruned. Cleanup runs once + # at startup and then once a day. Must be > 0. + retention_days: 30 -logging: - level: "info" # debug, info, warn, error - file: "/var/log/only-fan-controller.log" +# Logs go to stderr; there is no file-logging config. Docker/systemd/your +# process supervisor is responsible for capturing and rotating them (e.g. +# `docker logs`, or the json-file/local log driver's rotation options). diff --git a/docker-compose.yml b/docker-compose.yml index 105fa03..cfae66c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,5 +1,3 @@ -version: '3.8' - services: only-fan-controller: build: . diff --git a/internal/config/config.go b/internal/config/config.go index 89dbe91..1b34ccc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -16,7 +16,6 @@ type Config struct { API APIConfig `yaml:"api"` Dashboard DashboardConfig `yaml:"dashboard"` Storage StorageConfig `yaml:"storage"` - Logging LoggingConfig `yaml:"logging"` } type IDRACConfig struct { @@ -83,11 +82,9 @@ type DashboardConfig struct { type StorageConfig struct { Path string `yaml:"path"` -} - -type LoggingConfig struct { - Level string `yaml:"level"` - File string `yaml:"file"` + // RetentionDays is how long history readings are kept before being pruned. + // Cleanup runs once at startup and then daily. Must be > 0. + RetentionDays int `yaml:"retention_days"` } // Default normal-ramp thresholds, applied when the corresponding config field is @@ -174,6 +171,9 @@ func (c *Config) Validate() error { return fmt.Errorf("zones must be monotonic non-decreasing; zone %q breaks ordering", cur.Name) } } + if c.Storage.RetentionDays <= 0 { + return fmt.Errorf("invalid storage.retention_days: %d (require > 0)", c.Storage.RetentionDays) + } return nil } @@ -225,11 +225,8 @@ func Default() *Config { Port: 8086, }, Storage: StorageConfig{ - Path: "/var/lib/only-fan-controller/history.db", - }, - Logging: LoggingConfig{ - Level: "info", - File: "/var/log/only-fan-controller.log", + Path: "/var/lib/only-fan-controller/history.db", + RetentionDays: 30, }, } } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b1ee7df..c6ad87b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,6 +1,51 @@ package config -import "testing" +import ( + "os" + "path/filepath" + "testing" +) + +// TestExampleConfigIsValid loads config.example.yaml verbatim (as a fresh +// install would) and confirms it passes Validate(), so the shipped example +// never silently bit-rots into a config the service refuses to start with. +func TestExampleConfigIsValid(t *testing.T) { + cfg, err := Load(filepath.Join("..", "..", "config.example.yaml")) + if err != nil { + t.Fatalf("config.example.yaml failed to load/validate: %v", err) + } + if cfg.Storage.RetentionDays <= 0 { + t.Fatalf("expected a positive retention_days, got %d", cfg.Storage.RetentionDays) + } +} + +// TestLegacyLoggingSectionIsIgnored confirms that removing the logging config +// struct doesn't break loading of older config files that still have a +// (now-unused) `logging:` section: yaml.v3 ignores unknown keys, so this +// should parse fine and fall back to defaults for everything else. +func TestLegacyLoggingSectionIsIgnored(t *testing.T) { + path := filepath.Join(t.TempDir(), "legacy-config.yaml") + content := `idrac: + host: "local" +fan_control: + critical_cpu_temp: 85 + critical_gpu_temp: 90 +logging: + level: "debug" + file: "/var/log/foo.log" +` + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write temp config: %v", err) + } + + cfg, err := Load(path) + if err != nil { + t.Fatalf("legacy config with a logging: section should still load, got: %v", err) + } + if cfg.Storage.RetentionDays != 30 { + t.Fatalf("expected default retention_days=30, got %d", cfg.Storage.RetentionDays) + } +} func TestValidate(t *testing.T) { tests := []struct { diff --git a/internal/storage/history.go b/internal/storage/history.go index 2b2f109..ae1ba51 100644 --- a/internal/storage/history.go +++ b/internal/storage/history.go @@ -104,9 +104,22 @@ func (s *Store) GetHistory(duration time.Duration) ([]HistoryPoint, error) { return history, nil } -// Cleanup removes old readings beyond retention period -func (s *Store) Cleanup(retention time.Duration) error { +// Cleanup removes old readings beyond retention period and returns the number +// of rows deleted. +func (s *Store) Cleanup(retention time.Duration) (int64, error) { cutoff := time.Now().Add(-retention) - _, err := s.db.Exec("DELETE FROM readings WHERE timestamp < ?", cutoff) - return err + // Match GetHistory's cutoff formatting: bind a plain UTC "YYYY-MM-DD + // HH:MM:SS" string, the same text format the timestamp column's + // DATETIME DEFAULT CURRENT_TIMESTAMP writes. Binding a raw time.Time here + // instead lets go-sqlite3 serialize it as local time with a numeric + // offset and fractional seconds, and SQLite compares timestamps as plain + // text — so retention would silently drift by the host's UTC offset. + res, err := s.db.Exec( + "DELETE FROM readings WHERE timestamp < datetime(?)", + cutoff.UTC().Format("2006-01-02 15:04:05"), + ) + if err != nil { + return 0, err + } + return res.RowsAffected() } diff --git a/internal/storage/history_test.go b/internal/storage/history_test.go new file mode 100644 index 0000000..f84642c --- /dev/null +++ b/internal/storage/history_test.go @@ -0,0 +1,119 @@ +package storage + +import ( + "testing" + "time" +) + +// lastReadingID returns the id of the most recently inserted row. Safe here +// because each test uses its own private :memory: DB with no concurrent +// writers, so it reliably identifies whichever RecordReading call just ran. +func lastReadingID(t *testing.T, s *Store) int64 { + t.Helper() + var id int64 + if err := s.db.QueryRow("SELECT max(id) FROM readings").Scan(&id); err != nil { + t.Fatalf("failed to read last reading id: %v", err) + } + return id +} + +// ageReading rewrites a row's timestamp to `hours` in the past using SQLite's +// own datetime('now', ...), which produces the exact same plain-UTC-text +// format ("YYYY-MM-DD HH:MM:SS", no offset, no fractional seconds) that the +// timestamp column's DATETIME DEFAULT CURRENT_TIMESTAMP writes via +// RecordReading. This lets tests age a real, production-written row without +// reimplementing timestamp formatting (and without depending on the host's +// local timezone, since SQLite's 'now' is always UTC). +func ageReading(t *testing.T, s *Store, id int64, hours int) { + t.Helper() + _, err := s.db.Exec( + "UPDATE readings SET timestamp = datetime('now', printf('-%d hours', ?)) WHERE id = ?", + hours, id, + ) + if err != nil { + t.Fatalf("failed to age reading id=%d: %v", id, err) + } +} + +// TestCleanupRemovesOnlyOldReadings guards against a real bug: Cleanup used to +// bind a raw time.Time as the DELETE parameter. go-sqlite3 serializes a +// time.Time using the HOST's local timezone plus a numeric offset and +// fractional seconds (e.g. "2026-07-16 12:02:29.5-04:00"), but the readings +// table stores plain UTC text via CURRENT_TIMESTAMP (e.g. +// "2026-07-17 16:02:29"). SQLite compares timestamps as plain strings, so +// that mismatch made retention drift by the host's UTC offset - on a +// non-UTC host a row could survive (or be deleted) hours earlier/later than +// intended. +// +// Rows are seeded through the REAL write path (RecordReading, which relies on +// CURRENT_TIMESTAMP) and then aged via ageReading (SQLite's own datetime('now', +// ...), same text format) rather than via a hand-formatted timestamp string - +// only that exercises the same write/compare path production uses. The seeded +// rows sit just inside/outside a 24h retention window (23h old vs. 25h old): +// that boundary fails under the old buggy code on any non-UTC host, and passes +// on the fixed code regardless of the host's timezone. +func TestCleanupRemovesOnlyOldReadings(t *testing.T) { + s, err := New(":memory:") + if err != nil { + t.Fatalf("failed to open in-memory store: %v", err) + } + defer s.Close() + + if err := s.RecordReading(50, 0, 20); err != nil { + t.Fatalf("RecordReading failed: %v", err) + } + tooOld := lastReadingID(t, s) + ageReading(t, s, tooOld, 25) // 25h old: must be deleted under 24h retention + + if err := s.RecordReading(51, 0, 20); err != nil { + t.Fatalf("RecordReading failed: %v", err) + } + stillFresh := lastReadingID(t, s) + ageReading(t, s, stillFresh, 23) // 23h old: must survive 24h retention + + if err := s.RecordReading(52, 0, 20); err != nil { + t.Fatalf("RecordReading failed: %v", err) + } + // Left at "now" (unaged): must survive. + + deleted, err := s.Cleanup(24 * time.Hour) + if err != nil { + t.Fatalf("Cleanup returned error: %v", err) + } + if deleted != 1 { + t.Fatalf("expected 1 row deleted (the 25h-old reading), got %d", deleted) + } + + remaining, err := s.GetHistory(365 * 24 * time.Hour) + if err != nil { + t.Fatalf("GetHistory returned error: %v", err) + } + if len(remaining) != 2 { + t.Fatalf("expected 2 rows remaining (23h-old + now), got %d", len(remaining)) + } + for _, p := range remaining { + if p.CPUTemp == 50 { + t.Fatalf("the 25h-old reading (cpu_temp=50) should have been deleted, but survived") + } + } +} + +func TestCleanupNoOldReadingsDeletesNothing(t *testing.T) { + s, err := New(":memory:") + if err != nil { + t.Fatalf("failed to open in-memory store: %v", err) + } + defer s.Close() + + if err := s.RecordReading(40, 30, 20); err != nil { + t.Fatalf("RecordReading failed: %v", err) + } + + deleted, err := s.Cleanup(30 * 24 * time.Hour) + if err != nil { + t.Fatalf("Cleanup returned error: %v", err) + } + if deleted != 0 { + t.Fatalf("expected 0 rows deleted, got %d", deleted) + } +} diff --git a/scripts/hint-client.sh b/scripts/hint-client.sh index 12a92f2..5b4a3cc 100755 --- a/scripts/hint-client.sh +++ b/scripts/hint-client.sh @@ -1,12 +1,14 @@ #!/bin/bash -# hint-client.sh - Send workload hints to the Smart Fan Controller +# hint-client.sh - Send workload hints to the Only Fan Controller # # Usage: # hint-client.sh start whisper high # Signal high GPU load starting # hint-client.sh stop whisper # Signal load complete # hint-client.sh status # Check controller status -CONTROLLER_URL="${SMART_FAN_URL:-http://localhost:8086}" +# FAN_URL is the current env var; SMART_FAN_URL is accepted as a fallback for +# scripts written against the controller's old name. +CONTROLLER_URL="${FAN_URL:-${SMART_FAN_URL:-http://localhost:8086}}" # Bearer token for the mutating endpoints (override / hint). Set API_TOKEN to # match api.token in the controller config. When empty, the controller only @@ -20,78 +22,113 @@ if [ -n "$API_TOKEN" ]; then AUTH_ARGS=(-H "Authorization: Bearer $API_TOKEN") fi +# json_escape makes a string safe to embed inside a double-quoted JSON string +# literal: backslash/double-quote would otherwise break out of the literal, +# and a raw newline/CR/tab (or any other C0 control character) makes the JSON +# invalid per RFC 8259. This is the only defense for fields the server doesn't +# itself restrict to a safe character class (e.g. override's "reason"). +json_escape() { + local s="$1" + s="${s//\\/\\\\}" + s="${s//\"/\\\"}" + s="${s//$'\t'/\\t}" + s="${s//$'\r'/\\r}" + s="${s//$'\n'/\\n}" + # Strip any remaining C0 control characters (everything below 0x20 except + # the three handled above), which have no business in these fields. + printf '%s' "$s" | tr -d '\000-\010\013-\014\016-\037' +} + +# is_integer checks that its argument is a plain non-negative integer, as +# required for the numeric (unquoted) fields we send in JSON bodies. +is_integer() { + [[ "$1" =~ ^[0-9]+$ ]] +} + case "$1" in start) SOURCE="$2" INTENSITY="${3:-medium}" DURATION="${4:-0}" - + if [ -z "$SOURCE" ]; then echo "Usage: $0 start [intensity] [duration_seconds]" exit 1 fi - + if ! is_integer "$DURATION"; then + echo "Error: duration_seconds must be a non-negative integer, got '$DURATION'" + exit 1 + fi + curl -s -X POST "$CONTROLLER_URL/api/hint" \ "${AUTH_ARGS[@]}" \ -H "Content-Type: application/json" \ -d "{ \"type\": \"gpu_load\", \"action\": \"start\", - \"intensity\": \"$INTENSITY\", + \"intensity\": \"$(json_escape "$INTENSITY")\", \"duration_estimate\": $DURATION, - \"source\": \"$SOURCE\" + \"source\": \"$(json_escape "$SOURCE")\" }" | jq . ;; - + stop) SOURCE="$2" - + if [ -z "$SOURCE" ]; then echo "Usage: $0 stop " exit 1 fi - + curl -s -X POST "$CONTROLLER_URL/api/hint" \ "${AUTH_ARGS[@]}" \ -H "Content-Type: application/json" \ -d "{ \"type\": \"gpu_load\", \"action\": \"stop\", - \"source\": \"$SOURCE\" + \"source\": \"$(json_escape "$SOURCE")\" }" | jq . ;; - + status) curl -s "$CONTROLLER_URL/api/status" | jq . ;; - + override) SPEED="$2" DURATION="${3:-0}" REASON="${4:-manual}" - + if [ -z "$SPEED" ]; then echo "Usage: $0 override [duration_seconds] [reason]" exit 1 fi - + if ! is_integer "$SPEED"; then + echo "Error: speed must be a non-negative integer, got '$SPEED'" + exit 1 + fi + if ! is_integer "$DURATION"; then + echo "Error: duration_seconds must be a non-negative integer, got '$DURATION'" + exit 1 + fi + curl -s -X POST "$CONTROLLER_URL/api/override" \ "${AUTH_ARGS[@]}" \ -H "Content-Type: application/json" \ -d "{ \"speed\": $SPEED, \"duration\": $DURATION, - \"reason\": \"$REASON\" + \"reason\": \"$(json_escape "$REASON")\" }" | jq . ;; - + clear-override) curl -s -X DELETE "$CONTROLLER_URL/api/override" \ "${AUTH_ARGS[@]}" | jq . ;; - + *) - echo "Smart Fan Controller Hint Client" + echo "Only Fan Controller Hint Client" echo "" echo "Usage: $0 [args...]" echo "" @@ -105,7 +142,8 @@ case "$1" in echo "Intensity: low, medium, high" echo "" echo "Environment:" - echo " SMART_FAN_URL Controller base URL (default http://localhost:8086)" + echo " FAN_URL Controller base URL (default http://localhost:8086)" + echo " SMART_FAN_URL Deprecated alias for FAN_URL, still honored" echo " API_TOKEN Bearer token for override/hint (required off-host)" echo "" echo "Examples:"