diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-04-12 22:07:44 +0300 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-10-19 10:27:39 +0300 |
commit | 69ca63441ca27d9dca72e756f19714dd517795f4 (patch) | |
tree | c72460d6745303dbdf5bda326756bf5f280e186a | |
parent | 3c61501051072811471605a67bb761ccaa2f5146 (diff) |
Require storage directories to exist at startup
Right now GitLab tries to check if the disks are reachable, but this is
not it's responsibility.
This gave a couple of options to how Gitaly would do this, and I've
opted to detect at boot if the storage is reachable. A forgotten mount
will be clear this way, and logged without being hidden between lots of
other messages.
-rw-r--r-- | changelogs/unreleased/zj-gitaly-dont-start-no-storage.yml | 5 | ||||
-rw-r--r-- | internal/config/config.go | 10 | ||||
-rw-r--r-- | internal/config/config_test.go | 40 | ||||
-rw-r--r-- | internal/config/testdata/repositories/.gitkeep | 0 | ||||
-rw-r--r-- | internal/config/testdata/repositories2/.gitkeep | 0 |
5 files changed, 42 insertions, 13 deletions
diff --git a/changelogs/unreleased/zj-gitaly-dont-start-no-storage.yml b/changelogs/unreleased/zj-gitaly-dont-start-no-storage.yml new file mode 100644 index 000000000..fd85c6891 --- /dev/null +++ b/changelogs/unreleased/zj-gitaly-dont-start-no-storage.yml @@ -0,0 +1,5 @@ +--- +title: Require storage directories to exist at startup +merge_request: 675 +author: +type: changed diff --git a/internal/config/config.go b/internal/config/config.go index eb796acd3..9e8ed6566 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -146,6 +146,10 @@ func validateStorages() error { return fmt.Errorf("empty storage path in %v", storage) } + if fs, err := os.Stat(storage.Path); err != nil || !fs.IsDir() { + return fmt.Errorf("storage paths have to exist %v", storage) + } + stPath := filepath.Clean(storage.Path) for j := 0; j < i; j++ { other := Config.Storages[j] @@ -160,7 +164,11 @@ func validateStorages() error { } if strings.HasPrefix(stPath, otherPath) || strings.HasPrefix(otherPath, stPath) { - return fmt.Errorf("storage paths may not nest: %q and %q", storage.Name, storage.Name) + // If storages have the same sub directory, that is allowed + if filepath.Dir(stPath) == filepath.Dir(otherPath) { + continue + } + return fmt.Errorf("storage paths may not nest: %q and %q", storage.Name, other.Name) } } } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c842c6d1e..f02221f4e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -158,6 +159,14 @@ func TestValidateStorages(t *testing.T) { Config.Storages = oldStorages }(Config.Storages) + repositories, err := filepath.Abs("testdata/repositories") + require.NoError(t, err) + + repositories2, err := filepath.Abs("testdata/repositories2") + require.NoError(t, err) + + invalidDir := path.Join(repositories, t.Name()) + testCases := []struct { desc string storages []Storage @@ -166,23 +175,22 @@ func TestValidateStorages(t *testing.T) { { desc: "just 1 storage", storages: []Storage{ - {Name: "default", Path: "/home/git/repositories"}, + {Name: "default", Path: repositories}, }, }, { desc: "multiple storages", storages: []Storage{ - {Name: "default", Path: "/home/git/repositories1"}, - {Name: "other", Path: "/home/git/repositories2"}, - {Name: "third", Path: "/home/git/repositories3"}, + {Name: "default", Path: repositories}, + {Name: "other", Path: repositories2}, }, }, { desc: "multiple storages pointing to same directory", storages: []Storage{ - {Name: "default", Path: "/home/git/repositories"}, - {Name: "other", Path: "/home/git/repositories"}, - {Name: "third", Path: "/home/git/repositories"}, + {Name: "default", Path: repositories}, + {Name: "other", Path: repositories}, + {Name: "third", Path: repositories}, }, }, { @@ -206,23 +214,23 @@ func TestValidateStorages(t *testing.T) { { desc: "duplicate definition", storages: []Storage{ - {Name: "default", Path: "/home/git/repositories"}, - {Name: "default", Path: "/home/git/repositories"}, + {Name: "default", Path: repositories}, + {Name: "default", Path: repositories}, }, invalid: true, }, { desc: "re-definition", storages: []Storage{ - {Name: "default", Path: "/home/git/repositories1"}, - {Name: "default", Path: "/home/git/repositories2"}, + {Name: "default", Path: repositories}, + {Name: "default", Path: repositories2}, }, invalid: true, }, { desc: "empty name", storages: []Storage{ - {Name: "", Path: "/home/git/repositories1"}, + {Name: "", Path: repositories}, }, invalid: true, }, @@ -233,6 +241,14 @@ func TestValidateStorages(t *testing.T) { }, invalid: true, }, + { + desc: "non existing directory", + storages: []Storage{ + {Name: "default", Path: repositories}, + {Name: "nope", Path: invalidDir}, + }, + invalid: true, + }, } for _, tc := range testCases { diff --git a/internal/config/testdata/repositories/.gitkeep b/internal/config/testdata/repositories/.gitkeep new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/internal/config/testdata/repositories/.gitkeep diff --git a/internal/config/testdata/repositories2/.gitkeep b/internal/config/testdata/repositories2/.gitkeep new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/internal/config/testdata/repositories2/.gitkeep |