diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b5e2989c8..13a92ca3f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -149,3 +149,14 @@ jobs: with: directory: processed-templates config: templates/.kube-linter-config.yml + container-test: + name: "🚢 container test" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3.0.2 + - name: Install Prerequisites + run: | + sudo apt update + sudo apt install -y podman + - name: Build and test maintenance container + run: make ubi-maintenance-container-test diff --git a/.gitignore b/.gitignore index 8e7b6987a..99e56d4d7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ /image-builder rpmbuild /image-builder-db-test +/image-builder-maintenance +/image-builder-maintenance-test /image-builder-migrate-db /image-builder-migrate-db-tern dnf-json diff --git a/Makefile b/Makefile index 69715187a..8f1a20283 100644 --- a/Makefile +++ b/Makefile @@ -31,8 +31,12 @@ gen-oscap: image-builder-migrate-db-tern: go build -o image-builder-migrate-db-tern ./cmd/image-builder-migrate-db-tern/ +.PHONY: image-builder-maintenance +image-builder-maintenance: + go build -o image-builder-maintenance ./cmd/image-builder-maintenance/ + .PHONY: build -build: image-builder gen-oscap image-builder-migrate-db-tern +build: image-builder gen-oscap image-builder-migrate-db-tern image-builder-maintenance .PHONY: run run: @@ -46,7 +50,15 @@ check-api-spec: .PHONY: ubi-container ubi-container: if [ -f .git ]; then echo "You seem to be in a git worktree - build will fail here"; exit 1; fi - podman build --pull=always -t osbuild/image-builder -f distribution/Dockerfile-ubi . + # backwards compatibility with old podman used in github + podman build --pull=always -t osbuild/image-builder -f distribution/Dockerfile-ubi . || \ + podman build -t osbuild/image-builder -f distribution/Dockerfile-ubi . + +.PHONY: ubi-maintenance-container-test +ubi-maintenance-container-test: ubi-container + # just check if the container would start + # functional tests are in the target "db-tests" + podman run --rm --tty --entrypoint /app/image-builder-maintenance osbuild/image-builder 2>&1 | grep "Dry run, no state will be changed" .PHONY: generate-openscap-blueprints generate-openscap-blueprints: @@ -73,7 +85,7 @@ generate: go generate ./... .PHONY: push-check -push-check: generate build unit-tests +push-check: generate build unit-tests ubi-maintenance-container-test ./tools/prepare-source.sh @if [ 0 -ne $$(git status --porcelain --untracked-files|wc -l) ]; then \ echo "There should be no changed or untracked files"; \ diff --git a/cmd/image-builder-db-test/main_test.go b/cmd/image-builder-db-test/main_test.go index 2fcd3a009..15dbbff4b 100644 --- a/cmd/image-builder-db-test/main_test.go +++ b/cmd/image-builder-db-test/main_test.go @@ -1,24 +1,21 @@ -//go:build integration +//go:build dbtests package main import ( "context" "encoding/json" - "fmt" - "os/exec" - "strings" "testing" "time" "github.com/google/uuid" - "github.com/jackc/pgx/v5" "github.com/stretchr/testify/require" "github.com/osbuild/image-builder/internal/common" - "github.com/osbuild/image-builder/internal/config" "github.com/osbuild/image-builder/internal/db" v1 "github.com/osbuild/image-builder/internal/v1" + + "github.com/osbuild/image-builder/internal/tutils" ) const ( @@ -35,72 +32,8 @@ const ( fortnight = time.Hour * 24 * 14 ) -func conf(t *testing.T) *config.ImageBuilderConfig { - c := config.ImageBuilderConfig{ - ListenAddress: "unused", - LogLevel: "INFO", - TernMigrationsDir: "/usr/share/image-builder/migrations-tern", - PGHost: "localhost", - PGPort: "5432", - PGDatabase: "imagebuilder", - PGUser: "postgres", - PGPassword: "foobar", - PGSSLMode: "disable", - } - - err := config.LoadConfigFromEnv(&c) - require.NoError(t, err) - - return &c -} - -func connStr(t *testing.T) string { - c := conf(t) - return fmt.Sprintf("postgres://%s:%s@%s:%s/%s?sslmode=%s", c.PGUser, c.PGPassword, c.PGHost, c.PGPort, c.PGDatabase, c.PGSSLMode) -} - -func migrateTern(t *testing.T) { - // run tern migration on top of existing db - c := conf(t) - - a := []string{ - "migrate", - "--migrations", c.TernMigrationsDir, - "--database", c.PGDatabase, - "--host", c.PGHost, "--port", c.PGPort, - "--user", c.PGUser, "--password", c.PGPassword, - "--sslmode", c.PGSSLMode, - } - cmd := exec.Command("tern", a...) - - output, err := cmd.CombinedOutput() - require.NoErrorf(t, err, "tern command failed with non-zero code, cmd: tern %s, combined output: %s", strings.Join(a, " "), string(output)) -} - -func connect(t *testing.T) *pgx.Conn { - ctx := context.Background() - conn, err := pgx.Connect(ctx, connStr(t)) - require.NoError(t, err) - return conn -} - -func tearDown(t *testing.T) { - ctx := context.Background() - conn := connect(t) - defer conn.Close(ctx) - _, err := conn.Exec(ctx, "drop schema public cascade") - require.NoError(t, err) - _, err = conn.Exec(ctx, "create schema public") - require.NoError(t, err) - _, err = conn.Exec(ctx, "grant all on schema public to postgres") - require.NoError(t, err) - _, err = conn.Exec(ctx, "grant all on schema public to public") - require.NoError(t, err) -} - -func testInsertCompose(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testInsertCompose(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) imageName := "MyImageName" @@ -108,7 +41,7 @@ func testInsertCompose(t *testing.T) { blueprintId := uuid.New() versionId := uuid.New() - migrateTern(t) + tutils.MigrateTern(ctx, t) err = d.InsertBlueprint(ctx, blueprintId, versionId, ORGID1, ANR1, "blueprint", "blueprint desc", []byte("{}"), []byte("{}")) require.NoError(t, err) @@ -122,9 +55,8 @@ func testInsertCompose(t *testing.T) { require.NoError(t, err) } -func testGetCompose(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testGetCompose(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) imageName := "MyImageName" @@ -164,14 +96,13 @@ func testGetCompose(t *testing.T) { } -func testCountComposesSince(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testCountComposesSince(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) imageName := "MyImageName" - conn := connect(t) + conn := tutils.Connect(t) defer conn.Close(ctx) insert := "INSERT INTO composes(job_id, request, created_at, account_number, org_id, image_name) VALUES ($1, $2, CURRENT_TIMESTAMP - interval '2 days', $3, $4, $5)" _, err = conn.Exec(ctx, insert, uuid.New().String(), "{}", ANR3, ORGID3, &imageName) @@ -198,12 +129,11 @@ func testCountComposesSince(t *testing.T) { require.Equal(t, 3, count) } -func testCountGetComposesSince(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testCountGetComposesSince(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) - conn := connect(t) + conn := tutils.Connect(t) defer conn.Close(ctx) job1 := uuid.New() @@ -232,11 +162,10 @@ func testCountGetComposesSince(t *testing.T) { require.Equal(t, job1, composes[0].Id) } -func testGetComposeImageType(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testGetComposeImageType(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) - conn := connect(t) + conn := tutils.Connect(t) defer conn.Close(ctx) composeId := uuid.New() @@ -269,11 +198,10 @@ func testGetComposeImageType(t *testing.T) { require.Error(t, err) } -func testDeleteCompose(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testDeleteCompose(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) - conn := connect(t) + conn := tutils.Connect(t) defer conn.Close(ctx) composeId := uuid.New() @@ -299,11 +227,10 @@ func testDeleteCompose(t *testing.T) { require.Equal(t, 1, count) } -func testClones(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testClones(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) - conn := connect(t) + conn := tutils.Connect(t) defer conn.Close(ctx) composeId := uuid.New() @@ -373,11 +300,10 @@ func testClones(t *testing.T) { require.Equal(t, clones[1], *entry) } -func testBlueprints(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testBlueprints(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) - conn := connect(t) + conn := tutils.Connect(t) defer conn.Close(ctx) name1 := "name" @@ -526,11 +452,10 @@ func testBlueprints(t *testing.T) { require.Equal(t, 0, count) } -func testGetBlueprintComposes(t *testing.T) { - ctx := context.Background() - d, err := db.InitDBConnectionPool(connStr(t)) +func testGetBlueprintComposes(ctx context.Context, t *testing.T) { + d, err := db.InitDBConnectionPool(ctx, tutils.ConnStr(t)) require.NoError(t, err) - conn := connect(t) + conn := tutils.Connect(t) defer conn.Close(ctx) id := uuid.New() @@ -596,14 +521,9 @@ func testGetBlueprintComposes(t *testing.T) { require.Equal(t, 2, version) } -func runTest(t *testing.T, f func(*testing.T)) { - migrateTern(t) - defer tearDown(t) - f(t) -} - func TestAll(t *testing.T) { - fns := []func(*testing.T){ + ctx := context.Background() + fns := []func(context.Context, *testing.T){ testInsertCompose, testGetCompose, testCountComposesSince, @@ -615,6 +535,12 @@ func TestAll(t *testing.T) { } for _, f := range fns { - runTest(t, f) + select { + case <-ctx.Done(): + require.NoError(t, ctx.Err()) + return + default: + tutils.RunTest(ctx, t, f) + } } } diff --git a/cmd/image-builder-maintenance/config.go b/cmd/image-builder-maintenance/config.go new file mode 100644 index 000000000..bc84c1d0c --- /dev/null +++ b/cmd/image-builder-maintenance/config.go @@ -0,0 +1,61 @@ +package main + +import ( + "fmt" + "os" + "reflect" + "strconv" +) + +// Do not write this config to logs or stdout, it contains secrets! +type Config struct { + DryRun bool `env:"DRY_RUN"` + EnableDBMaintenance bool `env:"ENABLE_DB_MAINTENANCE"` + ClonesRetentionMonths int `env:"DB_CLONES_RETENTION_MONTHS"` + PGHost string `env:"PGHOST"` + PGPort string `env:"PGPORT"` + PGDatabase string `env:"PGDATABASE"` + PGUser string `env:"PGUSER"` + PGPassword string `env:"PGPASSWORD"` + PGSSLMode string `env:"PGSSLMODE"` +} + +// *string means the value is not required +// string means the value is required and should have a default value +func LoadConfigFromEnv(intf interface{}) error { + t := reflect.TypeOf(intf).Elem() + v := reflect.ValueOf(intf).Elem() + + for i := 0; i < v.NumField(); i++ { + fieldT := t.Field(i) + fieldV := v.Field(i) + key, ok := fieldT.Tag.Lookup("env") + if !ok { + return fmt.Errorf("No env tag in config field") + } + + confV, ok := os.LookupEnv(key) + kind := fieldV.Kind() + if ok { + switch kind { + case reflect.String: + fieldV.SetString(confV) + case reflect.Int: + value, err := strconv.ParseInt(confV, 10, 64) + if err != nil { + return err + } + fieldV.SetInt(value) + case reflect.Bool: + value, err := strconv.ParseBool(confV) + if err != nil { + return err + } + fieldV.SetBool(value) + default: + return fmt.Errorf("Unsupported type") + } + } + } + return nil +} diff --git a/cmd/image-builder-maintenance/db.go b/cmd/image-builder-maintenance/db.go new file mode 100644 index 000000000..a3e5be8fc --- /dev/null +++ b/cmd/image-builder-maintenance/db.go @@ -0,0 +1,208 @@ +package main + +import ( + "context" + "fmt" + "time" + + "github.com/jackc/pgx/v5" + + "github.com/sirupsen/logrus" +) + +const ( + sqlDeleteComposes = ` + DELETE FROM composes + WHERE created_at < $1` + sqlExpiredClonesCount = ` + SELECT COUNT(*) FROM clones + WHERE compose_id in ( + SELECT job_id + FROM composes + WHERE created_at < $1 + )` + sqlExpiredComposesCount = ` + SELECT COUNT(*) FROM composes + WHERE created_at < $1` + sqlVacuumAnalyze = ` + VACUUM ANALYZE` + sqlVacuumStats = ` + SELECT relname, pg_size_pretty(pg_total_relation_size(relid)), + n_tup_ins, n_tup_upd, n_tup_del, n_live_tup, n_dead_tup, + vacuum_count, autovacuum_count, analyze_count, autoanalyze_count, + last_vacuum, last_autovacuum, last_analyze, last_autoanalyze + FROM pg_stat_user_tables` +) + +type maintenanceDB struct { + Conn *pgx.Conn +} + +func newDB(ctx context.Context, dbURL string) (maintenanceDB, error) { + conn, err := pgx.Connect(ctx, dbURL) + if err != nil { + return maintenanceDB{}, err + } + + return maintenanceDB{ + conn, + }, nil +} + +func (d *maintenanceDB) Close() error { + return d.Conn.Close(context.Background()) +} + +func (d *maintenanceDB) DeleteComposes(ctx context.Context, emailRetentionDate time.Time) (int64, error) { + tag, err := d.Conn.Exec(ctx, sqlDeleteComposes, emailRetentionDate) + if err != nil { + return tag.RowsAffected(), fmt.Errorf("Error deleting composes: %v", err) + } + return tag.RowsAffected(), nil +} + +func (d *maintenanceDB) ExpiredClonesCount(ctx context.Context, emailRetentionDate time.Time) (int64, error) { + var count int64 + err := d.Conn.QueryRow(ctx, sqlExpiredClonesCount, emailRetentionDate).Scan(&count) + if err != nil { + return 0, err + } + return count, nil +} + +func (d *maintenanceDB) ExpiredComposesCount(ctx context.Context, emailRetentionDate time.Time) (int64, error) { + var count int64 + err := d.Conn.QueryRow(ctx, sqlExpiredComposesCount, emailRetentionDate).Scan(&count) + if err != nil { + return 0, err + } + return count, nil +} + +func (d *maintenanceDB) VacuumAnalyze(ctx context.Context) error { + _, err := d.Conn.Exec(ctx, sqlVacuumAnalyze) + if err != nil { + return fmt.Errorf("Error running VACUUM ANALYZE: %v", err) + } + return nil +} + +func (d *maintenanceDB) LogVacuumStats(ctx context.Context) (int64, error) { + rows, err := d.Conn.Query(ctx, sqlVacuumStats) + if err != nil { + return int64(0), fmt.Errorf("Error querying vacuum stats: %v", err) + } + defer rows.Close() + + deleted := int64(0) + + for rows.Next() { + select { + case <-ctx.Done(): + err = ctx.Err() + logrus.Errorf("Context cancelled LogVacuumStats: %v", err) + return int64(0), err + default: + var relName, relSize string + var ins, upd, del, live, dead, vc, avc, ac, aac int64 + var lvc, lavc, lan, laan *time.Time + + err = rows.Scan(&relName, &relSize, &ins, &upd, &del, &live, &dead, + &vc, &avc, &ac, &aac, + &lvc, &lavc, &lan, &laan) + if err != nil { + return int64(0), err + } + + logrus.WithFields(logrus.Fields{ + "table_name": relName, + "table_size": relSize, + "tuples_inserted": ins, + "tuples_updated": upd, + "tuples_deleted": del, + "tuples_live": live, + "tuples_dead": dead, + "vacuum_count": vc, + "autovacuum_count": avc, + "last_vacuum": lvc, + "last_autovacuum": lavc, + "analyze_count": ac, + "autoanalyze_count": aac, + "last_analyze": lan, + "last_autoanalyze": laan, + }).Info("Vacuum and analyze stats for table") + } + } + if rows.Err() != nil { + return int64(0), rows.Err() + } + return deleted, nil + +} + +func DBCleanup(ctx context.Context, dbURL string, dryRun bool, ClonesRetentionMonths int) error { + db, err := newDB(ctx, dbURL) + if err != nil { + return err + } + + _, err = db.LogVacuumStats(ctx) + if err != nil { + logrus.Errorf("Error running vacuum stats: %v", err) + } + + var rowsClones int64 + var rows int64 + + emailRetentionDate := time.Now().AddDate(0, ClonesRetentionMonths*-1, 0) + + for { + select { + case <-ctx.Done(): + err = ctx.Err() + logrus.Errorf("Context cancelled DBCleanup: %v", err) + return err + default: + // continue execution outside of select + // so `break` works as expected + } + if dryRun { + rowsClones, err = db.ExpiredClonesCount(ctx, emailRetentionDate) + if err != nil { + logrus.Warningf("Error querying expired clones: %v", err) + } + + rows, err = db.ExpiredComposesCount(ctx, emailRetentionDate) + if err != nil { + logrus.Warningf("Error querying expired composes: %v", err) + } + logrus.Infof("Dryrun, expired composes count: %d (affecting %d clones)", rows, rowsClones) + break + } + + rows, err = db.DeleteComposes(ctx, emailRetentionDate) + if err != nil { + logrus.Errorf("Error deleting composes: %v, %d rows affected", rows, err) + return err + } + + err = db.VacuumAnalyze(ctx) + if err != nil { + logrus.Errorf("Error running vacuum analyze: %v", err) + return err + } + + if rows == 0 { + break + } + + logrus.Infof("Deleted results for %d", rows) + } + + _, err = db.LogVacuumStats(ctx) + if err != nil { + logrus.Errorf("Error running vacuum stats: %v", err) + } + + return nil +} diff --git a/cmd/image-builder-maintenance/db_test.go b/cmd/image-builder-maintenance/db_test.go new file mode 100644 index 000000000..97d4c9d90 --- /dev/null +++ b/cmd/image-builder-maintenance/db_test.go @@ -0,0 +1,184 @@ +//go:build dbtests + +package main + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/osbuild/image-builder/internal/db" + "github.com/osbuild/image-builder/internal/tutils" + "github.com/stretchr/testify/require" +) + +const ( + ANR1 = "000001" + ANR2 = "000002" + ANR3 = "000003" + + ORGID1 = "100000" + ORGID2 = "100001" + ORGID3 = "100002" + + EMAIL1 = "user1@test.test" + + fortnight = time.Hour * 24 * 14 +) + +// testExpireCompose testing expiration of compose only +// also tests vacuum +func testExpireCompose(ctx context.Context, t *testing.T) { + connStr := tutils.ConnStr(t) + d, err := newDB(ctx, connStr) + require.NoError(t, err) + + dbClonesRetentionMonths := 5 + + alreadyExpiredTime := time.Now().AddDate(0, (dbClonesRetentionMonths+1)*-1, 0) + emailRetentionDate := time.Now().AddDate(0, dbClonesRetentionMonths*-1, 0) + + composeId := uuid.New() + insert := "INSERT INTO composes(job_id, request, created_at, account_number, org_id) VALUES ($1, $2, $3, $4, $5)" + _, err = d.Conn.Exec(ctx, insert, composeId, "{}", alreadyExpiredTime, ANR1, ORGID1) + require.NoError(t, err) + + require.NoError(t, d.VacuumAnalyze(ctx)) + deleted, err := d.LogVacuumStats(ctx) + require.NoError(t, err) + require.Equal(t, int64(0), deleted) + + rows, err := d.ExpiredComposesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(1), rows) + + rows, err = d.DeleteComposes(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(1), rows) + + // assure data to be flushed for the vacuum test to work + _, err = d.Conn.Exec(ctx, "CHECKPOINT") + require.NoError(t, err) + + rows, err = d.ExpiredComposesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(0), rows) + + require.NoError(t, d.VacuumAnalyze(ctx)) + _, err = d.LogVacuumStats(ctx) + //deleted, err = d.LogVacuumStats() + // skip check for now + // until we found out why this works locally but not in + // the github actions + //require.Equal(t, int64(1), deleted) + require.NoError(t, err) +} + +func testExpireByCallingDBCleanup(ctx context.Context, t *testing.T) { + connStr := tutils.ConnStr(t) + d, err := newDB(ctx, connStr) + require.NoError(t, err) + + internalDB, err := db.InitDBConnectionPool(ctx, connStr) + require.NoError(t, err) + + dbClonesRetentionMonths := 5 + + notYetExpiredTime := time.Now() + alreadyExpiredTime := time.Now().AddDate(0, (dbClonesRetentionMonths+1)*-1, 0) + emailRetentionDate := time.Now().AddDate(0, dbClonesRetentionMonths*-1, 0) + + composeIdNotYetExpired := uuid.New() + insert := "INSERT INTO composes(job_id, request, created_at, account_number, org_id) VALUES ($1, $2, $3, $4, $5)" + _, err = d.Conn.Exec(ctx, insert, composeIdNotYetExpired, "{}", notYetExpiredTime, ANR1, ORGID1) + + composeIdExpired := uuid.New() + insert = "INSERT INTO composes(job_id, request, created_at, account_number, org_id) VALUES ($1, $2, $3, $4, $5)" + _, err = d.Conn.Exec(ctx, insert, composeIdExpired, "{}", alreadyExpiredTime, ANR1, ORGID1) + + cloneId := uuid.New() + require.NoError(t, internalDB.InsertClone(ctx, composeIdExpired, cloneId, []byte(` +{ + "region": "us-east-2" +} +`))) + + // two rows inserted, only one is expired + rows, err := d.ExpiredComposesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(1), rows) + + rows, err = d.ExpiredClonesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(1), rows) + + err = DBCleanup(ctx, connStr, false, dbClonesRetentionMonths) + require.NoError(t, err) + + rows, err = d.ExpiredComposesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(0), rows) + + rows, err = d.ExpiredClonesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(0), rows) +} + +// testVacuum test if no vacuum is performed on a clean database +func testVacuum(ctx context.Context, t *testing.T) { + d, err := newDB(ctx, tutils.ConnStr(t)) + require.NoError(t, err) + + require.NoError(t, d.VacuumAnalyze(ctx)) + deleted, err := d.LogVacuumStats(ctx) + require.NoError(t, err) + require.Equal(t, int64(0), deleted) +} + +func testDryRun(ctx context.Context, t *testing.T) { + connStr := tutils.ConnStr(t) + d, err := newDB(ctx, connStr) + require.NoError(t, err) + + dbClonesRetentionMonths := 5 + + alreadyExpiredTime := time.Now().AddDate(0, (dbClonesRetentionMonths+1)*-1, 0) + emailRetentionDate := time.Now().AddDate(0, dbClonesRetentionMonths*-1, 0) + + composeIdExpired := uuid.New() + insert := "INSERT INTO composes(job_id, request, created_at, account_number, org_id) VALUES ($1, $2, $3, $4, $5)" + _, err = d.Conn.Exec(ctx, insert, composeIdExpired, "{}", alreadyExpiredTime, ANR1, ORGID1) + + rows, err := d.ExpiredComposesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(1), rows) + + err = DBCleanup(ctx, connStr, true, dbClonesRetentionMonths) + require.NoError(t, err) + + // still there + rows, err = d.ExpiredComposesCount(ctx, emailRetentionDate) + require.NoError(t, err) + require.Equal(t, int64(1), rows) +} + +func TestAll(t *testing.T) { + ctx := context.Background() + fns := []func(context.Context, *testing.T){ + testExpireCompose, + testExpireByCallingDBCleanup, + testDryRun, + testVacuum, + } + + for _, f := range fns { + select { + case <-ctx.Done(): + require.NoError(t, ctx.Err()) + return + default: + tutils.RunTest(ctx, t, f) + } + } +} diff --git a/cmd/image-builder-maintenance/main.go b/cmd/image-builder-maintenance/main.go new file mode 100644 index 000000000..1490ef211 --- /dev/null +++ b/cmd/image-builder-maintenance/main.go @@ -0,0 +1,73 @@ +package main + +import ( + "context" + "fmt" + "os" + "os/signal" + "syscall" + "time" + + "github.com/sirupsen/logrus" +) + +func main() { + ctx, applicationCancel := context.WithCancel(context.Background()) + defer applicationCancel() + + terminationSignal := make(chan os.Signal, 1) + signal.Notify(terminationSignal, syscall.SIGTERM, syscall.SIGINT) + defer signal.Stop(terminationSignal) + + // Channel to track graceful application shutdown + shutdownSignal := make(chan struct{}) + + go func() { + select { + case <-terminationSignal: + fmt.Println("Received termination signal, cancelling context...") + applicationCancel() + case <-shutdownSignal: + // Normal application shutdown, no logging + } + }() + + ctx, cancelTimeout := context.WithTimeout(ctx, 2*time.Hour) + defer cancelTimeout() + + logrus.SetReportCaller(true) + + conf := Config{ + DryRun: true, + EnableDBMaintenance: false, + ClonesRetentionMonths: 24, + } + + err := LoadConfigFromEnv(&conf) + if err != nil { + logrus.Fatal(err) + } + + if conf.DryRun { + logrus.Info("Dry run, no state will be changed") + } + + if !conf.EnableDBMaintenance { + logrus.Info("🦀🦀🦀 DB maintenance not enabled, skipping 🦀🦀🦀") + return + } + dbURL := fmt.Sprintf("postgres://%s:%s@%s:%s/%s?sslmode=%s", + conf.PGUser, + conf.PGPassword, + conf.PGHost, + conf.PGPort, + conf.PGDatabase, + conf.PGSSLMode, + ) + err = DBCleanup(ctx, dbURL, conf.DryRun, conf.ClonesRetentionMonths) + if err != nil { + logrus.Fatalf("Error during DBCleanup: %v", err) + } + logrus.Info("🦀🦀🦀 dbqueue cleanup done 🦀🦀🦀") + close(shutdownSignal) +} diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index 274b2f57b..ee6e10d7a 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "errors" "fmt" "runtime/debug" @@ -56,6 +57,8 @@ func main() { PGSSLMode: "prefer", } + ctx := context.Background() + err := config.LoadConfigFromEnv(&conf) if err != nil { panic(err) @@ -113,7 +116,7 @@ func main() { } connStr := fmt.Sprintf("postgres://%s:%s@%s:%s/%s?sslmode=%s", conf.PGUser, conf.PGPassword, conf.PGHost, conf.PGPort, conf.PGDatabase, conf.PGSSLMode) - dbase, err := db.InitDBConnectionPool(connStr) + dbase, err := db.InitDBConnectionPool(ctx, connStr) if err != nil { panic(err) } diff --git a/distribution/Dockerfile-ubi b/distribution/Dockerfile-ubi index 52ebdf713..e3c07446b 100644 --- a/distribution/Dockerfile-ubi +++ b/distribution/Dockerfile-ubi @@ -18,6 +18,7 @@ RUN mkdir /app RUN mkdir -p "/opt/migrate/" COPY --from=builder /opt/app-root/src/go/bin/image-builder /app/ COPY --from=builder /opt/app-root/src/go/bin/image-builder-migrate-db-tern /app/ +COPY --from=builder /opt/app-root/src/go/bin/image-builder-maintenance /app/ COPY ./distributions /app/distributions COPY ./internal/db/migrations-tern /app/migrations COPY ./distribution/openshift-startup.sh /opt/openshift-startup.sh diff --git a/internal/db/db.go b/internal/db/db.go index 4e7d83ab6..c0aee2166 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -157,7 +157,7 @@ const ( WHERE composes.org_id=$2)` ) -func InitDBConnectionPool(connStr string) (DB, error) { +func InitDBConnectionPool(ctx context.Context, connStr string) (DB, error) { dbConfig, err := pgxpool.ParseConfig(connStr) if err != nil { return nil, err @@ -165,7 +165,7 @@ func InitDBConnectionPool(connStr string) (DB, error) { dbConfig.ConnConfig.Tracer = &dbTracer{} - pool, err := pgxpool.NewWithConfig(context.Background(), dbConfig) + pool, err := pgxpool.NewWithConfig(ctx, dbConfig) if err != nil { return nil, err } diff --git a/internal/tutils/db_tutils.go b/internal/tutils/db_tutils.go new file mode 100644 index 000000000..8b3b14f1d --- /dev/null +++ b/internal/tutils/db_tutils.go @@ -0,0 +1,79 @@ +//go:build dbtests + +package tutils + +import ( + "context" + "fmt" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/osbuild/image-builder/internal/config" + "github.com/stretchr/testify/require" +) + +func conf(t *testing.T) *config.ImageBuilderConfig { + c := config.ImageBuilderConfig{ + ListenAddress: "unused", + LogLevel: "INFO", + TernMigrationsDir: "/usr/share/image-builder/migrations-tern", + PGHost: "localhost", + PGPort: "5432", + PGDatabase: "imagebuilder", + PGSSLMode: "disable", + } + + err := config.LoadConfigFromEnv(&c) + require.NoError(t, err) + + return &c +} + +func ConnStr(t *testing.T) string { + c := conf(t) + return fmt.Sprintf("postgres://%s:%s@%s:%s/%s?sslmode=%s", c.PGUser, c.PGPassword, c.PGHost, c.PGPort, c.PGDatabase, c.PGSSLMode) +} + +func Connect(t *testing.T) *pgx.Conn { + ctx := context.Background() + conn, err := pgx.Connect(ctx, ConnStr(t)) + require.NoError(t, err) + return conn +} + +func TearDown(ctx context.Context, t *testing.T) { + conn := Connect(t) + defer conn.Close(ctx) + _, err := conn.Exec(ctx, "drop schema public cascade") + require.NoError(t, err) + _, err = conn.Exec(ctx, "create schema public") + require.NoError(t, err) + _, err = conn.Exec(ctx, "grant all on schema public to postgres") + require.NoError(t, err) + _, err = conn.Exec(ctx, "grant all on schema public to public") + require.NoError(t, err) +} + +func MigrateTern(ctx context.Context, t *testing.T) { + // run tern migration on top of existing db + c := conf(t) + + output, err := callTernMigrate( + ctx, + TernMigrateOptions{ + MigrationsDir: c.TernMigrationsDir, + DBName: c.PGDatabase, + Hostname: c.PGHost, + DBPort: c.PGPort, + DBUser: c.PGUser, + DBPassword: c.PGPassword, + SSLMode: c.PGSSLMode, + }) + require.NoErrorf(t, err, "tern command failed with non-zero code, combined output: %s", string(output)) +} + +func RunTest(ctx context.Context, t *testing.T, f func(context.Context, *testing.T)) { + MigrateTern(ctx, t) + defer TearDown(ctx, t) + f(ctx, t) +} diff --git a/internal/tutils/psql.go b/internal/tutils/psql.go index 8f88b07dd..501b77f3b 100644 --- a/internal/tutils/psql.go +++ b/internal/tutils/psql.go @@ -1,6 +1,7 @@ package tutils import ( + "context" "fmt" "math/rand" "os/exec" @@ -110,27 +111,61 @@ func (p *PSQLContainer) Stop() error { return err } -func (p *PSQLContainer) NewDB() (db.DB, error) { +type TernMigrateOptions struct { + MigrationsDir string + Hostname string + DBName string + DBPort string + DBUser string + DBPassword string + SSLMode string +} + +func callTernMigrate(ctx context.Context, opt TernMigrateOptions) ([]byte, error) { + args := []string{ + "migrate", + } + + addArg := func(flag, value string) { + if value != "" { + args = append(args, flag, value) + } + } + + addArg("--migrations", opt.MigrationsDir) + addArg("--host", opt.Hostname) + addArg("--database", opt.DBName) + addArg("--port", opt.DBPort) + addArg("--user", opt.DBUser) + addArg("--password", opt.DBPassword) + addArg("--sslmode", opt.SSLMode) + + /* #nosec G204 */ + cmd := exec.CommandContext(ctx, "tern", args...) + + return cmd.CombinedOutput() +} + +func (p *PSQLContainer) NewDB(ctx context.Context) (db.DB, error) { dbName := fmt.Sprintf("test%s", strings.Replace(uuid.New().String(), "-", "", -1)) _, err := p.execQuery("", fmt.Sprintf("CREATE DATABASE %s", dbName)) if err != nil { return nil, err } - /* #nosec G204 */ - cmd := exec.Command( - "tern", - "migrate", - "-m", "../db/migrations-tern/", - "--database", dbName, - "--host", "localhost", - "--port", fmt.Sprintf("%d", p.port), - "--user", user, + out, err := callTernMigrate( + ctx, + TernMigrateOptions{ + MigrationsDir: "../db/migrations-tern/", + Hostname: "localhost", + DBName: dbName, + DBPort: fmt.Sprintf("%d", p.port), + DBUser: user, + }, ) - out, err := cmd.CombinedOutput() if err != nil { return nil, fmt.Errorf("tern command error: %w, output: %s", err, out) } - return db.InitDBConnectionPool(fmt.Sprintf("postgres://postgres@localhost:%d/%s", p.port, dbName)) + return db.InitDBConnectionPool(ctx, fmt.Sprintf("postgres://postgres@localhost:%d/%s", p.port, dbName)) } diff --git a/internal/v1/handler_blueprints_test.go b/internal/v1/handler_blueprints_test.go index deb46480f..957991e8d 100644 --- a/internal/v1/handler_blueprints_test.go +++ b/internal/v1/handler_blueprints_test.go @@ -34,7 +34,7 @@ type BlueprintExportResponseUnmarshal struct { } func makeTestServer(t *testing.T, apiSrv *string, csSrv *string) (dbase db.DB, srvURL string, shutdown func()) { - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(context.Background()) require.NoError(t, err) db_srv, tokenSrv := startServer(t, &testServerClientsConf{ @@ -542,7 +542,7 @@ func TestHandlers_ComposeBlueprint(t *testing.T) { })) defer apiSrv.Close() - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) srv, tokenSrv := startServer(t, &testServerClientsConf{ComposerURL: apiSrv.URL}, &ServerConfig{ DBase: dbase, diff --git a/internal/v1/handler_get_compose_status_test.go b/internal/v1/handler_get_compose_status_test.go index d90e093f2..43268084a 100644 --- a/internal/v1/handler_get_compose_status_test.go +++ b/internal/v1/handler_get_compose_status_test.go @@ -34,7 +34,7 @@ func TestComposeStatus(t *testing.T) { })) defer apiSrv.Close() - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) cr := ComposeRequest{ Distribution: "rhel-9", diff --git a/internal/v1/handler_post_compose_test.go b/internal/v1/handler_post_compose_test.go index 14a49bf72..d5640cf8f 100644 --- a/internal/v1/handler_post_compose_test.go +++ b/internal/v1/handler_post_compose_test.go @@ -403,7 +403,7 @@ func TestComposeStatusError(t *testing.T) { })) defer apiSrv.Close() - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) imageName := "MyImageName" clientId := "ui" diff --git a/internal/v1/handler_test.go b/internal/v1/handler_test.go index 650ee4ef0..b05b7ac1c 100644 --- a/internal/v1/handler_test.go +++ b/internal/v1/handler_test.go @@ -164,7 +164,7 @@ func TestGetComposeStatusErrorResponse(t *testing.T) { require.NoError(t, err) })) - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) cr := ComposeRequest{ Distribution: "rhel-9", @@ -267,7 +267,7 @@ func TestGetComposeMetadata(t *testing.T) { })) defer apiSrv.Close() - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) imageName := "MyImageName" clientId := "ui" @@ -331,7 +331,7 @@ func TestGetComposes(t *testing.T) { id5 := uuid.New() id6 := uuid.New() - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) db_srv, tokenSrv := startServer(t, &testServerClientsConf{}, &ServerConfig{ @@ -529,7 +529,7 @@ func TestGetClones(t *testing.T) { })) defer provSrv.Close() - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) err = dbase.InsertCompose(ctx, id, "500000", "user500000@test.test", "000000", nil, json.RawMessage(` { @@ -622,7 +622,7 @@ func TestGetCloneStatus(t *testing.T) { })) defer apiSrv.Close() - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) err = dbase.InsertCompose(ctx, id, "500000", "user500000@test.test", "000000", nil, json.RawMessage(` { diff --git a/internal/v1/server_test.go b/internal/v1/server_test.go index 7597ffffe..1afaee6de 100644 --- a/internal/v1/server_test.go +++ b/internal/v1/server_test.go @@ -1,6 +1,7 @@ package v1 import ( + "context" "encoding/json" "errors" "fmt" @@ -113,6 +114,8 @@ type testServer struct { } func startServer(t *testing.T, tscc *testServerClientsConf, conf *ServerConfig) (*testServer, *httptest.Server) { + ctx := context.Background() + var log = &logrus.Logger{ Out: os.Stderr, Formatter: new(logrus.TextFormatter), @@ -179,7 +182,7 @@ func startServer(t *testing.T, tscc *testServerClientsConf, conf *ServerConfig) } if serverConfig.DBase == nil { - dbase, err := dbc.NewDB() + dbase, err := dbc.NewDB(ctx) require.NoError(t, err) serverConfig.DBase = dbase } diff --git a/templates/maintenance.yml b/templates/maintenance.yml new file mode 100644 index 000000000..4f83affb5 --- /dev/null +++ b/templates/maintenance.yml @@ -0,0 +1,127 @@ +apiVersion: template.openshift.io/v1 +kind: Template +metadata: + name: image-builder-maintenance + annotations: + openshift.io/display-name: Image Builder maintenance + description: | + Cronjob related to maintaining the image-builder database. + tags: golang + iconClass: icon-shadowman + template.openshift.io/provider-display-name: Red Hat, Inc. +labels: + template: image-builder-maintenance +objects: +- apiVersion: batch/v1 + kind: CronJob + metadata: + labels: + service: image-builder + name: image-builder-maintenance + spec: + # run maintenance job at midnight on Tuesdays + schedule: 0 0 * * 2 + concurrencyPolicy: Forbid + # don't run if the job doesn't get scheduled within 30 minutes + startingDeadlineSeconds: 1800 + jobTemplate: + spec: + template: + spec: + serviceAccountName: image-builder-maintenance + restartPolicy: Never + containers: + - image: "${IMAGE_NAME}:${IMAGE_TAG}" + name: image-builder-maintenance + command: ["/app/image-builder-maintenance"] + resources: + requests: + cpu: "${CPU_REQUEST}" + memory: "${MEMORY_REQUEST}" + limits: + cpu: "${CPU_LIMIT}" + memory: "${MEMORY_LIMIT}" + env: + - name: PGHOST + valueFrom: + secretKeyRef: + name: image-builder-db + key: db.host + optional: true + - name: PGPORT + valueFrom: + secretKeyRef: + name: image-builder-db + key: db.port + optional: true + - name: PGDATABASE + valueFrom: + secretKeyRef: + name: image-builder-db + key: db.name + optional: true + - name: PGUSER + valueFrom: + secretKeyRef: + name: image-builder-db + key: db.user + optional: true + - name: PGPASSWORD + valueFrom: + secretKeyRef: + name: image-builder-db + key: db.password + optional: true + - name: PGSSLMODE + value: "${PGSSLMODE}" + - name: DRY_RUN + value: "${MAINTENANCE_DRY_RUN}" + - name: ENABLE_DB_MAINTENANCE + value: "${ENABLE_DB_MAINTENANCE}" + - name: DB_CLONES_RETENTION_MONTHS + value: "${DB_CLONES_RETENTION_MONTHS}" + +- apiVersion: v1 + kind: ServiceAccount + metadata: + name: image-builder-maintenance + imagePullSecrets: + - name: quay.io + +parameters: + - description: maintenance image name + name: IMAGE_NAME + value: quay.io/cloudservices/image-builder + required: true + - description: composer image tag + name: IMAGE_TAG + required: true + - name: CPU_REQUEST + description: CPU request per container + value: "50m" + - name: CPU_LIMIT + description: CPU limit per container + value: "100m" + - name: MEMORY_REQUEST + description: Memory request per container + value: "128Mi" + - name: MEMORY_LIMIT + description: Memory limit per container + value: "512Mi" + - description: composer-maintenance dry run + name: MAINTENANCE_DRY_RUN + # don't change this value, overwrite it in app-interface for a specific namespace + value: "true" + required: true + - description: Enable DB maintenance + name: ENABLE_DB_MAINTENANCE + # don't change this value, overwrite it in app-interface for a specific namespace + value: "false" + required: true + - description: Retention period for entries in the "clones" table (in months) + name: DB_CLONES_RETENTION_MONTHS + required: false + - description: postgres sslmode to use when connecting to the db + name: PGSSLMODE + value: "require" + required: true diff --git a/tools/dbtest-entrypoint.sh b/tools/dbtest-entrypoint.sh index d0a4a3bdb..1f67274e9 100755 --- a/tools/dbtest-entrypoint.sh +++ b/tools/dbtest-entrypoint.sh @@ -1,7 +1,12 @@ #!/usr/bin/env bash +set -euxo pipefail + # compile and execute instead of a plain # "go test", to have the correct working directory -go test -c -tags=integration -o image-builder-db-test ./cmd/image-builder-db-test/ +go test -c -tags=dbtests -o image-builder-db-test ./cmd/image-builder-db-test/ ./image-builder-db-test + +go test -c -tags=dbtests -o image-builder-maintenance-test ./cmd/image-builder-maintenance/ +./image-builder-maintenance-test