Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitLab 13.0.14 -> 13.6.0 #99116

Merged
merged 7 commits into from Nov 22, 2020
Merged

GitLab 13.0.14 -> 13.6.0 #99116

merged 7 commits into from Nov 22, 2020

Conversation

jslight90
Copy link
Contributor

@jslight90 jslight90 commented Sep 29, 2020

Motivation for this change

Upgrade GitLab to latest version

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Besides the version changes for GitLab and its dependencies, the other changes included are:

@talyz
Copy link
Contributor

talyz commented Oct 5, 2020

@GrahamcOfBorg test gitlab

Copy link
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for the most part, will test.

Copy link
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works 👍

@ghost
Copy link

ghost commented Oct 8, 2020

Thanks! Bumped to 13.4.3, now testing.

@ghost ghost changed the title GitLab 13.0.14 -> 13.4.0 GitLab 13.0.14 -> 13.4.3 Oct 8, 2020
@ghost
Copy link

ghost commented Oct 8, 2020

After updating from 13.0.14 I had to clean /var/gitlab/state/config and possibly /run/gitlab/shell-config.yml and run systemd-tmpfiles --create again, otherwise the secrets would be out of sync and every push would return a 401 unauthorized. It seems to work fine now, but it would be good to make sure the upgrade path works without manual intervention.

@lheckemann
Copy link
Member

@petabyteboy how did you "clean" /var/gitlab/state/config?

@lheckemann
Copy link
Member

lheckemann commented Oct 9, 2020

In my case, restarting gitaly after starting gitlab seems to have done the trick — but it occurs again after the machine is rebooted. I suspect this is because the secret is generated at gitlab's startup and changes every time. I suppose gitaly should only be started after it's generated in this case — I'll test if adding after = ["gitlab.service"]; to gitaly is sufficient for this. EDIT: it is not, that introduces a dependency cycle.

We should fix this before merging, but I'm surprised that this seems to have been introduced with this version bump — maybe it's a race condition that existed beforehand but wasn't as likely to be triggered, and is now triggered by unrelated code taking longer or finishing faster.

@ofborg eval

(since the evaluation seems to have failed for unrelated reasons)

@bgamari
Copy link
Contributor

bgamari commented Oct 18, 2020

I can confirm that simply restarting gitaly after gitlab has started is sufficient to avoid the issue.

@ghost
Copy link

ghost commented Nov 9, 2020

I propose to merge this to master, create a tracking issue for the remaining known issue(s?) and possibly show a warning during evaluation to make users aware of it. When the issue is solved we can backport it as well.

@lheckemann
Copy link
Member

I'm not keen on breaking gitlab on master, even with a warning. This will, AFAIK, always require manual intervention (including after reboots and such), and not be obvious until a user complains about not being able to push.

@lheckemann
Copy link
Member

I'm looking at fleshing out the gitlab nixos test a little, so we have a reliable way of testing an eventual fix.

@dpausp
Copy link
Contributor

dpausp commented Nov 12, 2020

Gitaly should start after Gitlab and Gitaly should be restarted when Gitlab restarts, so I changed the deps like this:

systemd.services.gitaly = {
  after = [ "network.target" "gitlab.service" ];
  requires = [ "gitlab.service" ];
  wantedBy = [ "multi-user.target" ];

and

systemd.services.gitlab = {
  after = [ "gitlab-workhorse.service" "network.target" "gitlab-postgresql.service" "redis.service" ];
  requires = [ "gitlab-sidekiq.service" ];
  wantedBy = [ "multi-user.target" ];

With this, pushing still worked after multiple restarts of the gitlab service.

We are running the code from this PR with my changes on our staging Gitlab at the Flying Circus and prod is currently upgrading :)

@ajs124 ajs124 mentioned this pull request Nov 15, 2020
@mohe2015
Copy link
Contributor

mohe2015 commented Nov 20, 2020

I don't know the procedures for such big packages but what about updating to 13.6.0?

@ghost
Copy link

ghost commented Nov 20, 2020

I was running 13.5.x locally, 13.6.0 will require some manual work because they added a ruby dependency that tries to download stuff during the build process >.>

@ghost
Copy link

ghost commented Nov 20, 2020

I pushed an update to 13.6.0 and the fix proposed by @dpausp

@ghost ghost requested review from flokli and ajs124 November 20, 2020 21:12
@ofborg ofborg bot added the 6.topic: ruby label Nov 20, 2020
@ghost ghost changed the title GitLab 13.0.14 -> 13.4.3 GitLab 13.0.14 -> 13.6.0 Nov 20, 2020
Milan Pässler added 3 commits November 21, 2020 01:38
Changed ruby version to 2.7.x to match upstream.
Added a gem config for gitlab-pg_query as it tries to download a source
tarball during the build process.
Also removed a patch for gitaly that has become obsolete by upstream fix
[here](https://gitlab.com/gitlab-org/gitaly/-/commit/de04077c25cc23b001317d2efdf5a9ead0bc86b9).
@ghost
Copy link

ghost commented Nov 21, 2020

@GrahamcOfBorg eval

Copy link
Contributor

@mohe2015 mohe2015 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only used a test install but the upgrade worked for me.

@flokli flokli merged commit 904f124 into NixOS:master Nov 22, 2020
@flokli
Copy link
Contributor

flokli commented Nov 22, 2020

Can someone take care of a backport PR for 20.09 at least?

@ghost
Copy link

ghost commented Nov 22, 2020

Thanks for merging. I will do a PR for the backport.

@ghost ghost mentioned this pull request Nov 22, 2020
10 tasks
@jslight90 jslight90 deleted the gitlab-13.4.0 branch November 24, 2020 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants