blt04 / puppet-rvm

A puppet module for installing and using RVM (Ruby Version Manager)
Other
242 stars 280 forks source link

Use of run stage causing ordering problems #33

Open ssnodgra opened 12 years ago

ssnodgra commented 12 years ago

Brandon, I've made some substantial modifications to puppet-rvm to add both support for using an RPM package as well as selinux support (everything is optional/backwards compatible), which I will make available soon in a pull request after additional testing. In the meantime, my testing has turned up a problem with run stages. My puppet environment uses a "pre" run stage to set up yum repos, otherwise every single package install would depend on those repos. The problem is that there is no relationship between my run stage and the one in your module, so it accidentally trumps my "pre" stage and then tries to install a bunch of RPMs from yum repos that don't exist yet, thus causing my puppet config to blow up when building a new box.

Can you explain the rationale behind the run stage? It seems like something that is better handled with dependencies, and I'm not sure that having a module implement a run stage is a good idea, but it's entirely possible that I haven't considered all the angles since I didn't write your module. :)

Thanks.

blt04 commented 12 years ago

The puppet providers included in this module (rvm_system_ruby, rvm_gemset, and rvm_gem) need the RVM binary located in /usr/local/rvm/bin/rvm. This binary is installed by the RVM installer. Unfortunately, puppet determines if a provider is _suitable_ (able to determine if the given resource is installed) before provider dependencies are calculated and installed. This produces a chicken and egg problem, as include rvm (technically exec system-rvm) installs RVM, but rvm_system_ruby is not suitable before RVM is installed. You can make rvm_system_ruby depend on exec system-rvm, but this won't help because provider suitability is determined before dependencies are considered. Determining suitability after dependencies are installed is called late binding and is not (yet) supported by puppet.

There are two supported ways for dealing with this problem:

  1. Two-pass install. Install RVM on the first puppet pass. Install rubies, gemsets and gems on a second pass. To achieve this you can wrap everything RVM related (except include rvm) in an if block (if $rvm_installed == "true").
  2. Run stages. Provider suitability is reevaluated for every run stage. So if we install RVM before the main stage, /usr/local/rvm/bin/rvm will be available when determining provider suitability in the main stage.

I agree that having a module implement a run stage is probably not a great idea. I was trying to make the module easy to use without requiring too much configuration in manifest files.

So I guess we have a few options:

  1. Remove run stages from this module, and include instructions in the README about how to set up run stages.
  2. Somehow tell the rvm-install stage to depend on the pre stage in your manifest. I haven't tested this, but do you think something like this would work (probably not):

    include rvm
    Stage['rvm-install'] <- Stage['pre']

    Or maybe we could allow passing parameters to the rvm module that would allow configuring the rvm run stage:

    class {'rvm': $before_stage => 'main', $after_stage => 'pre'}

I'm just thinking out loud. Thoughts?

ssnodgra commented 12 years ago

So I've been using method #1 (the two-pass install) as you describe above. What confuses me is the theory that run stages solve this, because I'm not seeing that. When I removed the "if $rvm_installed" logic from my node and try to rebuild it, with the rvm install in an early run stage, the whole manifest still blows up with:

puppet-agent[1214]: Failed to apply catalog: Could not find a default provider for rvm_system_ruby

Have you actually had success with getting run stages to install rvm before the manifest blows up due to lack of a provider? It's not working for me on puppet 2.6.12.

blt04 commented 12 years ago

Yes I have had success with run stages. It works great for me on 2.7.10. I'll admit I haven't tested it with older versions of puppet.

I do still see the "Could not find a default provider for rvm_system_ruby" when running in --noop mode. This is an unavoidable side effect of making a provider suitable in an earlier run stage. When actually running puppet, the error messages do not show up and everything is installed in one pass.

Also, you may want to make sure you have pulled the latest changes from master, as I refactored it slightly a few days ago due to run stages not behaving correctly in 2.7.1.

ssnodgra commented 12 years ago

Interesting, maybe there is a difference between 2.6.12 and 2.7.10 with respect to run stages and providers. I'll have to test 2.7 at some point; for now I will continue to rely on the two-pass method. What I did to solve the run stage problem is parameterize the run stage used (defaulting to rvm-install). That way I can elect to supply my own run stage that has whatever ordering I want. That patch will be included with the rest as soon as I send you a pull request (no later than tomorrow hopefully).

I do have the latest changes - I applaud the change of entry point into the module. Including "rvm::system" instead of "rvm" was kind of odd.

simondean commented 12 years ago

It might be worth replacing:

commands :rvmcmd => "/usr/local/rvm/bin/rvm"

with:

optional_commands :rvmcmd => "/usr/local/rvm/bin/rvm"

That might prevent puppet from checking to see whether the rvm bin exists, helping you avoid run stages.

alvagante commented 11 years ago

My 2 cents to the discussion , since I got into some problems due to the stages usage in the module. My starting point was that, for some dependency issues, I wanted to get rid of the stages part , which IMHO should not stay in a module (but here actually it seems unavoidable if you want a clean install in one single Puppet run).

and explicit dependencies when using the types (precisely the rvm_system_ruby as a sane usage of the other types requires a ruby installed):

include rvm

rvm_system_ruby {
  'ruby-1.9.2-p290':
    ensure => 'present',
    default_use => true,
    require => Class['rvm::system'],
 }

rvm_gemset {
  "ruby-1.9.2-p290@myproject":
    ensure => present,
    require => Rvm_system_ruby['ruby-1.9.2-p290'],
}

rvm_gem {
  'ruby-1.9.2-p290@myproject/bundler':
    ensure => '1.0.21',
    require => Rvm_gemset['ruby-1.9.2-p290@myproject'],
}

This approach worked well (not tested it widely), even if it requires the user to explicitly set dependencies, but that's something that it has already to be done for rvm_gem / rvm_gemsets types, so it would just involve one more require (require => Class['rvm::system'],) that can be quickly documented.

Code is here: https://github.com/example42/puppet-rvm/commit/be95623b6fec89d6f606551cfdcb2a395da013e7 If you want I can provide a PR for that (from a cleaner repo, since other changes had been done there)