Prevent Gitaly storages from using the same path
What does this MR do?
Prevent Gitaly storages from using the same path
Gitaly is making a breaking change with v17.0 to prevent multiple storages from sharing the same local path. This is being done as part of the work to add a write-ahead log to Gitaly.
Validate that Gitaly's config does not have than one storage using the same path.
Related issues
Checklist
See Definition of done.
For anything in this list which will not be completed, please provide a reason in the MR discussion.
Required
-
MR title and description are up to date, accurate, and descriptive. -
MR targeting the appropriate branch. -
Latest Merge Result pipeline is green. -
When ready for review, MR is labeled "~workflow::ready for review" per the Distribution MR workflow.
For GitLab team members
If you don't have access to this, the reviewer should trigger these jobs for you during the review process.
-
The manual Trigger:ee-package
jobs have a green pipeline running against latest commit. -
If config/software
orconfig/patches
directories are changed, make sure thebuild-package-on-all-os
job within theTrigger:ee-package
downstream pipeline succeeded. -
If you are changing anything SSL related, then the Trigger:package:fips
manual job within theTrigger:ee-package
downstream pipeline must succeed. -
If CI configuration is changed, the branch must be pushed to dev.gitlab.org
to confirm regular branch builds aren't broken.
Expected (please provide an explanation if not completing)
-
Test plan indicating conditions for success has been posted and passes. -
Documentation created/updated. -
Tests added. -
Integration tests added to GitLab QA. -
Equivalent MR/issue for the GitLab Chart opened. -
Validate potential values for new configuration settings. Formats such as integer 10
, duration10s
, URIscheme://user:passwd@host:port
may require quotation or other special handling when rendered in a template and written to a configuration file.
Merge request reports
Activity
changed milestone to %17.0
assigned to @wchandler
added 1 commit
- 8291dede - Prevent Gitaly storages from using the same path
1 Warning ⚠ You've made some changes at the locations which contain user facing configuration.
That's OK as long as you're refactoring existing code and not adding any new
configuration. If you are adding new user facing configuration, consider adding
to gitlab.rb.template located in files/gitlab-config-template/gitlab.rb.template .
Otherwise, please consider adding the typemaintenance label in that case.Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.
Reviewer Maintainer @deriamis
(UTC-7, 3 hours behind
@wchandler
)@rmarshall
(UTC-4, same timezone as
@wchandler
)If needed, you can retry the
🔁 danger-review
job that generated this comment.Generated by
🚫 Dangermentioned in issue gitaly#5997 (closed)
E2E Test Result Summary
allure-report-publisher
generated test report!qa-subset-test:
✅ test report for a626bd74expand test summary
+---------------------------------------------------------------------+ | suites summary | +----------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +----------------+--------+--------+---------+-------+-------+--------+ | Govern | 66 | 0 | 1 | 0 | 67 | ✅ | | Monitor | 16 | 0 | 1 | 0 | 17 | ✅ | | Data Stores | 33 | 0 | 0 | 0 | 33 | ✅ | | Package | 106 | 0 | 42 | 0 | 148 | ✅ | | Create | 94 | 0 | 13 | 0 | 107 | ✅ | | Plan | 70 | 0 | 7 | 0 | 77 | ✅ | | Verify | 15 | 0 | 0 | 0 | 15 | ✅ | | GitLab Metrics | 2 | 0 | 1 | 0 | 3 | ✅ | | Manage | 2 | 0 | 0 | 0 | 2 | ✅ | | Systems | 9 | 0 | 0 | 0 | 9 | ✅ | | Release | 3 | 0 | 0 | 0 | 3 | ✅ | | Configure | 1 | 0 | 0 | 0 | 1 | ✅ | +----------------+--------+--------+---------+-------+-------+--------+ | Total | 417 | 0 | 65 | 0 | 482 | ✅ | +----------------+--------+--------+---------+-------+-------+--------+
marked the checklist item If CI configuration is changed, the branch must be pushed to
dev.gitlab.org
to confirm regular branch builds aren't broken. as completed- Resolved by Robert Marshall
@gitlab-bot ready @brodock
added workflowready for review label
requested review from @brodock
- Resolved by Robert Marshall
This cannot be merged until gitaly#5997 (closed) has been resolved, but we can get started on reviews in the meantime.
added 14 commits
-
8291dede...1f0bfa5d - 13 commits from branch
master
- 4a023384 - Prevent Gitaly storages from using the same path
-
8291dede...1f0bfa5d - 13 commits from branch
marked the checklist item When ready for review, MR is labeled "~workflow::ready for review" per the Distribution MR workflow. as completed
mentioned in issue gitaly#5598 (closed)
mentioned in issue gitlab-com/support/support-team-meta#6021
- Resolved by Will Chandler (ex-GitLab)
- Resolved by Will Chandler (ex-GitLab)
- Resolved by Will Chandler (ex-GitLab)
@wchandler back to you
🏓
requested review from @brodock
requested review from @rmarshall
- Resolved by Robert Marshall
- Resolved by Robert Marshall
One quibble over the emitted message and a concern about hardlinks.
Super large projects (for example, mirrors) use hardlinks to retain paths without data on disk twice or symlink issues in some tools.
I did not see in the
realpath
source where they account for hardlinks.
added 1 commit
- 7c0e6cfc - Convert error to use 'path' instead of 'realpath'
requested review from @rmarshall
added workflowin review label and removed workflowready for review label
- Resolved by Robert Marshall
Practical tests
-
gitlab.rb
external_url 'http://EXAMPLE.TLD' gitaly['configuration'] = { storage: [ { name: 'default', path: '/var/opt/gitlab/git-data/my-favorite-repo-01', }, { name: 'sneaksy', path: '/var/opt/gitlab/git-data/my-favorite-repo-01' } ] }
- First
gitlab-ctl reconfigure
at installationRunning handlers: [2024-05-09T21:57:21+00:00] ERROR: Running exception handlers There was an error running gitlab-ctl reconfigure: One or more Gitaly storages are accessible by multiple filesystem paths: /var/opt/gitlab/git-data/my-favorite-repo-01: default, sneaksy
- Changed my
gitlab.rb
external_url 'http://EXAMPLE.TLD' gitaly['configuration'] = { storage: [ { name: 'default', path: '/var/opt/gitlab/git-data/my-favorite-repo-01', }, { name: 'sneaksy', path: '/var/opt/gitlab/git-data/my-favorite-repo-02' } ] }
- Ran
gitlab-ctl reconfigure
. It worked fine. - Changed my
gitlab.rb
back to test after workinggitlab-ctl reconfigure
.external_url 'http://EXAMPLE.TLD' gitaly['configuration'] = { storage: [ { name: 'default', path: '/var/opt/gitlab/git-data/my-favorite-repo-01', }, { name: 'sneaksy', path: '/var/opt/gitlab/git-data/my-favorite-repo-01' } ] }
- This gave the same error message.
There was an error running gitlab-ctl reconfigure: One or more Gitaly storages are accessible by multiple filesystem paths: /var/opt/gitlab/git-data/my-favorite-repo-01: default, sneaksy
Functionality appears to work.
Other things
I noticed that when I set the storages, they were not automatically created even though they were on the local filesystem.
Is that by design?
ls -la /var/opt/gitlab/git-data total 12 drwx------ 3 git git 4096 May 9 21:58 . drwxr-xr-x 21 root root 4096 May 9 22:02 .. drwxrws--- 2 git git 4096 May 9 21:58 repositories
-
mentioned in commit d0eba1b5
mentioned in incident gitlab-com/gl-infra/production#17988 (closed)
@wchandler @rmarshall We ran into this issue on dev.gitlab.org due to some legacy CephFS test projects: gitlab-com/gl-infra/production#17988 (closed)
One thing that's not obvious is that if you do have duplicate storages in the config, you need to make sure that the database entries for
projects.repository_storage
aren't present or dropping the storage from the config file may cause issues. Sincealternate
was pointing todefault
, I had to do this for 134 projects:irb(main):002:0> Project.where(repository_storage: 'alternate').update_all(repository_storage: 'default') => 134
@gerardo FYI this may come up for self-managed customers as well when they upgrade to 17.0.
@wchandler Do we need to document/add a Rake task that will remap storages?
@stanhu good idea. For now I'm adding your command ad hoc command in the breaking change docs, gitlab!152852 (merged).
Will work on adding a rake task as well.
mentioned in merge request !7600 (merged)
added workflowstaging-canary label and removed workflowin review label
added workflowcanary label and removed workflowstaging-canary label