diff options
author | Pavlo Strokov <pstrokov@gitlab.com> | 2020-05-06 15:24:35 +0300 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-05-06 15:24:35 +0300 |
commit | c1a0129bf7c6769a7a4316b2b003e2109d0524dd (patch) | |
tree | 5acd4f694ea713b15ef93a0e4479c6b28e6ef487 /internal/praefect/config | |
parent | feb75fc196dc07256d95aeb6d66387ebd35fbe08 (diff) |
Praefect: configuration verification
Praefect uses a .toml file as a source of configuration.
Before praefect starts handle requests the verification
must be executed to check if configuration is correct.
All `gitaly`-s must use distinct addresses.
Any virtual node must have at least one gitaly behind it.
Each virtual node must be named.
Diffstat (limited to 'internal/praefect/config')
-rw-r--r-- | internal/praefect/config/config.go | 47 | ||||
-rw-r--r-- | internal/praefect/config/config_test.go | 170 |
2 files changed, 148 insertions, 69 deletions
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go index 3508c4a23..fb28f2a20 100644 --- a/internal/praefect/config/config.go +++ b/internal/praefect/config/config.go @@ -72,8 +72,9 @@ var ( errNoListener = errors.New("no listen address or socket path configured") errNoPrimaries = errors.New("no primaries designated") errNoVirtualStorages = errors.New("no virtual storages configured") - errStorageAddressMismatch = errors.New("storages with the same name must have the same address") + errStorageAddressDuplicate = errors.New("multiple storages have the same address") errVirtualStoragesNotUnique = errors.New("virtual storages must have unique names") + errVirtualStorageUnnamed = errors.New("virtual storages must have a name") ) // Validate establishes if the config is valid @@ -86,55 +87,55 @@ func (c Config) Validate() error { return errNoVirtualStorages } - allStorages := make(map[string]string) - virtualStorages := make(map[string]struct{}) + allAddresses := make(map[string]struct{}) + virtualStorages := make(map[string]struct{}, len(c.VirtualStorages)) for _, virtualStorage := range c.VirtualStorages { - if _, ok := virtualStorages[virtualStorage.Name]; ok { - return errVirtualStoragesNotUnique + if virtualStorage.Name == "" { + return errVirtualStorageUnnamed + } + + if len(virtualStorage.Nodes) == 0 { + return fmt.Errorf("virtual storage %q: %w", virtualStorage.Name, errNoGitalyServers) } + if _, ok := virtualStorages[virtualStorage.Name]; ok { + return fmt.Errorf("virtual storage %q: %w", virtualStorage.Name, errVirtualStoragesNotUnique) + } virtualStorages[virtualStorage.Name] = struct{}{} - storages := make(map[string]struct{}) - var primaries int + storages := make(map[string]struct{}, len(virtualStorage.Nodes)) + primaries := 0 for _, node := range virtualStorage.Nodes { if node.DefaultPrimary { primaries++ } if primaries > 1 { - return fmt.Errorf("virtual storage %s: %v", virtualStorage.Name, errMoreThanOnePrimary) + return fmt.Errorf("virtual storage %q: %w", virtualStorage.Name, errMoreThanOnePrimary) } if node.Storage == "" { - return errGitalyWithoutStorage + return fmt.Errorf("virtual storage %q: %w", virtualStorage.Name, errGitalyWithoutStorage) } if node.Address == "" { - return errGitalyWithoutAddr + return fmt.Errorf("virtual storage %q: %w", virtualStorage.Name, errGitalyWithoutAddr) } if _, found := storages[node.Storage]; found { - return errDuplicateStorage + return fmt.Errorf("virtual storage %q: %w", virtualStorage.Name, errDuplicateStorage) } + storages[node.Storage] = struct{}{} - if address, found := allStorages[node.Storage]; found { - if address != node.Address { - return errStorageAddressMismatch - } - } else { - allStorages[node.Storage] = node.Address + if _, found := allAddresses[node.Address]; found { + return fmt.Errorf("virtual storage %q: address %q : %w", virtualStorage.Name, node.Address, errStorageAddressDuplicate) } - - storages[node.Storage] = struct{}{} + allAddresses[node.Address] = struct{}{} } if primaries == 0 { - return fmt.Errorf("virtual storage %s: %v", virtualStorage.Name, errNoPrimaries) - } - if len(storages) == 0 { - return fmt.Errorf("virtual storage %s: %v", virtualStorage.Name, errNoGitalyServers) + return fmt.Errorf("virtual storage %q: %w", virtualStorage.Name, errNoPrimaries) } } diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index 041c30188..0077e6094 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -1,7 +1,6 @@ package config import ( - "strings" "testing" "github.com/stretchr/testify/assert" @@ -13,59 +12,92 @@ import ( ) func TestConfigValidation(t *testing.T) { - nodes := []*models.Node{ - {Storage: "internal-1", Address: "localhost:23456", Token: "secret-token", DefaultPrimary: true}, - {Storage: "internal-2", Address: "localhost:23457", Token: "secret-token"}, - {Storage: "internal-3", Address: "localhost:23458", Token: "secret-token"}, + vs1Nodes := []*models.Node{ + {Storage: "internal-1.0", Address: "localhost:23456", Token: "secret-token-1", DefaultPrimary: true}, + {Storage: "internal-2.0", Address: "localhost:23457", Token: "secret-token-1"}, + {Storage: "internal-3.0", Address: "localhost:23458", Token: "secret-token-1"}, + } + + vs2Nodes := []*models.Node{ + // storage can have same name as storage in another virtual storage, but all addresses must be unique + {Storage: "internal-1.0", Address: "localhost:33456", Token: "secret-token-2", DefaultPrimary: true}, + {Storage: "internal-2.1", Address: "localhost:33457", Token: "secret-token-2"}, + {Storage: "internal-3.1", Address: "localhost:33458", Token: "secret-token-2"}, } testCases := []struct { desc string config Config - err error + errMsg string }{ { - desc: "No ListenAddr or SocketPath", - config: Config{ListenAddr: "", VirtualStorages: []*VirtualStorage{&VirtualStorage{Nodes: nodes}}}, - err: errNoListener, + desc: "Valid config with ListenAddr", + config: Config{ + ListenAddr: "localhost:1234", + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + {Name: "secondary", Nodes: vs2Nodes}, + }, + }, }, { - desc: "Only a SocketPath", - config: Config{SocketPath: "/tmp/praefect.socket", VirtualStorages: []*VirtualStorage{&VirtualStorage{Nodes: nodes}}}, - err: nil, + desc: "Valid config with SocketPath", + config: Config{ + SocketPath: "/tmp/praefect.socket", + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + }, + }, + }, + { + desc: "No ListenAddr or SocketPath", + config: Config{ + ListenAddr: "", + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + }, + }, + errMsg: "no listen address or socket path configured", }, { - desc: "No servers", + desc: "No virtual storages", config: Config{ListenAddr: "localhost:1234"}, - err: errNoVirtualStorages, + errMsg: "no virtual storages configured", }, { desc: "duplicate storage", config: Config{ ListenAddr: "localhost:1234", VirtualStorages: []*VirtualStorage{ - &VirtualStorage{Nodes: append(nodes, &models.Node{Storage: nodes[0].Storage, Address: nodes[1].Address})}, + { + Name: "default", + Nodes: append(vs1Nodes, &models.Node{ + Storage: vs1Nodes[0].Storage, + Address: vs1Nodes[1].Address, + }), + }, }, }, - err: errDuplicateStorage, + errMsg: `virtual storage "default": internal gitaly storages are not unique`, }, { - desc: "Valid config", - config: Config{ListenAddr: "localhost:1234", VirtualStorages: []*VirtualStorage{&VirtualStorage{Nodes: nodes}}}, - err: nil, - }, - { - desc: "No designated primaries", - config: Config{ListenAddr: "localhost:1234", VirtualStorages: []*VirtualStorage{&VirtualStorage{Nodes: nodes[1:]}}}, - err: errNoPrimaries, + desc: "No designated primaries", + config: Config{ + ListenAddr: "localhost:1234", + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes[1:]}, + }, + }, + errMsg: `virtual storage "default": no primaries designated`, }, { desc: "More than 1 primary", config: Config{ ListenAddr: "localhost:1234", VirtualStorages: []*VirtualStorage{ - &VirtualStorage{ - Nodes: append(nodes, + { + Name: "default", + Nodes: append(vs1Nodes, &models.Node{ Storage: "internal-4", Address: "localhost:23459", @@ -75,56 +107,102 @@ func TestConfigValidation(t *testing.T) { }, }, }, - err: errMoreThanOnePrimary, + errMsg: `virtual storage "default": only 1 node can be designated as a primary`, }, { - desc: "Node storage not unique", + desc: "Node storage has no name", config: Config{ ListenAddr: "localhost:1234", VirtualStorages: []*VirtualStorage{ - &VirtualStorage{Name: "default", Nodes: nodes}, - &VirtualStorage{ - Name: "backup", + { + Name: "default", Nodes: []*models.Node{ - &models.Node{ - Storage: nodes[0].Storage, - Address: "some.other.address", - DefaultPrimary: true}, + { + Storage: "", + Address: "localhost:23456", + Token: "secret-token-1", + DefaultPrimary: true, + }, }, }, }, }, - err: errStorageAddressMismatch, + errMsg: `virtual storage "default": all gitaly nodes must have a storage`, }, { - desc: "Node storage not unique", + desc: "Node storage has no address", config: Config{ ListenAddr: "localhost:1234", VirtualStorages: []*VirtualStorage{ - &VirtualStorage{Name: "default", Nodes: nodes}, - &VirtualStorage{ + { Name: "default", Nodes: []*models.Node{ - &models.Node{ - Storage: nodes[0].Storage, - Address: "some.other.address", - DefaultPrimary: true}}, + { + Storage: "internal", + Address: "", + Token: "secret-token-1", + DefaultPrimary: true, + }, + }, }, }, }, - err: errVirtualStoragesNotUnique, + errMsg: `virtual storage "default": all gitaly nodes must have an address`, + }, + { + desc: "Virtual storage has no name", + config: Config{ + ListenAddr: "localhost:1234", + VirtualStorages: []*VirtualStorage{ + {Name: "", Nodes: vs1Nodes}, + }, + }, + errMsg: `virtual storages must have a name`, + }, + { + desc: "Virtual storage not unique", + config: Config{ + ListenAddr: "localhost:1234", + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + {Name: "default", Nodes: vs2Nodes}, + }, + }, + errMsg: `virtual storage "default": virtual storages must have unique names`, + }, + { + desc: "Virtual storage has no nodes", + config: Config{ + ListenAddr: "localhost:1234", + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + {Name: "secondary", Nodes: nil}, + }, + }, + errMsg: `virtual storage "secondary": no primary gitaly backends configured`, + }, + { + desc: "Node storage has address duplicate", + config: Config{ + ListenAddr: "localhost:1234", + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + {Name: "secondary", Nodes: append(vs2Nodes, vs1Nodes[1])}, + }, + }, + errMsg: `multiple storages have the same address`, }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { err := tc.config.Validate() - if tc.err == nil { + if tc.errMsg == "" { assert.NoError(t, err) return } - assert.True(t, strings.Contains(err.Error(), tc.err.Error())) + assert.Contains(t, err.Error(), tc.errMsg) }) } } |