diff --git a/internal/config/config.go b/internal/config/config.go index ae61484..d2916a5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -26,7 +26,6 @@ type fileConfig struct { DefaultImageName string `toml:"default_image_name"` AutoStopStaleAfter string `toml:"auto_stop_stale_after"` StatsPollInterval string `toml:"stats_poll_interval"` - MetricsPoll string `toml:"metrics_poll_interval"` BridgeName string `toml:"bridge_name"` BridgeIP string `toml:"bridge_ip"` CIDR string `toml:"cidr"` @@ -54,16 +53,15 @@ type vmDefaultsFile struct { func Load(layout paths.Layout) (model.DaemonConfig, error) { cfg := model.DaemonConfig{ - LogLevel: "info", - AutoStopStaleAfter: 0, - StatsPollInterval: model.DefaultStatsPollInterval, - MetricsPollInterval: model.DefaultMetricsPollInterval, - BridgeName: model.DefaultBridgeName, - BridgeIP: model.DefaultBridgeIP, - CIDR: model.DefaultCIDR, - TapPoolSize: 4, - DefaultDNS: model.DefaultDNS, - DefaultImageName: "debian-bookworm", + LogLevel: "info", + AutoStopStaleAfter: 0, + StatsPollInterval: model.DefaultStatsPollInterval, + BridgeName: model.DefaultBridgeName, + BridgeIP: model.DefaultBridgeIP, + CIDR: model.DefaultCIDR, + TapPoolSize: 4, + DefaultDNS: model.DefaultDNS, + DefaultImageName: "debian-bookworm", } var file fileConfig @@ -120,13 +118,6 @@ func Load(layout paths.Layout) (model.DaemonConfig, error) { } 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 != "" { cfg.LogLevel = value } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c1e717d..d4ffb6d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -50,7 +50,6 @@ ssh_key_path = "/tmp/custom-key" default_image_name = "void" auto_stop_stale_after = "1h" stats_poll_interval = "15s" -metrics_poll_interval = "30s" bridge_name = "br-test" bridge_ip = "10.0.0.1" cidr = "25" @@ -84,9 +83,6 @@ default_dns = "9.9.9.9" if cfg.StatsPollInterval != 15*time.Second { 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" { t.Fatalf("bridge config = %+v", cfg) } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 3651c1c..7156473 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -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 { op := d.beginOperation("daemon.reconcile") vms, err := d.store.ListVMs(ctx) diff --git a/internal/model/types.go b/internal/model/types.go index 64011dd..bbda953 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -11,18 +11,17 @@ import ( ) const ( - DefaultBridgeName = "br-fc" - DefaultBridgeIP = "172.16.0.1" - DefaultCIDR = "24" - DefaultDNS = "1.1.1.1" - DefaultSystemOverlaySize = 8 * 1024 * 1024 * 1024 - DefaultWorkDiskSize = 8 * 1024 * 1024 * 1024 - DefaultMemoryMiB = 2048 - DefaultVCPUCount = 2 - DefaultStatsPollInterval = 10 * time.Second - DefaultStaleSweepInterval = 1 * time.Minute - DefaultMetricsPollInterval = 15 * time.Second - MaxDiskBytes int64 = 128 * 1024 * 1024 * 1024 + DefaultBridgeName = "br-fc" + DefaultBridgeIP = "172.16.0.1" + DefaultCIDR = "24" + DefaultDNS = "1.1.1.1" + DefaultSystemOverlaySize = 8 * 1024 * 1024 * 1024 + DefaultWorkDiskSize = 8 * 1024 * 1024 * 1024 + DefaultMemoryMiB = 2048 + DefaultVCPUCount = 2 + DefaultStatsPollInterval = 10 * time.Second + DefaultStaleSweepInterval = 1 * time.Minute + MaxDiskBytes int64 = 128 * 1024 * 1024 * 1024 ) type VMState string @@ -35,20 +34,19 @@ const ( ) type DaemonConfig struct { - LogLevel string - FirecrackerBin string - SSHKeyPath string - AutoStopStaleAfter time.Duration - StatsPollInterval time.Duration - MetricsPollInterval time.Duration - BridgeName string - BridgeIP string - CIDR string - TapPoolSize int - DefaultDNS string - DefaultImageName string - FileSync []FileSyncEntry - VMDefaults VMDefaultsOverride + LogLevel string + FirecrackerBin string + SSHKeyPath string + AutoStopStaleAfter time.Duration + StatsPollInterval time.Duration + BridgeName string + BridgeIP string + CIDR string + TapPoolSize int + DefaultDNS string + DefaultImageName string + FileSync []FileSyncEntry + VMDefaults VMDefaultsOverride } // FileSyncEntry is a user-declared host→guest file or directory copy diff --git a/internal/store/migrations.go b/internal/store/migrations.go index 68b4ad3..2490942 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -24,6 +24,7 @@ type migration struct { // entries — installed DBs key off the id column. var migrations = []migration{ {id: 1, name: "baseline", up: migrateBaseline}, + {id: 2, name: "drop_dead_image_columns", up: migrateDropDeadImageColumns}, } // runMigrations ensures schema_migrations exists, then applies every @@ -163,6 +164,55 @@ func migrateBaseline(tx *sql.Tx) error { 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" // (which the dialect lacks) as a library function. Used inside // migrations when a column needs to survive a database that went diff --git a/internal/store/migrations_test.go b/internal/store/migrations_test.go index 30d72bf..bf1e209 100644 --- a/internal/store/migrations_test.go +++ b/internal/store/migrations_test.go @@ -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) { db := openRawDB(t) orig := migrations diff --git a/internal/store/store_test.go b/internal/store/store_test.go index ea535fc..4da722e 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -178,8 +178,8 @@ func TestGetImageRejectsMalformedTimestamp(t *testing.T) { _, err := store.db.ExecContext(ctx, ` INSERT INTO images ( id, name, managed, artifact_dir, rootfs_path, kernel_path, initrd_path, - modules_dir, packages_path, build_size, docker, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + modules_dir, build_size, docker, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, "image-bad-time", "image-bad-time", 0, @@ -189,7 +189,6 @@ func TestGetImageRejectsMalformedTimestamp(t *testing.T) { "", "", "", - "", 0, "not-a-time", "not-a-time",