config + store: remove dead knobs and stale schema
Three drift items surfaced in review, each dead on arrival and each worth trusting a little more at v0.1.0. config: drop MetricsPollInterval. The field was parsed from TOML (metrics_poll_interval), stored on DaemonConfig, and ignored by every consumer — only StatsPollInterval drives the background poll loop. Users setting it in config.toml saw zero effect. Removed from the TOML surface, the model constant, and the config test. daemon: delete ensureDefaultImage. No callers, body was `_ = ctx; return nil`. Dead since whatever flow used to call it got removed. store: drop packages_path from the images table. The column was carried by the baseline migration but never referenced by UpsertImage (no INSERT / UPDATE mention) or any Go model field — a ghost from a build pipeline that no longer exists. Added migration id=2 (drop_dead_image_columns) with an idempotent dropColumnIfExists helper: fresh installs run baseline (creates the column) + 2 (drops it); legacy DBs where the column was never added get a no-op. Updated the direct-INSERT SQL in TestGetImageRejectsMalformedTimestamp to drop the column reference, and added a migration test covering both install paths (fresh + legacy). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2a7f55f028
commit
129475be20
7 changed files with 159 additions and 56 deletions
|
|
@ -26,7 +26,6 @@ type fileConfig struct {
|
||||||
DefaultImageName string `toml:"default_image_name"`
|
DefaultImageName string `toml:"default_image_name"`
|
||||||
AutoStopStaleAfter string `toml:"auto_stop_stale_after"`
|
AutoStopStaleAfter string `toml:"auto_stop_stale_after"`
|
||||||
StatsPollInterval string `toml:"stats_poll_interval"`
|
StatsPollInterval string `toml:"stats_poll_interval"`
|
||||||
MetricsPoll string `toml:"metrics_poll_interval"`
|
|
||||||
BridgeName string `toml:"bridge_name"`
|
BridgeName string `toml:"bridge_name"`
|
||||||
BridgeIP string `toml:"bridge_ip"`
|
BridgeIP string `toml:"bridge_ip"`
|
||||||
CIDR string `toml:"cidr"`
|
CIDR string `toml:"cidr"`
|
||||||
|
|
@ -54,16 +53,15 @@ type vmDefaultsFile struct {
|
||||||
|
|
||||||
func Load(layout paths.Layout) (model.DaemonConfig, error) {
|
func Load(layout paths.Layout) (model.DaemonConfig, error) {
|
||||||
cfg := model.DaemonConfig{
|
cfg := model.DaemonConfig{
|
||||||
LogLevel: "info",
|
LogLevel: "info",
|
||||||
AutoStopStaleAfter: 0,
|
AutoStopStaleAfter: 0,
|
||||||
StatsPollInterval: model.DefaultStatsPollInterval,
|
StatsPollInterval: model.DefaultStatsPollInterval,
|
||||||
MetricsPollInterval: model.DefaultMetricsPollInterval,
|
BridgeName: model.DefaultBridgeName,
|
||||||
BridgeName: model.DefaultBridgeName,
|
BridgeIP: model.DefaultBridgeIP,
|
||||||
BridgeIP: model.DefaultBridgeIP,
|
CIDR: model.DefaultCIDR,
|
||||||
CIDR: model.DefaultCIDR,
|
TapPoolSize: 4,
|
||||||
TapPoolSize: 4,
|
DefaultDNS: model.DefaultDNS,
|
||||||
DefaultDNS: model.DefaultDNS,
|
DefaultImageName: "debian-bookworm",
|
||||||
DefaultImageName: "debian-bookworm",
|
|
||||||
}
|
}
|
||||||
|
|
||||||
var file fileConfig
|
var file fileConfig
|
||||||
|
|
@ -120,13 +118,6 @@ func Load(layout paths.Layout) (model.DaemonConfig, error) {
|
||||||
}
|
}
|
||||||
cfg.StatsPollInterval = duration
|
cfg.StatsPollInterval = duration
|
||||||
}
|
}
|
||||||
if value := strings.TrimSpace(file.MetricsPoll); value != "" {
|
|
||||||
duration, err := time.ParseDuration(value)
|
|
||||||
if err != nil {
|
|
||||||
return cfg, err
|
|
||||||
}
|
|
||||||
cfg.MetricsPollInterval = duration
|
|
||||||
}
|
|
||||||
if value := strings.TrimSpace(os.Getenv("BANGER_LOG_LEVEL")); value != "" {
|
if value := strings.TrimSpace(os.Getenv("BANGER_LOG_LEVEL")); value != "" {
|
||||||
cfg.LogLevel = value
|
cfg.LogLevel = value
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -50,7 +50,6 @@ ssh_key_path = "/tmp/custom-key"
|
||||||
default_image_name = "void"
|
default_image_name = "void"
|
||||||
auto_stop_stale_after = "1h"
|
auto_stop_stale_after = "1h"
|
||||||
stats_poll_interval = "15s"
|
stats_poll_interval = "15s"
|
||||||
metrics_poll_interval = "30s"
|
|
||||||
bridge_name = "br-test"
|
bridge_name = "br-test"
|
||||||
bridge_ip = "10.0.0.1"
|
bridge_ip = "10.0.0.1"
|
||||||
cidr = "25"
|
cidr = "25"
|
||||||
|
|
@ -84,9 +83,6 @@ default_dns = "9.9.9.9"
|
||||||
if cfg.StatsPollInterval != 15*time.Second {
|
if cfg.StatsPollInterval != 15*time.Second {
|
||||||
t.Fatalf("StatsPollInterval = %s", cfg.StatsPollInterval)
|
t.Fatalf("StatsPollInterval = %s", cfg.StatsPollInterval)
|
||||||
}
|
}
|
||||||
if cfg.MetricsPollInterval != 30*time.Second {
|
|
||||||
t.Fatalf("MetricsPollInterval = %s", cfg.MetricsPollInterval)
|
|
||||||
}
|
|
||||||
if cfg.BridgeName != "br-test" || cfg.BridgeIP != "10.0.0.1" || cfg.CIDR != "25" {
|
if cfg.BridgeName != "br-test" || cfg.BridgeIP != "10.0.0.1" || cfg.CIDR != "25" {
|
||||||
t.Fatalf("bridge config = %+v", cfg)
|
t.Fatalf("bridge config = %+v", cfg)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -517,11 +517,6 @@ func (d *Daemon) backgroundLoop() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *Daemon) ensureDefaultImage(ctx context.Context) error {
|
|
||||||
_ = ctx
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
func (d *Daemon) reconcile(ctx context.Context) error {
|
func (d *Daemon) reconcile(ctx context.Context) error {
|
||||||
op := d.beginOperation("daemon.reconcile")
|
op := d.beginOperation("daemon.reconcile")
|
||||||
vms, err := d.store.ListVMs(ctx)
|
vms, err := d.store.ListVMs(ctx)
|
||||||
|
|
|
||||||
|
|
@ -11,18 +11,17 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
const (
|
const (
|
||||||
DefaultBridgeName = "br-fc"
|
DefaultBridgeName = "br-fc"
|
||||||
DefaultBridgeIP = "172.16.0.1"
|
DefaultBridgeIP = "172.16.0.1"
|
||||||
DefaultCIDR = "24"
|
DefaultCIDR = "24"
|
||||||
DefaultDNS = "1.1.1.1"
|
DefaultDNS = "1.1.1.1"
|
||||||
DefaultSystemOverlaySize = 8 * 1024 * 1024 * 1024
|
DefaultSystemOverlaySize = 8 * 1024 * 1024 * 1024
|
||||||
DefaultWorkDiskSize = 8 * 1024 * 1024 * 1024
|
DefaultWorkDiskSize = 8 * 1024 * 1024 * 1024
|
||||||
DefaultMemoryMiB = 2048
|
DefaultMemoryMiB = 2048
|
||||||
DefaultVCPUCount = 2
|
DefaultVCPUCount = 2
|
||||||
DefaultStatsPollInterval = 10 * time.Second
|
DefaultStatsPollInterval = 10 * time.Second
|
||||||
DefaultStaleSweepInterval = 1 * time.Minute
|
DefaultStaleSweepInterval = 1 * time.Minute
|
||||||
DefaultMetricsPollInterval = 15 * time.Second
|
MaxDiskBytes int64 = 128 * 1024 * 1024 * 1024
|
||||||
MaxDiskBytes int64 = 128 * 1024 * 1024 * 1024
|
|
||||||
)
|
)
|
||||||
|
|
||||||
type VMState string
|
type VMState string
|
||||||
|
|
@ -35,20 +34,19 @@ const (
|
||||||
)
|
)
|
||||||
|
|
||||||
type DaemonConfig struct {
|
type DaemonConfig struct {
|
||||||
LogLevel string
|
LogLevel string
|
||||||
FirecrackerBin string
|
FirecrackerBin string
|
||||||
SSHKeyPath string
|
SSHKeyPath string
|
||||||
AutoStopStaleAfter time.Duration
|
AutoStopStaleAfter time.Duration
|
||||||
StatsPollInterval time.Duration
|
StatsPollInterval time.Duration
|
||||||
MetricsPollInterval time.Duration
|
BridgeName string
|
||||||
BridgeName string
|
BridgeIP string
|
||||||
BridgeIP string
|
CIDR string
|
||||||
CIDR string
|
TapPoolSize int
|
||||||
TapPoolSize int
|
DefaultDNS string
|
||||||
DefaultDNS string
|
DefaultImageName string
|
||||||
DefaultImageName string
|
FileSync []FileSyncEntry
|
||||||
FileSync []FileSyncEntry
|
VMDefaults VMDefaultsOverride
|
||||||
VMDefaults VMDefaultsOverride
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// FileSyncEntry is a user-declared host→guest file or directory copy
|
// FileSyncEntry is a user-declared host→guest file or directory copy
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,7 @@ type migration struct {
|
||||||
// entries — installed DBs key off the id column.
|
// entries — installed DBs key off the id column.
|
||||||
var migrations = []migration{
|
var migrations = []migration{
|
||||||
{id: 1, name: "baseline", up: migrateBaseline},
|
{id: 1, name: "baseline", up: migrateBaseline},
|
||||||
|
{id: 2, name: "drop_dead_image_columns", up: migrateDropDeadImageColumns},
|
||||||
}
|
}
|
||||||
|
|
||||||
// runMigrations ensures schema_migrations exists, then applies every
|
// runMigrations ensures schema_migrations exists, then applies every
|
||||||
|
|
@ -163,6 +164,55 @@ func migrateBaseline(tx *sql.Tx) error {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// migrateDropDeadImageColumns removes image-table columns that the
|
||||||
|
// store never reads or writes. `packages_path` was introduced for a
|
||||||
|
// build pipeline that no longer exists; the baseline migration still
|
||||||
|
// creates it for historical fidelity, and this migration drops it on
|
||||||
|
// new installs + any upgrader that still carries it. Idempotent via
|
||||||
|
// dropColumnIfExists so running the migration twice (or against a
|
||||||
|
// DB where the column was already gone) is a no-op.
|
||||||
|
func migrateDropDeadImageColumns(tx *sql.Tx) error {
|
||||||
|
return dropColumnIfExists(tx, "images", "packages_path")
|
||||||
|
}
|
||||||
|
|
||||||
|
// dropColumnIfExists is SQLite's "ALTER TABLE DROP COLUMN IF EXISTS"
|
||||||
|
// (which the dialect lacks) as a library function. modernc.org/sqlite
|
||||||
|
// bundles SQLite 3.42+, which supports plain DROP COLUMN — we add the
|
||||||
|
// existence guard so the statement is idempotent across repeat runs
|
||||||
|
// and legacy DBs that never had the column in the first place.
|
||||||
|
func dropColumnIfExists(tx *sql.Tx, table, column string) error {
|
||||||
|
rows, err := tx.Query(fmt.Sprintf("PRAGMA table_info(%s)", table))
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
defer rows.Close()
|
||||||
|
var found bool
|
||||||
|
for rows.Next() {
|
||||||
|
var (
|
||||||
|
cid int
|
||||||
|
name string
|
||||||
|
valueType string
|
||||||
|
notNull int
|
||||||
|
defaultV sql.NullString
|
||||||
|
pk int
|
||||||
|
)
|
||||||
|
if err := rows.Scan(&cid, &name, &valueType, ¬Null, &defaultV, &pk); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if name == column {
|
||||||
|
found = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if !found {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
_, err = tx.Exec(fmt.Sprintf("ALTER TABLE %s DROP COLUMN %s", table, column))
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// addColumnIfMissing is SQLite's "ALTER TABLE ADD COLUMN IF NOT EXISTS"
|
// addColumnIfMissing is SQLite's "ALTER TABLE ADD COLUMN IF NOT EXISTS"
|
||||||
// (which the dialect lacks) as a library function. Used inside
|
// (which the dialect lacks) as a library function. Used inside
|
||||||
// migrations when a column needs to survive a database that went
|
// migrations when a column needs to survive a database that went
|
||||||
|
|
|
||||||
|
|
@ -141,6 +141,80 @@ func TestApplyMigrationRollsBackOnBodyError(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestMigrateDropDeadImageColumns_AcrossInstallPaths verifies the
|
||||||
|
// drop-column migration is correct on both paths it can land on:
|
||||||
|
// a fresh install (baseline created the column, migration 2 drops
|
||||||
|
// it) and a legacy DB that somehow lost or never had the column
|
||||||
|
// (migration 2 is a no-op). Runs migrations end-to-end so the
|
||||||
|
// invariant-check is the real system, not the helper in isolation.
|
||||||
|
func TestMigrateDropDeadImageColumns_AcrossInstallPaths(t *testing.T) {
|
||||||
|
hasColumn := func(t *testing.T, db *sql.DB, table, column string) bool {
|
||||||
|
t.Helper()
|
||||||
|
rows, err := db.Query("PRAGMA table_info(" + table + ")")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("PRAGMA table_info: %v", err)
|
||||||
|
}
|
||||||
|
defer rows.Close()
|
||||||
|
for rows.Next() {
|
||||||
|
var (
|
||||||
|
cid int
|
||||||
|
name string
|
||||||
|
valueType string
|
||||||
|
notNull int
|
||||||
|
defaultV sql.NullString
|
||||||
|
pk int
|
||||||
|
)
|
||||||
|
if err := rows.Scan(&cid, &name, &valueType, ¬Null, &defaultV, &pk); err != nil {
|
||||||
|
t.Fatalf("scan table_info row: %v", err)
|
||||||
|
}
|
||||||
|
if name == column {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
t.Fatalf("rows.Err: %v", err)
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Run("fresh install drops packages_path", func(t *testing.T) {
|
||||||
|
db := openRawDB(t)
|
||||||
|
if err := runMigrations(db); err != nil {
|
||||||
|
t.Fatalf("runMigrations: %v", err)
|
||||||
|
}
|
||||||
|
if hasColumn(t, db, "images", "packages_path") {
|
||||||
|
t.Fatal("packages_path column survived migration 2 on fresh install")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("legacy DB without column is a no-op", func(t *testing.T) {
|
||||||
|
db := openRawDB(t)
|
||||||
|
// Simulate a DB whose baseline was applied against a modified
|
||||||
|
// schema that never had packages_path: seed schema_migrations,
|
||||||
|
// run baseline, drop the column out-of-band, then run
|
||||||
|
// runMigrations and expect migration 2 to succeed regardless.
|
||||||
|
if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations (
|
||||||
|
id INTEGER PRIMARY KEY,
|
||||||
|
name TEXT NOT NULL,
|
||||||
|
applied_at TEXT NOT NULL
|
||||||
|
)`); err != nil {
|
||||||
|
t.Fatalf("seed schema_migrations: %v", err)
|
||||||
|
}
|
||||||
|
if err := applyMigration(db, migrations[0]); err != nil {
|
||||||
|
t.Fatalf("apply baseline: %v", err)
|
||||||
|
}
|
||||||
|
if _, err := db.Exec("ALTER TABLE images DROP COLUMN packages_path"); err != nil {
|
||||||
|
t.Fatalf("pre-drop packages_path: %v", err)
|
||||||
|
}
|
||||||
|
if err := runMigrations(db); err != nil {
|
||||||
|
t.Fatalf("runMigrations after manual pre-drop: %v", err)
|
||||||
|
}
|
||||||
|
if hasColumn(t, db, "images", "packages_path") {
|
||||||
|
t.Fatal("packages_path reappeared after runMigrations")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func TestRunMigrationsRejectsDuplicateID(t *testing.T) {
|
func TestRunMigrationsRejectsDuplicateID(t *testing.T) {
|
||||||
db := openRawDB(t)
|
db := openRawDB(t)
|
||||||
orig := migrations
|
orig := migrations
|
||||||
|
|
|
||||||
|
|
@ -178,8 +178,8 @@ func TestGetImageRejectsMalformedTimestamp(t *testing.T) {
|
||||||
_, err := store.db.ExecContext(ctx, `
|
_, err := store.db.ExecContext(ctx, `
|
||||||
INSERT INTO images (
|
INSERT INTO images (
|
||||||
id, name, managed, artifact_dir, rootfs_path, kernel_path, initrd_path,
|
id, name, managed, artifact_dir, rootfs_path, kernel_path, initrd_path,
|
||||||
modules_dir, packages_path, build_size, docker, created_at, updated_at
|
modules_dir, build_size, docker, created_at, updated_at
|
||||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
|
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||||
"image-bad-time",
|
"image-bad-time",
|
||||||
"image-bad-time",
|
"image-bad-time",
|
||||||
0,
|
0,
|
||||||
|
|
@ -189,7 +189,6 @@ func TestGetImageRejectsMalformedTimestamp(t *testing.T) {
|
||||||
"",
|
"",
|
||||||
"",
|
"",
|
||||||
"",
|
"",
|
||||||
"",
|
|
||||||
0,
|
0,
|
||||||
"not-a-time",
|
"not-a-time",
|
||||||
"not-a-time",
|
"not-a-time",
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue