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

Using rbenv instead of system ruby #114

Closed
wants to merge 19 commits into from

Conversation

andyleejordan
Copy link
Collaborator

This would remove the need to manage Ruby either manually, or with puppet-gitlab-requirements. Instead it uses the popular rbenv module to setup the proper Ruby version for GitLab. I'll update tests later.

Conflicts:
	Modulefile
	manifests/install.pp
@igalic
Copy link
Collaborator

igalic commented Feb 3, 2014

Two comments,

@andyleejordan
Copy link
Collaborator Author

Good comments, it's not quite finished it. It works so far, if I get it tested, documented, and cleaned up, would it be of interest to replace puppetlabs/ruby in gitlab-requirements?

@atomaka
Copy link
Collaborator

atomaka commented Feb 4, 2014

A couple of thoughts/questions:

  • We currently aren't managing Ruby at all in this module. This forces Ruby management via rbenv restricting freedom of using system ruby / rvm / etc.
  • GitLab works as expected with rbenv? I haven't followed lately, but I thought they advised against it at one point.

@andyleejordan
Copy link
Collaborator Author

Which is fair enough, but the nice thing is that it's restricted to the GitLab user, and ensures the use of a working Ruby. So it manages Ruby, but for only the specific user, meaning system Ruby / rvm / rbenv for other users aren't needed. I could setup up a $manage_ruby switch pretty easily, to turn on/off rbenv.

GitLab is working totally as expected with rbenv for me. I didn't read anything in the literature against using it, and a cursory Google search only revealed someone's frustrations (the fix being sourcing rvbenv in .bashrc for ssh, which this does).

@igalic
Copy link
Collaborator

igalic commented Feb 4, 2014

IMO, rbenv + path, is way saner than RVM.
I'm using rbenv + fpm, which I install in a special path. For us this is "system-wide", and the reason why I introduced #90

@andyleejordan
Copy link
Collaborator Author

@igalic #90 by the way made this part a lot easier. Yay $exec_path, it was beautiful to see in master (I'd originally implemented this on top of the latest tag).

I'll write tests/documentation hopefully Thursday.

@andyleejordan
Copy link
Collaborator Author

@atomaka Ah! This document does indeed say:

The use of ruby version managers such as RVM, rbenv or chruby with GitLab in production frequently leads to hard to diagnose problems. Version managers are not supported and we stronly advise everyone to follow the instructions below to use a system ruby.

However, those hard-to-diagnose problems are probably caused by non-"global" installation. I've setup rbenv to install for the GitLab user "globally", so as far as the user is aware, the rbenv Ruby is the only Ruby. I'm running in production with no problems. The only other gotcha was an ssh session for the GitLab user, but it was fixed by setting .bashrc as the file to source for Rbenv (perhaps a better fix would be to set it in ~/.ssh/environment). GitLab uses scripts which have a shebang of #!/usr/bin/env ruby, which with the global setting for rbenv returns the proper Ruby binary.

Anyway, more testing is always better, but I think this works cleaner than interfering with system Ruby.

@atomaka
Copy link
Collaborator

atomaka commented Feb 21, 2014

@andschwa I'm still considering my thoughts on this. We are introducing more dependencies, but abstract it away to the user. Hopefully, @sbadia will chip in when he gets a chance. It definitely will not beat the next GitLab release.

@andyleejordan
Copy link
Collaborator Author

@atomaka I'm glad it's actually being seriously considered. GitLab is fantastic, and I've been running a while now without a hitch. When school winds down I can fix the tests.

@igalic
Copy link
Collaborator

igalic commented Feb 21, 2014

I've been contemplating Brightbox' ruby for Ubuntu now for quite some time. It's compatible with Ubuntu's Ruby, but comes with sane versions. The problem of course in our setup comes when we have to install specific gems: Brightbox' ruby (which ever version we use) would have to be made the system default…

@nodoubleg
Copy link

I'm 👍 on this. This greatly simplifies life for those stuck with RHEL/CentOS. Changing the system ruby in RHEL can be quite painful and problematic.

@mc0e
Copy link

mc0e commented Feb 28, 2014

I'd have thought this should only be contemplated if there's also a suitable proposal on how to maintain security updates for the ruby version and ruby libraries that get installed this way.

rbenv::compile { 'gitlab/2.0.0-p353':
user => $git_user,
home => $git_home,
ruby => '2.0.0-p353',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is already outdated…

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could parameterize it for easy updating; however, it's a strong point of Puppet to be explicit about version updates. Might be slower, but it means everything gets tested first.

I'll be testing and updating it soon.

@sammcj
Copy link

sammcj commented Mar 25, 2014

+1
This would be excellent!

@andyleejordan
Copy link
Collaborator Author

Heh, I've been reading the discussion as it comes into my inbox, but haven't had much in the way of input. I'm still running my GitLab instance pretty flawlessly with my fork.

Anyway: the exec { 'install gitlab' } resource in install.pp could probably be replaced with @alup's bundle resource included in the rbenv module. I'm using it to setup stringer in a new module right now, I'll see how well it works out. One thing already is that the '--without development aws test database' flag might throw us off a bit.

Just a thought.

@andyleejordan
Copy link
Collaborator Author

Hey, I merged the latest master branch in and parameterized Ruby, setting its default to 2.1.1 (per the cookbook. Ran a clean provision on a Vagrant box and tested it out, all working with Gitlab 6.8.1, gitlab-shell 1.9.3, Ubuntu 12.04.4, Puppet 3.4.3 (haven't updated it to 3.5 yet but should be okay).

@andyleejordan
Copy link
Collaborator Author

I need to figure out what tests to update to get this passing.

@sbadia
Copy link
Owner

sbadia commented Apr 24, 2014

@andschwa Yep, it would be nice!
But i can update the test if it's a pita for you ;-)
Thanks for this change!

@andyleejordan
Copy link
Collaborator Author

@sbadia Dude actually that would be great, as it's been a while since I've written Puppet tests.

@andyleejordan
Copy link
Collaborator Author

If you want to use the latest Ruby version, you may have to manually update the ruby-build plugin of rbenv for the git user, as the rbenv module does not do it automatically. If rbenv install can't find the specified Ruby version to install, its failure is kinda ugly.

@andyleejordan
Copy link
Collaborator Author

And at least a bit of test coverage now exists.

@andyleejordan
Copy link
Collaborator Author

Ugh, okay apparently older versions of Ruby have a syntax error with the tests I added. Did I mention I really don't know Ruby tests?

We could also add 2.1.1, 2.1.0, etc.
These fail for reasons beyond our control (as far as I can tell)
@andyleejordan
Copy link
Collaborator Author

And all passing! Woot woot. Turning in for the night.

@sammcj
Copy link

sammcj commented Jul 15, 2014

Any update on this? - It'd be really nice to be able to deploy Gitlab 7

@andyleejordan
Copy link
Collaborator Author

@sammcj We have the tests passing now, with coverage of rbenv as well. I've tested it from scratch on Ubuntu 14.04, and use it in "production" (I'm the only user) on Ubuntu 12.04. More testing is always better, but I'm satisfied.

@andyleejordan
Copy link
Collaborator Author

You know, one thing we may want is a parameter gitlab_use_rbenv to completely disable this feature on an as-wanted basis.

@sammcj
Copy link

sammcj commented Jul 21, 2014

I'm assuming most people are moving to https://github.com/spuder/puppet-gitlab because of this.

@andyleejordan
Copy link
Collaborator Author

@sammcj I hadn't seen that project yet, looks interesting. By "this" do you mean it not being merged? I would if I could, but am not a maintainer. We'd need a new major release for sure, but that's fine.

@andyleejordan
Copy link
Collaborator Author

Spuder's module does not appear to take care of the Ruby environment whatsoever, which is a necessity because many distributions lack the required minimum Ruby version.

@sbadia
Copy link
Owner

sbadia commented Jul 21, 2014

Smoketests / functional testing in progress ( https://github.com/sbadia/vagrant-gitlab/tree/gitlab-rbenv ) thanks @andschwa !

@sammcj, Spencer's (spuder) module use only the omnibus package (recommended way to install from Gitlab team), but IMHO I don't really like this kind of bundleized
package is a pain with distribution packages… We work with the ruby
team in Debian to package all the gems for Gitlab (see
http://people.debian.org/~boutil/gitlab/) (green means ok, orange in progress
and black/red not packaged), maybe let the choice to the user to use
source/omnibus/distro package is a way to go (I want to go in this direction for
sbadia-gitlab, WDYT?). And with the great work of @andschwa it would be easy!

@sbadia
Copy link
Owner

sbadia commented Jul 21, 2014

@andschwa maintainer flag added :-p welcome \o/

@sbadia
Copy link
Owner

sbadia commented Jul 21, 2014

Testing ok! @andschwa can you rebase please? or i manage this? @atomaka / @igalic
it's ok for you?

rbenv::install { $git_user:
group => $git_user,
home => $git_home,

Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove this space

sbadia added a commit that referenced this pull request Aug 8, 2014
sbadia added a commit that referenced this pull request Aug 9, 2014
@sbadia sbadia mentioned this pull request Aug 9, 2014
@sbadia
Copy link
Owner

sbadia commented Aug 9, 2014

@andschwa @igalic Rebased and updated (acording Igor comments) in #170

Smoketests and spec ok! :-) We can merge ?

@sbadia
Copy link
Owner

sbadia commented Aug 10, 2014

Merged in #170 many thanks !!
And thanks for your perceptiveness :-D

@sbadia sbadia closed this Aug 10, 2014
@andyleejordan
Copy link
Collaborator Author

Woo! Awesome work all around people. Sorry I've been out of the loop, been finishing up my internship these last few weeks (exactly one more week to go). This is great stuff guys 😄

@sbadia Thanks for handling the rebase and things (and the maintainer flag)! I had those emails flagged, it's just been crazy busy. Will be hopping back into Puppet development in a week.

@andyleejordan andyleejordan deleted the rbenv branch August 10, 2014 18:31
@wazoo wazoo mentioned this pull request Nov 18, 2015
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.

7 participants