Skip to content

Commit

Permalink
Replace custom Redis config struct with go-redis UniversalOptions
Browse files Browse the repository at this point in the history
  • Loading branch information
andsens committed Mar 19, 2024
1 parent 3cb985c commit 89a7cc5
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 101 deletions.
9 changes: 4 additions & 5 deletions cmd/registry/config-cache.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ http:
headers:
X-Content-Type-Options: [nosniff]
redis:
addr: localhost:6379
pool:
maxidle: 16
maxactive: 64
idletimeout: 300s
addrs: [localhost:6379]
maxidleconns: 16
poolsize: 64
connmaxidletime: 300s
dialtimeout: 10ms
readtimeout: 10ms
writetimeout: 10ms
Expand Down
42 changes: 3 additions & 39 deletions configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"reflect"
"strings"
"time"

"github.com/redis/go-redis/v9"
)

// Configuration is a versioned registry configuration, intended to be provided by a yaml file, and
Expand Down Expand Up @@ -173,7 +175,7 @@ type Configuration struct {
Notifications Notifications `yaml:"notifications,omitempty"`

// Redis configures the redis pool available to the registry webapp.
Redis Redis `yaml:"redis,omitempty"`
Redis redis.UniversalOptions `yaml:"redis,omitempty"`

Health Health `yaml:"health,omitempty"`
Catalog Catalog `yaml:"catalog,omitempty"`
Expand Down Expand Up @@ -277,44 +279,6 @@ type FileChecker struct {
Threshold int `yaml:"threshold,omitempty"`
}

// Redis configures the redis pool available to the registry webapp.
type Redis struct {
// Addr specifies the the redis instance available to the application.
Addr string `yaml:"addr,omitempty"`

// Usernames can be used as a finer-grained permission control since the introduction of the redis 6.0.
Username string `yaml:"username,omitempty"`

// Password string to use when making a connection.
Password string `yaml:"password,omitempty"`

// DB specifies the database to connect to on the redis instance.
DB int `yaml:"db,omitempty"`

// TLS configures settings for redis in-transit encryption
TLS struct {
Enabled bool `yaml:"enabled,omitempty"`
} `yaml:"tls,omitempty"`

DialTimeout time.Duration `yaml:"dialtimeout,omitempty"` // timeout for connect
ReadTimeout time.Duration `yaml:"readtimeout,omitempty"` // timeout for reads of data
WriteTimeout time.Duration `yaml:"writetimeout,omitempty"` // timeout for writes of data

// Pool configures the behavior of the redis connection pool.
Pool struct {
// MaxIdle sets the maximum number of idle connections.
MaxIdle int `yaml:"maxidle,omitempty"`

// MaxActive sets the maximum number of connections that should be
// opened before blocking a connection request.
MaxActive int `yaml:"maxactive,omitempty"`

// IdleTimeout sets the amount time to wait before closing
// inactive connections.
IdleTimeout time.Duration `yaml:"idletimeout,omitempty"`
} `yaml:"pool,omitempty"`
}

// HTTPChecker is a type of entry in the health section for checking HTTP URIs.
type HTTPChecker struct {
// Timeout is the duration to wait before timing out the HTTP request
Expand Down
30 changes: 12 additions & 18 deletions configuration/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/redis/go-redis/v9"
"github.com/stretchr/testify/suite"
"gopkg.in/yaml.v2"
)
Expand Down Expand Up @@ -130,20 +131,14 @@ var configStruct = Configuration{
Enabled: true,
},
},
Redis: Redis{
Addr: "localhost:6379",
Redis: redis.UniversalOptions{
Addrs: []string{"localhost:6379"},
Username: "alice",
Password: "123456",
DB: 1,
Pool: struct {
MaxIdle int `yaml:"maxidle,omitempty"`
MaxActive int `yaml:"maxactive,omitempty"`
IdleTimeout time.Duration `yaml:"idletimeout,omitempty"`
}{
MaxIdle: 16,
MaxActive: 64,
IdleTimeout: time.Second * 300,
},
MaxIdleConns: 16,
PoolSize: 64,
ConnMaxIdleTime: time.Second * 300,
DialTimeout: time.Millisecond * 10,
ReadTimeout: time.Millisecond * 10,
WriteTimeout: time.Millisecond * 10,
Expand Down Expand Up @@ -190,14 +185,13 @@ http:
headers:
X-Content-Type-Options: [nosniff]
redis:
addr: localhost:6379
addrs: [localhost:6379]
username: alice
password: 123456
db: 1
pool:
maxidle: 16
maxactive: 64
idletimeout: 300s
maxidleconns: 16
poolsize: 64
connmaxidletime: 300s
dialtimeout: 10ms
readtimeout: 10ms
writetimeout: 10ms
Expand Down Expand Up @@ -269,7 +263,7 @@ func (suite *ConfigSuite) TestParseSimple() {
func (suite *ConfigSuite) TestParseInmemory() {
suite.expectedConfig.Storage = Storage{"inmemory": Parameters{}}
suite.expectedConfig.Log.Fields = nil
suite.expectedConfig.Redis = Redis{}
suite.expectedConfig.Redis = redis.UniversalOptions{}

config, err := Parse(bytes.NewReader([]byte(inmemoryConfigYamlV0_1)))
suite.Require().NoError(err)
Expand All @@ -289,7 +283,7 @@ func (suite *ConfigSuite) TestParseIncomplete() {
suite.expectedConfig.Auth = Auth{"silly": Parameters{"realm": "silly"}}
suite.expectedConfig.Notifications = Notifications{}
suite.expectedConfig.HTTP.Headers = nil
suite.expectedConfig.Redis = Redis{}
suite.expectedConfig.Redis = redis.UniversalOptions{}

// Note: this also tests that REGISTRY_STORAGE and
// REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY can be used together
Expand Down
18 changes: 8 additions & 10 deletions docs/content/about/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -241,16 +241,15 @@ notifications:
actions:
- pull
redis:
addr: localhost:6379
addrs: [localhost:6379]
password: asecret
db: 0
dialtimeout: 10ms
readtimeout: 10ms
writetimeout: 10ms
pool:
maxidle: 16
maxactive: 64
idletimeout: 300s
maxidleconns: 16
poolsize: 64
connmaxidletime: 300s
tls:
enabled: false
health:
Expand Down Expand Up @@ -954,16 +953,15 @@ The `events` structure configures the information provided in event notification

```yaml
redis:
addr: localhost:6379
addrs: [localhost:6379]
password: asecret
db: 0
dialtimeout: 10ms
readtimeout: 10ms
writetimeout: 10ms
pool:
maxidle: 16
maxactive: 64
idletimeout: 300s
maxidleconns: 16
poolsize: 64
connmaxidletime: 300s
tls:
enabled: false
```
Expand Down
31 changes: 9 additions & 22 deletions registry/handlers/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ type App struct {
source notifications.SourceRecord
}

redis *redis.Client
redis redis.UniversalClient

// isCache is true if this registry is configured as a pull through cache
isCache bool
Expand Down Expand Up @@ -487,12 +487,12 @@ func (app *App) configureEvents(configuration *configuration.Configuration) {
}

func (app *App) configureRedis(cfg *configuration.Configuration) {
if cfg.Redis.Addr == "" {
if len(cfg.Redis.Addrs) == 0 {
dcontext.GetLogger(app).Infof("redis not configured")
return
}

app.redis = app.createPool(cfg.Redis)
app.redis = app.createPool(&cfg.Redis)

// Enable metrics instrumentation.
if err := redisotel.InstrumentMetrics(app.redis); err != nil {
Expand All @@ -514,25 +514,12 @@ func (app *App) configureRedis(cfg *configuration.Configuration) {
}))
}

func (app *App) createPool(cfg configuration.Redis) *redis.Client {
return redis.NewClient(&redis.Options{
Addr: cfg.Addr,
OnConnect: func(ctx context.Context, cn *redis.Conn) error {
res := cn.Ping(ctx)
return res.Err()
},
Username: cfg.Username,
Password: cfg.Password,
DB: cfg.DB,
MaxRetries: 3,
DialTimeout: cfg.DialTimeout,
ReadTimeout: cfg.ReadTimeout,
WriteTimeout: cfg.WriteTimeout,
PoolFIFO: false,
MaxIdleConns: cfg.Pool.MaxIdle,
PoolSize: cfg.Pool.MaxActive,
ConnMaxIdleTime: cfg.Pool.IdleTimeout,
})
func (app *App) createPool(cfg *redis.UniversalOptions) redis.UniversalClient {
cfg.OnConnect = func(ctx context.Context, cn *redis.Conn) error {
res := cn.Ping(ctx)
return res.Err()
};
return redis.NewUniversalClient(cfg);
}

// configureLogHook prepares logging hook parameters.
Expand Down
4 changes: 2 additions & 2 deletions registry/storage/cache/redis/redis.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
// Note that there is no implied relationship between these two caches. The
// layer may exist in one, both or none and the code must be written this way.
type redisBlobDescriptorService struct {
pool *redis.Client
pool redis.UniversalClient

// TODO(stevvooe): We use a pool because we don't have great control over
// the cache lifecycle to manage connections. A new connection if fetched
Expand All @@ -37,7 +37,7 @@ var _ distribution.BlobDescriptorService = &redisBlobDescriptorService{}

// NewRedisBlobDescriptorCacheProvider returns a new redis-based
// BlobDescriptorCacheProvider using the provided redis connection pool.
func NewRedisBlobDescriptorCacheProvider(pool *redis.Client) cache.BlobDescriptorCacheProvider {
func NewRedisBlobDescriptorCacheProvider(pool redis.UniversalClient) cache.BlobDescriptorCacheProvider {
return metrics.NewPrometheusCacheProvider(
&redisBlobDescriptorService{
pool: pool,
Expand Down
9 changes: 4 additions & 5 deletions tests/conf-e2e-cloud-storage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@ log:
formatter: text
level: debug
redis:
addr: redis:6379
addrs: [redis:6379]
db: 0
dialtimeout: 5s
readtimeout: 10ms
writetimeout: 10ms
pool:
idletimeout: 60s
maxactive: 64
maxidle: 16
maxidleconns: 16
poolsize: 64
connmaxidletime: 300s
storage:
redirect:
disable: true
Expand Down

0 comments on commit 89a7cc5

Please sign in to comment.