diff --git a/backends/aerospike.go b/backends/aerospike.go index e81e38e1..01f0e68c 100644 --- a/backends/aerospike.go +++ b/backends/aerospike.go @@ -53,34 +53,9 @@ type AerospikeBackend struct { // NewAerospikeBackend validates config.Aerospike and returns an AerospikeBackend func NewAerospikeBackend(cfg config.Aerospike, metrics *metrics.Metrics) *AerospikeBackend { - var hosts []*as.Host - - clientPolicy := as.NewClientPolicy() - // cfg.User and cfg.Password are optional parameters - // if left blank in the config, they will default to the empty - // string and be ignored - clientPolicy.User = cfg.User - clientPolicy.Password = cfg.Password - - // Aerospike's connection idle deadline default is 55 seconds. If greater than zero, this - // value will override - if cfg.ConnIdleTimeoutSecs > 0 { - clientPolicy.IdleTimeout = time.Duration(cfg.ConnIdleTimeoutSecs) * time.Second - } - - // Aerospike's default connection queue size per node is 256. - // If cfg.ConnQueueSize is greater than zero, it will override the default. - if cfg.ConnQueueSize > 0 { - clientPolicy.ConnectionQueueSize = cfg.ConnQueueSize - } - if len(cfg.Host) > 1 { - hosts = append(hosts, as.NewHost(cfg.Host, cfg.Port)) - log.Info("config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts") - } - for _, host := range cfg.Hosts { - hosts = append(hosts, as.NewHost(host, cfg.Port)) - } + clientPolicy := generateAerospikeClientPolicy(cfg) + hosts := generateHostsList(cfg) client, err := as.NewClientWithPolicyAndHost(clientPolicy, hosts...) if err != nil { @@ -110,6 +85,45 @@ func NewAerospikeBackend(cfg config.Aerospike, metrics *metrics.Metrics) *Aerosp } } +// generateAerospikeClientPolicy returns an Aerospike ClientPolicy object configured according to values +// in config.Aerospike fields +func generateAerospikeClientPolicy(cfg config.Aerospike) *as.ClientPolicy { + clientPolicy := as.NewClientPolicy() + // cfg.User and cfg.Password are optional parameters + // if left blank in the config, they will default to the empty + // string and be ignored + clientPolicy.User = cfg.User + clientPolicy.Password = cfg.Password + + // Aerospike's connection idle deadline default is 55 seconds. If greater than zero, this + // value will override + if cfg.ConnIdleTimeoutSecs > 0 { + clientPolicy.IdleTimeout = time.Duration(cfg.ConnIdleTimeoutSecs) * time.Second + } + + // Aerospike's default connection queue size per node is 256. + // If cfg.ConnQueueSize is greater than zero, it will override the default. + if cfg.ConnQueueSize > 0 { + clientPolicy.ConnectionQueueSize = cfg.ConnQueueSize + } + + return clientPolicy +} + +func generateHostsList(cfg config.Aerospike) []*as.Host { + var hosts []*as.Host + + if len(cfg.Host) > 1 { + hosts = append(hosts, as.NewHost(cfg.Host, cfg.Port)) + log.Info("config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts") + } + for _, host := range cfg.Hosts { + hosts = append(hosts, as.NewHost(host, cfg.Port)) + } + + return hosts +} + // Get creates an aerospike key based on the UUID key parameter, perfomrs the client's Get call // and validates results. Can return a KEY_NOT_FOUND error or other Aerospike server errors func (a *AerospikeBackend) Get(ctx context.Context, key string) (string, error) { diff --git a/backends/aerospike_test.go b/backends/aerospike_test.go index 82d2bbfc..f02e2bdf 100644 --- a/backends/aerospike_test.go +++ b/backends/aerospike_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "testing" + "time" as "github.com/aerospike/aerospike-client-go/v6" as_types "github.com/aerospike/aerospike-client-go/v6/types" @@ -17,70 +18,225 @@ import ( "github.com/stretchr/testify/assert" ) -func TestNewAerospikeBackend(t *testing.T) { +func TestGenerateAerospikeClientPolicy(t *testing.T) { + testCases := []struct { + desc string + inCfg config.Aerospike + expected *as.ClientPolicy + }{ + { + desc: "Blank configuration", + inCfg: config.Aerospike{}, + expected: as.NewClientPolicy(), + }, + { + desc: "Config with credentials", + inCfg: config.Aerospike{ + User: "foobar", + Password: "password", + }, + expected: &as.ClientPolicy{ + User: "foobar", + Password: "password", + AuthMode: as.AuthModeInternal, + Timeout: 30 * time.Second, + IdleTimeout: 0 * time.Second, + LoginTimeout: 10 * time.Second, + ConnectionQueueSize: 100, + OpeningConnectionThreshold: 0, + FailIfNotConnected: true, + TendInterval: time.Second, + LimitConnectionsToQueueSize: true, + IgnoreOtherSubnetAliases: false, + MaxErrorRate: 100, + ErrorRateWindow: 1, + }, + }, + { + desc: "Config with ConnIdleTimeoutSecs", + inCfg: config.Aerospike{ + ConnIdleTimeoutSecs: 3600, + }, + expected: &as.ClientPolicy{ + AuthMode: as.AuthModeInternal, + Timeout: 30 * time.Second, + IdleTimeout: 3600 * time.Second, + LoginTimeout: 10 * time.Second, + ConnectionQueueSize: 100, + OpeningConnectionThreshold: 0, + FailIfNotConnected: true, + TendInterval: time.Second, + LimitConnectionsToQueueSize: true, + IgnoreOtherSubnetAliases: false, + MaxErrorRate: 100, + ErrorRateWindow: 1, + }, + }, + { + desc: "Config with ConnIdleTimeoutSecs", + inCfg: config.Aerospike{ + ConnQueueSize: 31416, + }, + expected: &as.ClientPolicy{ + AuthMode: as.AuthModeInternal, + Timeout: 30 * time.Second, + IdleTimeout: 0 * time.Second, + LoginTimeout: 10 * time.Second, + ConnectionQueueSize: 31416, + OpeningConnectionThreshold: 0, + FailIfNotConnected: true, + TendInterval: time.Second, + LimitConnectionsToQueueSize: true, + IgnoreOtherSubnetAliases: false, + MaxErrorRate: 100, + ErrorRateWindow: 1, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + asPolicy := generateAerospikeClientPolicy(tc.inCfg) + assert.Equal(t, tc.expected, asPolicy) + }) + } +} + +func TestGenerateHostsList(t *testing.T) { type logEntry struct { msg string lvl logrus.Level } - testCases := []struct { desc string inCfg config.Aerospike - expectPanic bool + expectedOutput []*as.Host expectedLogEntries []logEntry }{ { - desc: "Unable to connect hosts fakeTestUrl panic and log fatal error when passed additional hosts", + desc: "No host, hosts nor port", + inCfg: config.Aerospike{}, + expectedOutput: []*as.Host{}, + }, + { + desc: "Hosts list but no host nor port", inCfg: config.Aerospike{ - Hosts: []string{"foo.com", "bat.com"}, - Port: 8888, + Hosts: []string{"foo.com", "bar.com"}, + }, + expectedOutput: []*as.Host{ + as.NewHost("foo.com", 0), + as.NewHost("bar.com", 0), }, - expectPanic: true, + }, + { + desc: "Host no port nor hosts list", + inCfg: config.Aerospike{Host: "foo.com"}, + expectedOutput: []*as.Host{as.NewHost("foo.com", 0)}, expectedLogEntries: []logEntry{ { - msg: "Error creating Aerospike backend: ResultCode: TIMEOUT, Iteration: 0, InDoubt: false, Node: : command execution timed out on client: See `Policy.Timeout`", - lvl: logrus.FatalLevel, + msg: "config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts", + lvl: logrus.InfoLevel, }, }, }, { - desc: "Unable to connect host and hosts panic and log fatal error when passed additional hosts", + desc: "Host and hosts list, no port", inCfg: config.Aerospike{ - Host: "fakeTestUrl.foo", - Hosts: []string{"foo.com", "bat.com"}, - Port: 8888, + Host: "foo.com", + Hosts: []string{"foo.com", "bar.com"}, + }, + expectedOutput: []*as.Host{ + as.NewHost("foo.com", 0), + as.NewHost("foo.com", 0), + as.NewHost("bar.com", 0), }, - expectPanic: true, expectedLogEntries: []logEntry{ { msg: "config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts", lvl: logrus.InfoLevel, }, - { - msg: "Error creating Aerospike backend: ResultCode: TIMEOUT, Iteration: 0, InDoubt: false, Node: : command execution timed out on client: See `Policy.Timeout`", - lvl: logrus.FatalLevel, - }, }, }, { - desc: "Unable to connect host panic and log fatal error", + desc: "Port but no host nor hosts list", + inCfg: config.Aerospike{Port: 8888}, + expectedOutput: []*as.Host{}, + }, + { + desc: "Port, hosts list, no host", + inCfg: config.Aerospike{ + Port: 8888, + Hosts: []string{"foo.com", "bar.com"}, + }, + expectedOutput: []*as.Host{ + as.NewHost("foo.com", 8888), + as.NewHost("bar.com", 8888), + }, + }, + { + desc: "Port and host but no hosts list", inCfg: config.Aerospike{ - Host: "fakeTestUrl.foo", Port: 8888, + Host: "foo.com", }, - expectPanic: true, + expectedOutput: []*as.Host{as.NewHost("foo.com", 8888)}, expectedLogEntries: []logEntry{ { msg: "config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts", lvl: logrus.InfoLevel, }, + }, + }, + { + desc: "Port, host and hosts list", + inCfg: config.Aerospike{ + Port: 8888, + Host: "foo.com", + Hosts: []string{"foo.com", "bar.com"}, + }, + expectedOutput: []*as.Host{ + as.NewHost("foo.com", 8888), + as.NewHost("foo.com", 8888), + as.NewHost("bar.com", 8888), + }, + expectedLogEntries: []logEntry{ { - msg: "Error creating Aerospike backend: ResultCode: TIMEOUT, Iteration: 0, InDoubt: false, Node: : command execution timed out on client: See `Policy.Timeout`", - lvl: logrus.FatalLevel, + msg: "config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts", + lvl: logrus.InfoLevel, }, }, }, } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + asHosts := generateHostsList(tc.inCfg) + assert.ElementsMatch(t, tc.expectedOutput, asHosts) + }) + } +} + +func TestNewAerospikeBackend(t *testing.T) { + testCases := []struct { + desc string + inCfg config.Aerospike + expectedLogEntryLevels []logrus.Level + }{ + { + desc: "Unable to connect to URLs in Hosts list.", + inCfg: config.Aerospike{ + Hosts: []string{"fakeUrl"}, + Port: 8888, + }, + expectedLogEntryLevels: []logrus.Level{logrus.FatalLevel}, + }, + { + desc: "Unable to connect to URL in Host field", + inCfg: config.Aerospike{ + Host: "fakeUrl", + Port: 8888, + }, + expectedLogEntryLevels: []logrus.Level{logrus.InfoLevel, logrus.FatalLevel}, + }, + } // logrus entries will be recorded to this `hook` object so we can compare and assert them hook := test.NewGlobal() @@ -92,10 +248,9 @@ func TestNewAerospikeBackend(t *testing.T) { for _, test := range testCases { // Run test assert.Panics(t, func() { NewAerospikeBackend(test.inCfg, nil) }, "Aerospike library's NewClientWithPolicyAndHost() should have thrown an error and didn't, hence the panic didn't happen") - if assert.Len(t, hook.Entries, len(test.expectedLogEntries), test.desc) { - for i := 0; i < len(test.expectedLogEntries); i++ { - assert.Equal(t, test.expectedLogEntries[i].msg, hook.Entries[i].Message, test.desc) - assert.Equal(t, test.expectedLogEntries[i].lvl, hook.Entries[i].Level, test.desc) + if assert.Len(t, hook.Entries, len(test.expectedLogEntryLevels), test.desc) { + for i := 0; i < len(test.expectedLogEntryLevels); i++ { + assert.Equal(t, test.expectedLogEntryLevels[i], hook.Entries[i].Level, test.desc) } }