From cdd7a2cfd2c5eb1ebd1656352374d8930044b115 Mon Sep 17 00:00:00 2001 From: Serge Zaitsev Date: Tue, 2 Sep 2025 17:24:30 +0200 Subject: [PATCH] Chore: Disable CGo in tests (#108764) * make cgo optional for sqlite * update go.mod; check error code differently * reduce api surface even more * move test errors into sqlite package * CGO_ENABLED=0 in unit tests * disable for enterprise, too * add driver name constant * remove unused constants * make test an integration one * try integration tests without cgo * implement error codes for modernc sqlite driver * typo fix * missing return * use error pointer as an interface * alias the driver * update workspace, check for test errors too * check error properly * add missing driver after rebase * fix missing import after rebase * debugging, lets try again * properly parse options, revert many previous changes * remove another log * better url parsing * revert test rename, leave it for later * revert reusedSession in unistore * revert more code * remove driver name * revert formatting * add integration test without cgo for sqlite * remove tracing and logging * bring driver alias back * fix type * wrong package --- .github/workflows/backend-unit-tests.yml | 4 +- .github/workflows/pr-test-integration.yml | 34 +++++- pkg/storage/unified/sql/db/service.go | 7 -- pkg/util/sqlite/sqlite_nocgo.go | 129 +++++++++++++++++++--- 4 files changed, 150 insertions(+), 24 deletions(-) diff --git a/.github/workflows/backend-unit-tests.yml b/.github/workflows/backend-unit-tests.yml index b81d8b4cc10..a0670dfc6bd 100644 --- a/.github/workflows/backend-unit-tests.yml +++ b/.github/workflows/backend-unit-tests.yml @@ -68,7 +68,7 @@ jobs: run: | set -euo pipefail readarray -t PACKAGES <<< "$(./scripts/ci/backend-tests/shard.sh -N"$SHARD")" - go test -short -timeout=30m "${PACKAGES[@]}" + CGO_ENABLED=0 go test -short -timeout=30m "${PACKAGES[@]}" grafana-enterprise: # Run this workflow for non-PR events (like pushes to `main` or `release-*`) OR for internal PRs (PRs not from forks) @@ -118,7 +118,7 @@ jobs: readarray -t PACKAGES <<< "$(./scripts/ci/backend-tests/shard.sh -N"$SHARD")" # This tee requires pipefail to be set, otherwise `go test`'s exit code is thrown away. # That means having no `-o pipefail` => failing tests => exit code 0, which is wrong. - go test -short -timeout=30m "${PACKAGES[@]}" + CGO_ENABLED=0 go test -short -timeout=30m "${PACKAGES[@]}" # This is the job that is actually required by rulesets. # We need to require EITHER the OSS or the Enterprise job to pass. diff --git a/.github/workflows/pr-test-integration.yml b/.github/workflows/pr-test-integration.yml index 555364efdcd..f54e5a77b37 100644 --- a/.github/workflows/pr-test-integration.yml +++ b/.github/workflows/pr-test-integration.yml @@ -37,7 +37,6 @@ jobs: uses: ./.github/actions/change-detection with: self: .github/workflows/pr-test-integration.yml - sqlite: needs: detect-changes if: needs.detect-changes.outputs.changed == 'true' @@ -70,6 +69,39 @@ jobs: set -euo pipefail readarray -t PACKAGES <<< "$(./scripts/ci/backend-tests/pkgs-with-tests-named.sh -b TestIntegration | ./scripts/ci/backend-tests/shard.sh -N"$SHARD" -d-)" go test -tags=sqlite -timeout=8m -run '^TestIntegration' "${PACKAGES[@]}" + + sqlite_nocgo: + needs: detect-changes + if: needs.detect-changes.outputs.changed == 'true' + strategy: + matrix: + # We don't need more than this since it has to wait for the other tests. + shard: [ + 1/4, 2/4, 3/4, 4/4, + ] + fail-fast: false + + name: Sqlite Without CGo (${{ matrix.shard }}) + runs-on: ubuntu-x64-large-io + permissions: + contents: read + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + persist-credentials: false + - name: Setup Go + uses: actions/setup-go@v5.5.0 + with: + go-version-file: go.mod + cache: true + - name: Run tests + env: + SHARD: ${{ matrix.shard }} + run: | + set -euo pipefail + readarray -t PACKAGES <<< "$(./scripts/ci/backend-tests/pkgs-with-tests-named.sh -b TestIntegration | ./scripts/ci/backend-tests/shard.sh -N"$SHARD" -d-)" + CGO_ENABLED=0 go test -tags=sqlite -timeout=8m -run '^TestIntegration' "${PACKAGES[@]}" mysql: needs: detect-changes if: needs.detect-changes.outputs.changed == 'true' diff --git a/pkg/storage/unified/sql/db/service.go b/pkg/storage/unified/sql/db/service.go index 4d9a3f2479f..71f72d6a32a 100755 --- a/pkg/storage/unified/sql/db/service.go +++ b/pkg/storage/unified/sql/db/service.go @@ -12,13 +12,6 @@ import ( //go:generate mockery --with-expecter --name Rows //go:generate mockery --with-expecter --exported --name result -const ( - DriverPostgres = "postgres" - DriverMySQL = "mysql" - DriverSQLite = "sqlite" - DriverSQLite3 = "sqlite3" -) - // DBProvider provides access to a SQL Database. type DBProvider interface { // Init initializes the SQL Database, running migrations if needed. It is diff --git a/pkg/util/sqlite/sqlite_nocgo.go b/pkg/util/sqlite/sqlite_nocgo.go index 28afe0f985f..ec77078ea20 100644 --- a/pkg/util/sqlite/sqlite_nocgo.go +++ b/pkg/util/sqlite/sqlite_nocgo.go @@ -4,12 +4,17 @@ package sqlite import ( "database/sql" + "database/sql/driver" "errors" + "fmt" + "net/url" + "strings" "modernc.org/sqlite" + sqlite3 "modernc.org/sqlite/lib" ) -const DriverName = "sqlite" +type Driver = sqlite.Driver // The errors below are used in tests to simulate specific SQLite errors. It's a temporary solution // until we rewrite the tests not to depend on the sqlite3 package internals directly. @@ -20,26 +25,122 @@ var ( TestErrLocked = errors.New("database is locked (simulated)") ) -func init() { - // alias the driver name to match the CGo driver - sql.Register("sqlite3", &Driver{}) +var dsnAlias = map[string]string{ + "_vacuum": "_auto_vacuum", + "_timeout": "_busy_timeout", + "_cslike": "_case_sensitive_like", + "_defer_fk": "_defer_foreign_keys", + "_fk": "_foreign_keys", + "_journal": "_journal_mode", + "_locking": "_locking_mode", + "_rt": "_recursive_triggers", + "_sync": "_synchronous", } -// -// FIXME (@zserge) -// -// This non-CGo "implementation" is merely a stub to make Grafana compile without CGo. -// Any attempts to actually use this driver are likely to fail at runtime in the most brutal ways. -// +var dsnMapping = map[string]string{ + "cache": "", // unsupported + "mode": "", // unsupported + "_journal_mode": "_pragma", + "_synchronous": "_pragma", + "_locking_mode": "_pragma", + "_busy_timeout": "_pragma", + "_foreign_keys": "_pragma", + "_auto_vacuum": "_pragma", + "_cache_size": "_pragma", + "_case_sensitive_like": "_pragma", + "_defer_foreign_keys": "_pragma", + "_temp_store": "_pragma", + "_secure_delete": "_pragma", + "_txlock": "_txlock", + "_time_format": "_time_format", +} -type Driver = sqlite.Driver +func convertSQLite3URL(dsn string) (string, error) { + pos := strings.IndexRune(dsn, '?') + if pos < 1 { + return dsn, nil // no parameters to convert + } + params, err := url.ParseQuery(dsn[pos+1:]) + if err != nil { + return "", err + } + newDSN := dsn[:pos] + + q := url.Values{} + q.Add("_pragma", "busy_timeout(5000)") + + for key, values := range params { + if alias, ok := dsnAlias[strings.ToLower(key)]; ok { + key = alias + } + mapped, ok := dsnMapping[key] + if !ok || len(values) == 0 { + continue + } + value := values[0] + switch mapped { + case "_pragma": + value = strings.TrimPrefix(value, "_") + q.Add("_pragma", fmt.Sprintf("%s(%s)", key, value)) + case "_txlock": + q.Set("_txlock", value) + case "_time_format": + q.Set("_time_format", value) + } + } + if len(q) > 0 { + newDSN += "?" + q.Encode() + } + return newDSN, nil +} + +// moderncDriver is a wrapper for modernc.org/sqlite driver to convert DSN. +type moderncDriver struct { + driver.Driver +} + +// Open converts a dsn from sqlite3 to modernc.org/sqlite format and opens a connection. +func (d *moderncDriver) Open(name string) (driver.Conn, error) { + convertedName, err := convertSQLite3URL(name) + if err != nil { + return nil, err + } + return d.Driver.Open(convertedName) +} + +func init() { + sql.Register("sqlite3", &moderncDriver{Driver: &Driver{}}) +} func IsBusyOrLocked(err error) bool { - return false // FIXME + var sqliteErr *sqlite.Error + if errors.As(err, &sqliteErr) { + // Code is 32-bit number, low 8 bits are the SQLite error code, high 24 bits are extended code. + code := sqliteErr.Code() & 0xff + return code == sqlite3.SQLITE_BUSY || code == sqlite3.SQLITE_LOCKED + } + if errors.Is(err, TestErrBusy) || errors.Is(err, TestErrLocked) { + return true + } + return false } + func IsUniqueConstraintViolation(err error) bool { - return false // FIXME + var sqliteErr *sqlite.Error + if errors.As(err, &sqliteErr) { + // These constants are extended codes combined with primary code, so we can check them directly. + return sqliteErr.Code() == sqlite3.SQLITE_CONSTRAINT_PRIMARYKEY || sqliteErr.Code() == sqlite3.SQLITE_CONSTRAINT_UNIQUE + } + if errors.Is(err, TestErrUniqueConstraintViolation) { + return true + } + return false } + func ErrorMessage(err error) string { - return "" // FIXME + var sqliteErr *sqlite.Error + if errors.As(err, &sqliteErr) { + return sqliteErr.Error() + } + return "" }