Merge pull request #37 from /issues/36

Issues/36

.ruby-version

@@ -1 +1 @@
-2.1.0
+2.1.1

CHANGES.md

@@ -2,9 +2,13 @@
 
 ## master (unreleased)
 
+## 0.4.1 2015-11-18 Jason Antman <[email protected]>
+
+* [#36](https://github.com/jantman/vagrant-r10k/issues/36) - Fix "no implicit conversion of nil into String" issue with puppet4, caused by ``config.r10k.manifest_file`` and/or ``config.r10k.manifests_path`` not being specified. This removes all use of these parameters, which were only used in log messages. It also removes validation that the Puppet provisioner's ``module_path`` matches that specified for r10k.
+
 ## 0.4.0 2015-10-29 Jason Antman <[email protected]>
 
-* [#13]() / [PR #34/35](https://github.com/jantman/vagrant-r10k/pull/35) - Upgrade r10k dependency to 1.5.1 (thanks to [@cdenneen](https://github.com/cdenneen) for the work).
+* [#13](https://github.com/jantman/vagrant-r10k/issues/13) / [PR #34/35](https://github.com/jantman/vagrant-r10k/pull/35) - Upgrade r10k dependency to 1.5.1 (thanks to [@cdenneen](https://github.com/cdenneen) for the work).
 
 ## 0.3.0 2015-09-04 Jason Antman <[email protected]>
 

README.md

@@ -132,6 +132,36 @@ The configuration for r10k and puppet would look like:
       puppet.module_path = ["puppet/modules", "puppet/vendor"]
     end
 
+### Usage With Puppet4 Environment-Based Provisioner
+
+Puppet4 discontinues use of the ``manifest_file`` and ``manifests_path`` parameters, and also makes the ``module_path`` parameter optional
+for Puppet. In cases where only ``environment`` and ``environment_path`` are specified, ``module_path`` will be parsed from the environment's
+``environment.conf``.
+
+vagrant-r10k does not handle parsing ``environment.conf``; you __must__ still specify the ``module_path`` for r10k to deploy modules into.
+
+Here is an example Vagrantfile (taken from vagrant-r10k's [acceptance tests](https://github.com/jantman/vagrant-r10k/blob/master/spec/acceptance/skeletons/puppet4/Vagrantfile))
+for use with environment-based configuration. Note that ``config.r10k.module_path`` is still specified. You can see the directory structure of this example
+[here](https://github.com/jantman/vagrant-r10k/tree/master/spec/acceptance/skeletons/puppet4).
+
+```
+Vagrant.configure("2") do |config|
+  config.vm.box = "vagrantr10kspec"
+  config.vm.network "private_network", type: "dhcp"
+
+  # r10k plugin to deploy puppet modules
+  config.r10k.puppet_dir = "environments/myenv"
+  config.r10k.puppetfile_path = "environments/myenv/Puppetfile"
+  config.r10k.module_path = "environments/myenv/modules"
+
+  # Provision the machine with the appliction
+  config.vm.provision "puppet" do |puppet|
+    puppet.environment = "myenv"
+    puppet.environment_path  = "environments"
+  end
+end
+```
+
 ## Getting Help
 
 Bug reports, feature requests, and pull requests are always welcome. At this time, the

lib/vagrant-r10k/action/deploy.rb

@@ -29,8 +29,6 @@ def call(env)
           @logger.debug("vagrant::r10k::deploy: env_dir_path=#{config[:env_dir_path]}")
           @logger.debug("vagrant::r10k::deploy: puppetfile_path=#{config[:puppetfile_path]}")
           @logger.debug("vagrant::r10k::deploy: module_path=#{config[:module_path]}")
-          @logger.debug("vagrant::r10k::deploy: manifests=#{config[:manifests]}")
-          @logger.debug("vagrant::r10k::deploy: manifest_file=#{config[:manifest_file]}")
           @logger.debug("vagrant::r10k::deploy: puppet_dir=#{config[:puppet_dir]}")
 
           deploy(env, config)

lib/vagrant-r10k/helpers.rb

@@ -97,15 +97,13 @@ def puppet_provisioner(env)
       #
       # @return [Hash]
       def r10k_config(env)
-        ret = { :manifests => nil, :module_path => nil, :manifest_file => nil }
+        ret = { :module_path => nil }
         ret[:env_dir_path] = env_dir(env)
         ret[:puppetfile_path] = puppetfile_path(env)
         prov = puppet_provisioner(env)
         return nil if prov.nil?
         ret[:module_path] = module_path(env, prov, ret[:env_dir_path])
         return nil if ret[:module_path].nil?
-        ret[:manifest_file] = File.join(ret[:env_dir_path], prov.config.manifest_file)
-        ret[:manifests] = File.join(ret[:env_dir_path], prov.config.manifests_path[1])
         ret[:puppet_dir] = File.join(ret[:env_dir_path], env[:machine].config.r10k.puppet_dir)
         ret
       end
@@ -123,8 +121,10 @@ def module_path(env, prov, env_dir_path)
           module_path = env[:machine].config.r10k.module_path
           if prov.config.module_path.is_a?(Array) and ! prov.config.module_path.include?(module_path)
             raise ErrorWrapper.new(RuntimeError.new("vagrant-r10k: module_path \"#{module_path}\" is not within the ones defined in puppet provisioner; please correct this condition"))
+          elsif prov.config.module_path.nil?
+            env[:ui].info "vagrant-r10k: Puppet provisioner module_path is nil, assuming puppet4 environment mode"
           elsif ! prov.config.module_path.is_a?(Array) and prov.config.module_path != module_path
-            raise ErrorWrapper.new(RuntimeError.new("vagrant-r10k: module_path \"#{module_path}\" is not the same as in puppet provisioner; please correct this condition"))
+            raise ErrorWrapper.new(RuntimeError.new("vagrant-r10k: module_path \"#{module_path}\" is not the same as in puppet provisioner (#{prov.config.module_path}); please correct this condition"))
           end
         # no modulepath explict set in config, build one from the provisioner config
         else

lib/vagrant-r10k/version.rb

@@ -1,6 +1,6 @@
 module VagrantPlugins
   # hold version number constant
   module R10k
-    VERSION = "0.4.0"
+    VERSION = "0.4.1"
   end
 end

spec/acceptance/skeletons/puppet4/Vagrantfile

@@ -0,0 +1,21 @@
+# -*- mode: ruby -*-
+# vi: set ft=ruby :
+
+#require 'vagrant-vmware-workstation'
+puts ENV
+
+Vagrant.configure("2") do |config|
+  config.vm.box = "vagrantr10kspec"
+  config.vm.network "private_network", type: "dhcp"
+
+  # r10k plugin to deploy puppet modules
+  config.r10k.puppet_dir = "environments/myenv"
+  config.r10k.puppetfile_path = "environments/myenv/Puppetfile"
+  config.r10k.module_path = "environments/myenv/modules"
+
+  # Provision the machine with the appliction
+  config.vm.provision "puppet" do |puppet|
+    puppet.environment = "myenv"
+    puppet.environment_path  = "environments"
+  end
+end

spec/acceptance/skeletons/puppet4/environments/myenv/Puppetfile

@@ -0,0 +1,12 @@
+# This is currently a noop but will be supported in the future.
+forge 'forge.puppetlabs.com'
+
+# v1.0.1 -> cdb8d7a186846b49326cec1cfb4623bd77529b04
+mod 'reviewboard',
+  :git => 'https://github.com/jantman/puppet-reviewboard.git',
+  :ref => 'v1.0.1'
+
+# 0.1.0 -> 3a504b5f66ebe1853bda4ee065fce18118958d84
+mod 'nodemeister',
+  :git => 'https://github.com/jantman/puppet-nodemeister.git',
+  :ref => '0.1.0'

spec/acceptance/skeletons/puppet4/environments/myenv/manifests/site.pp

@@ -0,0 +1 @@
+notify {'vagrant-r10k puppet run': }

spec/acceptance/skeletons/puppet4/gitcheck.sh

@@ -0,0 +1,20 @@
+#!/bin/bash
+# helper script to determine if a git clone is checked out to a
+# PR, tag, or branch, and then output some information about the status
+if [ $# -gt 0 ]; then
+    cd $1
+fi
+origin=$(git remote show -n origin | grep 'Fetch URL:' | awk '{print $3}')
+if tmp=$(git status | grep "refs/pull/origin"); then
+    foo=$(echo "$tmp" | awk '{print $4}' | awk -F / '{print $4}')
+    echo "PR: ${foo} @ $(git rev-parse HEAD) (origin: ${origin})"
+elif tagname=$(git describe --exact-match --tags $(git log -n1 --pretty='%h') 2>/dev/null); then
+    echo "tag: ${tagname} @ $(git rev-parse HEAD) (origin: ${origin})"
+elif git symbolic-ref -q HEAD &>/dev/null; then
+    branch_name=$(git symbolic-ref -q HEAD)
+    branch_name=${branch_name##refs/heads/}
+    branch_name=${branch_name:-HEAD}
+    echo "branch: ${branch_name} @ $(git rev-parse HEAD) (origin: ${origin})"
+else
+    echo "sha: $(git rev-parse HEAD) (origin: ${origin})"
+fi

spec/acceptance/vagrant-r10k/vagrant-r10k_spec.rb

@@ -50,6 +50,37 @@
     end
   end
 
+  describe 'configured correctly - puppet4 environment' do
+    before do
+      setup_before('puppet4', options)
+    end
+    after do
+      assert_execute("vagrant", "destroy", "--force")
+    end
+
+    it 'deploys Puppetfile modules in an environment' do
+      status("Test: vagrant up")
+      up_result = assert_execute('vagrant', 'up', "--provider=#{provider}", '--debug')
+      ensure_successful_run(up_result, environment.workdir, expect_building=false, moddir='environments/myenv/modules', pupfile='environments/myenv/Puppetfile', do_ensure_ran=false)
+      expect(up_result.stdout).to include('Running provisioner: puppet')
+      expect(up_result.stdout).to include('Running Puppet with environment myenv')
+      expect(up_result.stdout).to include('vagrant-r10k puppet run')
+      status("Test: reviewboard module")
+      rb_dir = File.join(environment.workdir, 'environments', 'myenv', 'modules', 'reviewboard')
+      expect(File.directory?(rb_dir)).to be_truthy
+      rb_result = assert_execute('bash', 'gitcheck.sh', 'environments/myenv/modules/reviewboard')
+      expect(rb_result).to exit_with(0)
+      expect(rb_result.stdout).to match(/tag: v1\.0\.1 @ cdb8d7a186846b49326cec1cfb4623bd77529b04 \(origin: https:\/\/github\.com\/jantman\/puppet-reviewboard\.git\)/)
+
+      status("Test: nodemeister module")
+      nm_dir = File.join(environment.workdir, 'environments', 'myenv', 'modules', 'nodemeister')
+      expect(File.directory?(nm_dir)).to be_truthy
+      nm_result = assert_execute('bash', 'gitcheck.sh', 'environments/myenv/modules/nodemeister')
+      expect(nm_result).to exit_with(0)
+      expect(nm_result.stdout).to match(/tag: 0\.1\.0 @ 3a504b5f66ebe1853bda4ee065fce18118958d84 \(origin: https:\/\/github\.com\/jantman\/puppet-nodemeister\.git\)/)
+    end
+  end
+
   describe 'destroy when configured correctly' do
     before do
       setup_before('correct', options)
@@ -103,7 +134,7 @@
       status("Test: vagrant up")
       up_result = execute('vagrant', 'up', "--provider=#{provider}")
       expect(up_result).to exit_with(1)
-      expect(up_result.stderr).to match('RuntimeError: vagrant-r10k: module_path "puppet/NOTmodules" is not the same as in puppet provisioner; please correct this condition')
+      expect(up_result.stderr).to match(/RuntimeError: vagrant-r10k: module_path "puppet\/NOTmodules" is not the same as in puppet provisioner \(puppet\/modules\); please correct this condition/)
       ensure_r10k_didnt_run(up_result, environment.workdir)
       ensure_puppet_didnt_run(up_result)
     end
@@ -190,7 +221,7 @@ def setup_before(skel_name, options)
   end
 
   # checks for a successful up run with r10k deployment and puppet provisioning
-  def ensure_successful_run(up_result, workdir)
+  def ensure_successful_run(up_result, workdir, expect_building=true, moddir='puppet/modules', pupfile='puppet/Puppetfile', do_ensure_ran=true)
     expect(up_result).to exit_with(0)
     # version checks
     expect(up_result.stderr).to include('global:   - r10k = 1.5.1')
@@ -207,16 +238,20 @@ def ensure_successful_run(up_result, workdir)
     # modulegetter BEFORE ConfigValidate
     expect(up_result.stderr).to match(%r"(?!ConfigValidate)+default: vagrant-r10k: Deploy finished.*Vagrant::Action::Builtin::ConfigValidate"m), "modulegetter runs before ConfigValidate"
     # other checks
-    expect(up_result.stdout).to include('vagrant-r10k: Building the r10k module path with puppet provisioner module_path "puppet/modules"')
-    expect(up_result.stdout).to include("vagrant-r10k: Beginning r10k deploy of puppet modules into #{workdir}/puppet/modules using #{workdir}/puppet/Puppetfile")
+    if expect_building then
+      expect(up_result.stdout).to include('vagrant-r10k: Building the r10k module path with puppet provisioner module_path "puppet/modules"')
+    end
+    expect(up_result.stdout).to include("vagrant-r10k: Beginning r10k deploy of puppet modules into #{workdir}/#{moddir} using #{workdir}/#{pupfile}")
     expect(up_result.stdout).to include('vagrant-r10k: Deploy finished')
     # file tests
-    expect(File).to exist("#{workdir}/puppet/modules/reviewboard/Modulefile")
-    expect(File).to exist("#{workdir}/puppet/modules/nodemeister/Modulefile")
-    expect(File).to exist("#{workdir}/puppet/modules/nodemeister/manifests/init.pp")
+    expect(File).to exist("#{workdir}/#{moddir}/reviewboard/Modulefile")
+    expect(File).to exist("#{workdir}/#{moddir}/nodemeister/Modulefile")
+    expect(File).to exist("#{workdir}/#{moddir}/nodemeister/manifests/init.pp")
 
     # ensure puppet ran
-    ensure_puppet_ran(up_result)
+    if do_ensure_ran then
+      ensure_puppet_ran(up_result)
+    end
   end
 
   # ensure that r10k didnt run

spec/unit/action_deploy_spec.rb

@@ -145,7 +145,6 @@
                       :puppetfile_path => 'puppetfile/path',
                       :module_path => 'module/path',
                       :manifests => 'manifests',
-                      :manifest_file => 'manifest/file',
                       :puppet_dir => 'puppet/dir',
                     }}
       let(:pf_dbl) { double }
@@ -184,8 +183,6 @@
         expect(logger).to receive(:debug).once.ordered.with("vagrant::r10k::deploy: env_dir_path=env/dir/path")
         expect(logger).to receive(:debug).once.ordered.with("vagrant::r10k::deploy: puppetfile_path=puppetfile/path")
         expect(logger).to receive(:debug).once.ordered.with("vagrant::r10k::deploy: module_path=module/path")
-        expect(logger).to receive(:debug).once.ordered.with("vagrant::r10k::deploy: manifests=manifests")
-        expect(logger).to receive(:debug).once.ordered.with("vagrant::r10k::deploy: manifest_file=manifest/file")
         expect(logger).to receive(:debug).once.ordered.with("vagrant::r10k::deploy: puppet_dir=puppet/dir")
         expect(subject).to receive(:deploy).with(env, config).once
         expect(app).to receive(:call).once.with(env)

spec/unit/helpers_spec.rb

@@ -214,7 +214,6 @@
     context 'no methods return nil' do
       before do
         prov_dbl = double
-        prov_dbl.stub_chain(:config, :manifest_file).and_return('manifest/file')
         prov_dbl.stub_chain(:config, :manifests_path).and_return([0, 'manifests/path'])
         allow_any_instance_of(VagrantPlugins::R10k::Helpers).to receive(:env_dir).and_return('/my/env/dir')
         allow_any_instance_of(VagrantPlugins::R10k::Helpers).to receive(:puppetfile_path).and_return('puppet/file/path')
@@ -228,8 +227,6 @@
                                          :env_dir_path    => '/my/env/dir',
                                          :puppetfile_path => 'puppet/file/path',
                                          :module_path     => 'module/path',
-                                         :manifest_file   => '/my/env/dir/manifest/file',
-                                         :manifests       => '/my/env/dir/manifests/path',
                                          :puppet_dir      => '/my/env/dir/puppet/dir',
                                        })
       end
@@ -284,7 +281,17 @@
         env = {:machine => double, :ui => double}
         allow(env[:ui]).to receive(:info).with("vagrant-r10k: Building the r10k module path with puppet provisioner module_path \"\". (if module_path is an array, first element is used)")
         env[:machine].stub_chain(:config, :r10k, :module_path).and_return('r10kmodule/path')
-        expect { module_path(env, prov_dbl, '/env/dir/path') }.to raise_error(VagrantPlugins::R10k::Helpers::ErrorWrapper, /module_path "r10kmodule\/path" is not the same as in puppet provisioner; please correct this condition/)
+        expect { module_path(env, prov_dbl, '/env/dir/path') }.to raise_error(VagrantPlugins::R10k::Helpers::ErrorWrapper, /module_path "r10kmodule\/path" is not the same as in puppet provisioner \(module\/path\); please correct this condition/)
+      end
+    end
+    context 'config.r10k.module_path set and provisioner module_path nil' do
+      it 'returns the joined module_path' do
+        prov_dbl = double
+        prov_dbl.stub_chain(:config, :module_path).and_return(nil)
+        env = {:machine => double, :ui => double}
+        allow(env[:ui]).to receive(:info).with("vagrant-r10k: Puppet provisioner module_path is nil, assuming puppet4 environment mode")
+        env[:machine].stub_chain(:config, :r10k, :module_path).and_return('module/path')
+        expect(module_path(env, prov_dbl, '/env/dir/path')).to eq('/env/dir/path/module/path')
       end
     end
   end