Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to slog #507

Merged
merged 12 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/docker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
password: ${{ secrets.DOCKER_REGISTRY_TOKEN }}

- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Setup Go
uses: actions/setup-go@v5
Expand All @@ -38,7 +38,7 @@ jobs:
cache: false

- name: Lint
uses: golangci/golangci-lint-action@v3
uses: golangci/golangci-lint-action@v4
with:
args: --build-tags integration -p bugs -p unused -D protogetter --timeout=5m

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test-unit:

.PHONY: test-integration
test-integration:
go test -tags=integration -timeout 600s -p 1 ./...
go test -v -tags=integration -timeout 600s -p 1 ./...

.PHONY: check-diff
check-diff: spec
Expand Down
5 changes: 3 additions & 2 deletions cmd/metal-api/internal/datastore/event.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package datastore

import (
"log/slog"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/fsm"
"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
"go.uber.org/zap"
)

// ListProvisioningEventContainers returns all machine provisioning event containers.
Expand Down Expand Up @@ -38,7 +39,7 @@ func (rs *RethinkStore) UpsertProvisioningEventContainer(ec *metal.ProvisioningE
return rs.upsertEntity(rs.eventTable(), ec)
}

func (rs *RethinkStore) ProvisioningEventForMachine(log *zap.SugaredLogger, event *metal.ProvisioningEvent, machineID string) (*metal.ProvisioningEventContainer, error) {
func (rs *RethinkStore) ProvisioningEventForMachine(log *slog.Logger, event *metal.ProvisioningEvent, machineID string) (*metal.ProvisioningEventContainer, error) {
ec, err := rs.FindProvisioningEventContainer(machineID)
if err != nil && !metal.IsNotFound(err) {
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions cmd/metal-api/internal/datastore/integer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package datastore
import (
"errors"
"fmt"
"log/slog"
"time"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
"go.uber.org/zap"

"github.com/avast/retry-go/v4"
r "gopkg.in/rethinkdb/rethinkdb-go.v6"
Expand Down Expand Up @@ -117,19 +117,19 @@ func (ip *IntegerPool) String() string {
// - releasing the integer is fast
// - you do not have gaps (because you can give the integers back to the pool)
// - everything can be done atomically, so there are no race conditions
func (ip *IntegerPool) initIntegerPool(log *zap.SugaredLogger) error {
func (ip *IntegerPool) initIntegerPool(log *slog.Logger) error {
var info integerinfo
err := ip.infoTable.ReadOne(&info, ip.session)
if err != nil && !errors.Is(err, r.ErrEmptyResult) {
return err
}

log.Infow("pool info", "id", ip.String(), "info", info)
log.Info("pool info", "id", ip.String(), "info", info)
if info.IsInitialized {
return nil
}

log.Infow("initializing integer pool", "for", ip.String(), "RangeMin", ip.min, "RangeMax", ip.max)
log.Info("initializing integer pool", "for", ip.String(), "RangeMin", ip.min, "RangeMax", ip.max)
intRange := makeRange(ip.min, ip.max)
_, err = ip.poolTable.Insert(intRange).RunWrite(ip.session, r.RunOpts{ArrayLimit: ip.max})
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ package datastore

import (
"context"
"log/slog"
"sync"
"time"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
"github.com/metal-stack/metal-api/test"
"go.uber.org/zap/zaptest"

"testing"

Expand All @@ -25,7 +25,7 @@ func TestRethinkStore_AcquireRandomUniqueIntegerIntegration(t *testing.T) {
_ = container.Terminate(context.Background())
}()

rs := New(zaptest.NewLogger(t).Sugar(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs := New(slog.Default(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs.VRFPoolRangeMin = 10000
rs.VRFPoolRangeMax = 10010
rs.ASNPoolRangeMin = 10000
Expand All @@ -50,7 +50,7 @@ func TestRethinkStore_AcquireUniqueIntegerTwiceIntegration(t *testing.T) {
_ = container.Terminate(context.Background())
}()

rs := New(zaptest.NewLogger(t).Sugar(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs := New(slog.Default(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs.VRFPoolRangeMin = 10000
rs.VRFPoolRangeMax = 10010
rs.ASNPoolRangeMin = 10000
Expand All @@ -77,7 +77,7 @@ func TestRethinkStore_AcquireUniqueIntegerPoolExhaustionIntegration(t *testing.T
_ = container.Terminate(context.Background())
}()

rs := New(zaptest.NewLogger(t).Sugar(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs := New(slog.Default(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs.VRFPoolRangeMin = 10000
rs.VRFPoolRangeMax = 10010
rs.ASNPoolRangeMin = 10000
Expand Down
2 changes: 1 addition & 1 deletion cmd/metal-api/internal/datastore/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ func (rs *RethinkStore) FindWaitingMachine(projectid, partitionid string, size m
for _, m := range candidates {
ec, ok := ecMap[m.ID]
if !ok {
rs.log.Errorw("cannot find machine provisioning event container", "machine", m, "error", err)
rs.log.Error("cannot find machine provisioning event container", "machine", m, "error", err)
// fall through, so the rest of the machines is getting evaluated
continue
}
Expand Down
16 changes: 8 additions & 8 deletions cmd/metal-api/internal/datastore/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,34 +113,34 @@ func (rs *RethinkStore) Migrate(targetVersion *int, dry bool) error {
}

if len(ms) == 0 {
rs.log.Infow("no database migration required", "current-version", current.Version)
rs.log.Info("no database migration required", "current-version", current.Version)
return nil
}

rs.log.Infow("database migration required", "current-version", current.Version, "newer-versions", len(ms), "target-version", ms[len(ms)-1].Version)
rs.log.Info("database migration required", "current-version", current.Version, "newer-versions", len(ms), "target-version", ms[len(ms)-1].Version)

if dry {
for _, m := range ms {
rs.log.Infow("database migration dry run", "version", m.Version, "name", m.Name)
rs.log.Info("database migration dry run", "version", m.Version, "name", m.Name)
}
return nil
}

rs.log.Infow("setting demoted runtime user to read only", "user", DemotedUser)
rs.log.Info("setting demoted runtime user to read only", "user", DemotedUser)
_, err = rs.db().Grant(DemotedUser, map[string]interface{}{"read": true, "write": false}).RunWrite(rs.session)
if err != nil {
return err
}
defer func() {
rs.log.Infow("removing read only", "user", DemotedUser)
rs.log.Info("removing read only", "user", DemotedUser)
_, err = rs.db().Grant(DemotedUser, map[string]interface{}{"read": true, "write": true}).RunWrite(rs.session)
if err != nil {
rs.log.Errorw("error giving back write permissions", "user", DemotedUser)
rs.log.Error("error giving back write permissions", "user", DemotedUser)
}
}()

for _, m := range ms {
rs.log.Infow("running database migration", "version", m.Version, "name", m.Name)
rs.log.Info("running database migration", "version", m.Version, "name", m.Name)
err = m.Up(rs.db(), rs.session, rs)
if err != nil {
return fmt.Errorf("error running database migration: %w", err)
Expand All @@ -154,7 +154,7 @@ func (rs *RethinkStore) Migrate(targetVersion *int, dry bool) error {
}
}

rs.log.Infow("database migration succeeded")
rs.log.Info("database migration succeeded")

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package migrations_integration

import (
"context"
"log/slog"
"time"

"github.com/google/go-cmp/cmp"
Expand All @@ -13,7 +14,6 @@ import (
_ "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore/migrations"
"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
"github.com/metal-stack/metal-api/test"
"go.uber.org/zap/zaptest"

"testing"

Expand All @@ -28,7 +28,7 @@ func Test_Migration(t *testing.T) {
_ = container.Terminate(context.Background())
}()

rs := datastore.New(zaptest.NewLogger(t).Sugar(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs := datastore.New(slog.Default(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs.VRFPoolRangeMin = 10000
rs.VRFPoolRangeMax = 10010
rs.ASNPoolRangeMin = 10000
Expand Down
10 changes: 5 additions & 5 deletions cmd/metal-api/internal/datastore/rethinkdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package datastore

import (
"fmt"
"log/slog"
"reflect"
"strings"
"time"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"

"go.uber.org/zap"
r "gopkg.in/rethinkdb/rethinkdb-go.v6"
)

Expand All @@ -25,7 +25,7 @@ var tables = []string{

// A RethinkStore is the database access layer for rethinkdb.
type RethinkStore struct {
log *zap.SugaredLogger
log *slog.Logger

session r.QueryExecutor
dbsession *r.Session
Expand All @@ -43,7 +43,7 @@ type RethinkStore struct {
}

// New creates a new rethink store.
func New(log *zap.SugaredLogger, dbhost string, dbname string, dbuser string, dbpass string) *RethinkStore {
func New(log *slog.Logger, dbhost string, dbname string, dbuser string, dbpass string) *RethinkStore {
return &RethinkStore{
log: log,
dbhost: dbhost,
Expand Down Expand Up @@ -296,11 +296,11 @@ func connect(hosts []string, dbname, user, pwd string) (*r.Session, error) {
// retryConnect infinitely tries to establish a database connection.
// in case a connection could not be established, the function will
// wait for a short period of time and try again.
func retryConnect(log *zap.SugaredLogger, hosts []string, dbname, user, pwd string) *r.Session {
func retryConnect(log *slog.Logger, hosts []string, dbname, user, pwd string) *r.Session {
tryAgain:
s, err := connect(hosts, dbname, user, pwd)
if err != nil {
log.Errorw("db connection error", "db", dbname, "hosts", hosts, "error", err)
log.Error("db connection error", "db", dbname, "hosts", hosts, "error", err)
time.Sleep(3 * time.Second)
goto tryAgain
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package datastore

import (
"context"
"log/slog"
"os"
"sort"

Expand All @@ -15,7 +16,6 @@ import (
"github.com/metal-stack/metal-lib/pkg/testcommon"
"github.com/stretchr/testify/require"
"github.com/testcontainers/testcontainers-go"
"go.uber.org/zap"

"testing"
)
Expand Down Expand Up @@ -45,7 +45,7 @@ func startRethinkInitialized() (container testcontainers.Container, ds *RethinkS
panic(err)
}

rs := New(zap.L().Sugar(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs := New(slog.Default(), c.IP+":"+c.Port, c.DB, c.User, c.Password)
rs.VRFPoolRangeMin = 10000
rs.VRFPoolRangeMax = 10010
rs.ASNPoolRangeMin = 10000
Expand Down
7 changes: 3 additions & 4 deletions cmd/metal-api/internal/datastore/rethinkdb_test.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
package datastore

import (
"log/slog"
"reflect"
"testing"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/testdata"
"go.uber.org/zap"
"go.uber.org/zap/zaptest"
r "gopkg.in/rethinkdb/rethinkdb-go.v6"
)

func TestNew(t *testing.T) {
logger := zaptest.NewLogger(t).Sugar()
logger := slog.Default()
type args struct {
log *zap.SugaredLogger
log *slog.Logger
dbhost string
dbname string
dbuser string
Expand Down
4 changes: 2 additions & 2 deletions cmd/metal-api/internal/datastore/testing.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package datastore

import (
"log/slog"
"testing"

"go.uber.org/zap/zaptest"
r "gopkg.in/rethinkdb/rethinkdb-go.v6"
)

Expand All @@ -20,7 +20,7 @@ Return Values:
*/
func InitMockDB(t *testing.T) (*RethinkStore, *r.Mock) {
rs := New(
zaptest.NewLogger(t).Sugar(),
slog.Default(),
"db-addr",
"mockdb",
"db-user",
Expand Down
Loading