Skip to content

Commit

Permalink
APPS-1412 Default values for server and logger (#286)
Browse files Browse the repository at this point in the history
* set default config and time.Duration for time in models

* update comments

* update tests

* update tests
  • Loading branch information
korotkov-aerospike authored Dec 19, 2024
1 parent 44c0c86 commit 26598d7
Show file tree
Hide file tree
Showing 19 changed files with 60 additions and 58 deletions.
4 changes: 2 additions & 2 deletions cmd/backup/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func startService(configFile string, remote bool) error {
)

// run HTTP server
err = runHTTPServer(ctx, config.ServiceConfig.HTTPServer, httpService)
err = runHTTPServer(ctx, config.ServiceConfig.GetHTTPServerOrDefault(), httpService)

// stop the scheduler
scheduler.Stop()
Expand All @@ -123,7 +123,7 @@ func startService(configFile string, remote bool) error {

func setDefaultLoggers(ctx context.Context, config *model.Config) *slog.Logger {
appLogger := slog.New(
util.LogHandler(config.ServiceConfig.Logger),
util.LogHandler(config.ServiceConfig.GetLoggerOrDefault()),
)
slog.SetDefault(appLogger)
logger.SetDefault(util.NewQuartzLogger(ctx))
Expand Down
2 changes: 1 addition & 1 deletion internal/server/configuration/configuration_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func readConfig(reader io.Reader, nsValidator aerospike.NamespaceValidator) (*mo
}
slog.Info("Service configuration:\n" + string(configBytes))

config := dto.NewConfigWithDefaultValues()
config := &dto.Config{}
if err := yaml.Unmarshal(configBytes, config); err != nil {
return nil, fmt.Errorf("failed to unmarshal configuration: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/server/handlers/config_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func testSeedNode() dto.SeedNode {

func testConfigCluster() *dto.AerospikeCluster {
label := "label"
timeout := int32(10)
timeout := 10
useAlternate := false
queueSize := 1
return &dto.AerospikeCluster{
Expand Down
5 changes: 2 additions & 3 deletions internal/server/handlers/config_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@ const (
)

func testConfigBackupPolicy() *dto.BackupPolicy {
testIn32 := int32(10)
testInt := 10
keepFiles := dto.KeepAll
return &dto.BackupPolicy{
RemoveFiles: &keepFiles,
Parallel: &testInt,
SocketTimeout: &testIn32,
TotalTimeout: &testIn32,
SocketTimeout: &testInt,
TotalTimeout: &testInt,
RetryPolicy: &dto.RetryPolicy{
BaseTimeout: 1,
Multiplier: 1,
Expand Down
11 changes: 5 additions & 6 deletions internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"log/slog"
"net/http"
"strings"
"time"

"github.com/aerospike/aerospike-backup-service/v2/internal/server/handlers"
"github.com/aerospike/aerospike-backup-service/v2/internal/server/middleware"
Expand All @@ -27,14 +26,14 @@ type HTTPServer struct {
// NewHTTPServer returns a new instance of HTTPServer.
func NewHTTPServer(serverConfig *model.HTTPServerConfig, h *handlers.Service) *HTTPServer {
addr := fmt.Sprintf("%s:%d", serverConfig.GetAddressOrDefault(), serverConfig.GetPortOrDefault())
httpTimeout := time.Duration(serverConfig.GetTimeout()) * time.Millisecond

rateLimiterConfig := serverConfig.GetRateOrDefault()
rateLimiter := util.NewIPRateLimiter(
rate.Limit(serverConfig.GetRateOrDefault().GetTpsOrDefault()),
serverConfig.GetRateOrDefault().GetSizeOrDefault(),
rate.Limit(rateLimiterConfig.GetTpsOrDefault()),
rateLimiterConfig.GetSizeOrDefault(),
)

whitelist := util.NewIPWhiteList(serverConfig.GetRateOrDefault().GetWhiteListOrDefault())
whitelist := util.NewIPWhiteList(rateLimiterConfig.GetWhiteListOrDefault())
mw := middleware.RateLimiter(rateLimiter, whitelist)

router := NewRouter(
Expand All @@ -46,7 +45,7 @@ func NewHTTPServer(serverConfig *model.HTTPServerConfig, h *handlers.Service) *H
return &HTTPServer{
server: &http.Server{
Addr: addr,
ReadHeaderTimeout: httpTimeout,
ReadHeaderTimeout: serverConfig.GetTimeoutOrDefault(),
Handler: router,
},
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/dto/aerospike_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type AerospikeCluster struct {
// The seed nodes details.
SeedNodes []SeedNode `yaml:"seed-nodes,omitempty" json:"seed-nodes,omitempty"`
// The connection timeout in milliseconds.
ConnTimeout *int32 `yaml:"conn-timeout,omitempty" json:"conn-timeout,omitempty" example:"5000"`
ConnTimeout *int `yaml:"conn-timeout,omitempty" json:"conn-timeout,omitempty" example:"5000"`
// Whether should use "services-alternate" instead of "services" in info request during cluster tending.
UseServicesAlternate *bool `yaml:"use-services-alternate,omitempty" json:"use-services-alternate,omitempty"`
// The authentication details to the Aerospike cluster.
Expand Down Expand Up @@ -78,7 +78,7 @@ func (a *AerospikeCluster) fromModel(m *model.AerospikeCluster, config *model.Co
for i, v := range m.SeedNodes {
a.SeedNodes[i].fromModel(v)
}
a.ConnTimeout = m.ConnTimeout
a.ConnTimeout = durationToMillis(m.ConnTimeout)
a.UseServicesAlternate = m.UseServicesAlternate
if m.Credentials != nil {
a.Credentials = &Credentials{}
Expand All @@ -100,7 +100,7 @@ func (a *AerospikeCluster) ToModel(config *model.Config) (*model.AerospikeCluste
return &model.AerospikeCluster{
ClusterLabel: a.ClusterLabel,
SeedNodes: a.seedNodesToModel(),
ConnTimeout: a.ConnTimeout,
ConnTimeout: millisToDuration(a.ConnTimeout),
UseServicesAlternate: a.UseServicesAlternate,
Credentials: credentials,
TLS: a.TLS.toModel(),
Expand Down
10 changes: 5 additions & 5 deletions pkg/dto/backup_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ type BackupPolicy struct {
Parallel *int `yaml:"parallel,omitempty" json:"parallel,omitempty" example:"1"`
// Socket timeout in milliseconds. Default is 10 seconds. If this value is 0, it is set to total-timeout.
// If both are 0, there is no socket idle time limit.
SocketTimeout *int32 `yaml:"socket-timeout,omitempty" json:"socket-timeout,omitempty" example:"1000"`
SocketTimeout *int `yaml:"socket-timeout,omitempty" json:"socket-timeout,omitempty" example:"1000"`
// Total socket timeout in milliseconds. Default is 0, that is, no timeout.
TotalTimeout *int32 `yaml:"total-timeout,omitempty" json:"total-timeout,omitempty" example:"2000"`
TotalTimeout *int `yaml:"total-timeout,omitempty" json:"total-timeout,omitempty" example:"2000"`
// RetryPolicy defines the configuration for retry attempts in case of failures.
// If nil, default policy is used.
RetryPolicy *RetryPolicy `yaml:"retry-policy,omitempty" json:"retry-policy,omitempty"`
Expand Down Expand Up @@ -133,19 +133,19 @@ func (p *BackupPolicy) ToModel() *model.BackupPolicy {
}
}

func millisToDuration(ms *int32) *time.Duration {
func millisToDuration(ms *int) *time.Duration {
if ms == nil {
return nil
}
duration := time.Duration(*ms) * time.Millisecond
return &duration
}

func durationToMillis(d *time.Duration) *int32 {
func durationToMillis(d *time.Duration) *int {
if d == nil {
return nil
}
ms := int32((*d) / time.Millisecond)
ms := int((*d) / time.Millisecond)
return &ms
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/dto/backup_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (

func TestBackupPolicyConversionIsLossless(t *testing.T) {
parallel := 4
socketTimeout := int32(5000)
totalTimeout := int32(10000)
socketTimeout := 5000
totalTimeout := 10000
retryPolicy := &RetryPolicy{MaxRetries: 3}
removeFiles := KeepAll
noRecords := true
Expand Down
8 changes: 0 additions & 8 deletions pkg/dto/backup_service_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@ type BackupServiceConfig struct {
Logger *LoggerConfig `yaml:"logger,omitempty" json:"logger,omitempty"`
}

// NewBackupServiceConfigWithDefaultValues returns a new BackupServiceConfig with default values.
func NewBackupServiceConfigWithDefaultValues() BackupServiceConfig {
return BackupServiceConfig{
HTTPServer: &HTTPServerConfig{},
Logger: &LoggerConfig{},
}
}

func (b *BackupServiceConfig) ToModel() *model.BackupServiceConfig {
return &model.BackupServiceConfig{
HTTPServer: b.HTTPServer.ToModel(),
Expand Down
11 changes: 0 additions & 11 deletions pkg/dto/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,6 @@ func (c *Config) validate() error {
return nil
}

// NewConfigWithDefaultValues returns a new Config with default values.
func NewConfigWithDefaultValues() *Config {
return &Config{
ServiceConfig: NewBackupServiceConfigWithDefaultValues(),
Storage: map[string]*Storage{},
BackupRoutines: map[string]*BackupRoutine{},
BackupPolicies: map[string]*BackupPolicy{},
AerospikeClusters: map[string]*AerospikeCluster{},
}
}

func (c *Config) ToModel(nsValidator aerospike.NamespaceValidator) (*model.Config, error) {
if err := c.validate(); err != nil {
return nil, fmt.Errorf("configuration validation failed: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/dto/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

func validConfig() *Config {
return &Config{
ServiceConfig: NewBackupServiceConfigWithDefaultValues(),
ServiceConfig: BackupServiceConfig{},
BackupRoutines: map[string]*BackupRoutine{
"routine1": {
SourceCluster: "cluster1",
Expand Down
7 changes: 6 additions & 1 deletion pkg/dto/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/aerospike/aerospike-backup-service/v2/pkg/service/aerospike"
"github.com/aerospike/aerospike-backup-service/v2/pkg/util"
saClient "github.com/aerospike/backup-go/pkg/secret-agent"
"github.com/aws/smithy-go/ptr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand All @@ -18,7 +19,11 @@ func TestConfigModelConversionIsLossless(t *testing.T) {
}

originalConfig := &Config{
ServiceConfig: NewBackupServiceConfigWithDefaultValues(),
ServiceConfig: BackupServiceConfig{
HTTPServer: &HTTPServerConfig{
Address: ptr.String("localhost"),
},
},
AerospikeClusters: map[string]*AerospikeCluster{
"cluster1": {
SeedNodes: []SeedNode{{HostName: "host", Port: 80}},
Expand Down
4 changes: 2 additions & 2 deletions pkg/dto/http_server_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (s *HTTPServerConfig) ToModel() *model.HTTPServerConfig {
Port: s.Port,
Rate: s.Rate.ToModel(),
ContextPath: s.ContextPath,
Timeout: s.Timeout,
Timeout: millisToDuration(s.Timeout),
}
}

Expand All @@ -63,7 +63,7 @@ func (s *HTTPServerConfig) fromModel(m *model.HTTPServerConfig) {
s.Rate.fromModel(m.Rate)
}
s.ContextPath = m.ContextPath
s.Timeout = m.Timeout
s.Timeout = durationToMillis(m.Timeout)
}

// Compare HTTPServerConfig object with another and return detailed errors.
Expand Down
4 changes: 2 additions & 2 deletions pkg/dto/restore_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ type RestorePolicy struct {
// Timeout (ms) for Aerospike commands to write records, create indexes and create UDFs.
// Socket timeout in milliseconds. Default is 10 seconds. If this value is 0, it is set to total-timeout.
// If both are 0, there is no socket idle time limit.
SocketTimeout *int32 `yaml:"socket-timeout,omitempty" json:"socket-timeout,omitempty" example:"1000"`
SocketTimeout *int `yaml:"socket-timeout,omitempty" json:"socket-timeout,omitempty" example:"1000"`
// Total socket timeout in milliseconds. Default is 0, that is, no timeout.
TotalTimeout *int32 `yaml:"total-timeout,omitempty" json:"total-timeout,omitempty" example:"2000"`
TotalTimeout *int `yaml:"total-timeout,omitempty" json:"total-timeout,omitempty" example:"2000"`
// Disables the use of batch writes when restoring records to the Aerospike cluster.
// By default, the cluster is checked for batch write support.
DisableBatchWrites *bool `json:"disable-batch-writes,omitempty"`
Expand Down
6 changes: 3 additions & 3 deletions pkg/model/aerospike_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ type AerospikeCluster struct {
ClusterLabel *string
// The seed nodes details.
SeedNodes []SeedNode
// The connection timeout in milliseconds.
ConnTimeout *int32
// The connection timeout.
ConnTimeout *time.Duration
// Whether should use "services-alternate" instead of "services" in info request during cluster tending.
UseServicesAlternate *bool
// The authentication details to the Aerospike cluster.
Expand Down Expand Up @@ -136,7 +136,7 @@ func (c *AerospikeCluster) ASClientPolicy() *as.ClientPolicy {
}
}
if c.ConnTimeout != nil {
policy.Timeout = time.Duration(*c.ConnTimeout) * time.Millisecond
policy.Timeout = *c.ConnTimeout
}
if c.UseServicesAlternate != nil {
policy.UseServicesAlternate = *c.UseServicesAlternate
Expand Down
4 changes: 2 additions & 2 deletions pkg/model/backup_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ type RemoveFilesType string
type BackupPolicy struct {
// Maximum number of scan calls to run in parallel.
Parallel *int
// Socket timeout in milliseconds. If this value is 0, it is set to total-timeout.
// Socket timeout. If this value is 0, it is set to total-timeout.
// If both are 0, there is no socket idle time limit.
SocketTimeout *time.Duration
// Total socket timeout in milliseconds. Default is 0, that is, no timeout.
// Total socket timeout. Default is 0, that is, no timeout.
TotalTimeout *time.Duration
// RetryPolicy defines the configuration for retry attempts in case of failures.
RetryPolicy *models.RetryPolicy
Expand Down
16 changes: 16 additions & 0 deletions pkg/model/backup_service_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,19 @@ type BackupServiceConfig struct {
// Logger is the backup service logger configuration.
Logger *LoggerConfig
}

func (c BackupServiceConfig) GetHTTPServerOrDefault() *HTTPServerConfig {
if c.HTTPServer != nil {
return c.HTTPServer
}

return &defaultConfig.http
}

func (c BackupServiceConfig) GetLoggerOrDefault() *LoggerConfig {
if c.Logger != nil {
return c.Logger
}

return &defaultConfig.logger
}
2 changes: 1 addition & 1 deletion pkg/model/config_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var defaultConfig = struct {
WhiteList: []string{},
},
ContextPath: util.Ptr("/"),
Timeout: util.Ptr(5000),
Timeout: util.Ptr(5 * time.Second),
},
logger: LoggerConfig{
Level: util.Ptr("DEBUG"),
Expand Down
10 changes: 6 additions & 4 deletions pkg/model/http_server_config.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package model

import "time"

// HTTPServerConfig represents the service's HTTP server configuration.
// @Description HTTPServerConfig represents the service's HTTP server configuration.
type HTTPServerConfig struct {
Expand All @@ -11,8 +13,8 @@ type HTTPServerConfig struct {
Rate *RateLimiterConfig
// ContextPath customizes path for the API endpoints.
ContextPath *string
// Timeout for http server operations in milliseconds.
Timeout *int
// Timeout for http server operations.
Timeout *time.Duration
}

// GetAddressOrDefault returns the value of the Address property.
Expand All @@ -33,9 +35,9 @@ func (s *HTTPServerConfig) GetPortOrDefault() int {
return *defaultConfig.http.Port
}

// GetTimeout returns the value of the Timeout property.
// GetTimeoutOrDefault returns the value of the Timeout property.
// If the property is not set, it returns the default value = 5s.
func (s *HTTPServerConfig) GetTimeout() int {
func (s *HTTPServerConfig) GetTimeoutOrDefault() time.Duration {
if s.Timeout != nil {
return *s.Timeout
}
Expand Down

0 comments on commit 26598d7

Please sign in to comment.