Closed bfabio closed 7 years ago
@drybjed Sounds good.
@bfabio Hi, I already had something similar in my queue, look at my changes here:https://github.com/cultcom/ansible-gitlab/tree/cultcom-9.x
Maybe you can adopt the gitaly versions and the service file. The systemd service file still needs some attendance I think.
@bfabio, can you rebase this PR on the recent changes merged in the master
branch? I'll update the Travis test playbook to not include the removed variable.
@bfabio what are 'libsqlite3-dev' and 'python-mysqldb' meant for?
the later one should be included by debops.mariadb_server or am I wrong here?
@bfabio and the "gitconfig" template does not need a version check for > 9.x. The writeback was first mentioned here https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/update/8.11-to-8.12.md
@cultcom python-mysqldb
should not be required, I misinterpreted the output from the CI build.
libsqlite3-dev
should be required by bundle install
, I think it can be removed using gitlab_ce_bundle_install_without
. Let's see what Travis has to say about that.
I removed the conditional in gitconfig.j2
, thanks for the hint.
@cultcom Also, I'm going to merge your changes if that's ok with you.
@bfabio Forgot apt_key
task, I think?
@drybjed Yup :)
@bfabio I just pushed a cleanup for the gitaly service file, have a look at it as well please. Your are welcome to integrate the useful stuff from my branch ;-)
Guys, don't forget to add yourselves to the COPYRIGHT
file.
@bfabio, while you are working on this PR, is it OK for me to merge https://github.com/debops/ansible-gitlab/pull/86 which has mostly cosmetic changes? You will need to rebase your PR afterwards.
@drybjed, sure, go ahead.
@bfabio The PR has been merged, you need to rebase this one.
@bfabio Anything else planned for the PR? Let me know when it's ready to merge.
@bfabio Another PR merged, can you fix the conflicts and rebase once again? I guess this PR can be merged next?
@drybjed I would like to add @cultcom's work and install gitaly
as well, because it turns out gitlab-shell doesn't handle its absence, even when gitaly
is explicitly disabled in the configuration file.
@drybjed @bfabio I am not sure but until 9.2 the documentation mentioned gitaly as optional. The 9.3 update guide is missing that part but does not explicitly mark it mandatory. On the other hand the example config changed to use it by default.
Is there anything left to do until it will be merged? I can help, as I want to be able to install version 9.3 via ansible.
@alinalexandru I think the gitaly installation part is still missing if you like you may add it. Unfortunately I am currently out of time for it.
Don't know what is gitaly. I have to learn about it
This steps fo I have to follow ? https://docs.gitlab.com/ce/install/installation.html#install-gitaly
@alinalexandru Yes, sounds like it. Keep in mind that debops.gitlab
uses different home directory path. You can check how IIRC gitlab-workhorse
was installed to see how the role handled Go applications.
This should be all.
I didn't test this in production though, so testers are welcome.
@bfabio Great, thanks for your work!
@alinalexandru Any chance for you to test this PR before merging?
Only next week. BTW. are the runners updated?
@alinalexandru Do you mean debops.gitlab_runner
? There are some pending PRs, I'll try to look them over during the weekend.
meanwhile 9.4 was released 😄
@alinalexandru I'm testing the upgrade path of this PR on one of my hosts, if it passes, I'm going to merge it in about 1-2 hours.
@drybjed Do you think is difficult to add support for 9.4?
From the upgrade instructions it looks like there are no major changes. Just adding the correct versions to the version map should be enough. Want to try it?
I don't have time now, but I will try when I will put gitlab via ansible.
@bfabio Nice work. However, @cultcom is also working on a parallel PR https://github.com/debops/ansible-gitlab/pull/81 which adds support for Debian Stretch. The Travis-CI test at the moment cannot handle both PRs because you are changing the playbook and removing a variable (which is fine, BTW, no issue there).
How about this - let's do the Stretch changes first because the current test playbook works with that PR, then I'll change the playbook and we do the 9.x PR next?