Merge pull request #223 from ripienaar/207.4

(#207) ensure mcollective is installed via choria module

.fixtures.yml

@@ -1,7 +1,6 @@
 fixtures:
   forge_modules:
     choria: "choria/mcollective_choria"
-    mcollective: "choria/mcollective"
     stdlib: "puppetlabs/stdlib"
     apt: "puppetlabs/apt"
     translate: "puppetlabs/translate"

manifests/init.pp

@@ -73,6 +73,7 @@
   Enum[debug, info, warn, error, fatal] $server_log_level = $log_level,
   Stdlib::Compat::Absolute_path $broker_logfile = $logfile,
   Stdlib::Compat::Absolute_path $server_logfile = $logfile,
+  Boolean $manage_mcollective = true,
 ) {
   if $manage_package_repo {
     class{"choria::repo":
@@ -102,6 +103,10 @@
     contain choria::service_disable
   }
 
+  if $manage_mcollective {
+    contain mcollective
+  }
+
   contain choria::install
   contain choria::scout_checks
   contain choria::scout_metrics

manifests/service.pp

@@ -11,8 +11,10 @@
         enable => true,
       }
 
-      # Ensures that module plugin changes will restart choria
-      Mcollective::Module_plugin <| |> ~> Service[$choria::server_service_name]
+      if $choria::manage_mcollective {
+        # Ensures that module plugin changes will restart choria
+        Mcollective::Module_plugin <| |> ~> Service[$choria::server_service_name]
+      }
 
       # Without this when a mcollective plugin is removed if purge is on the service
       # would not be restarted, unfortunate side effect that a client uninstall will

spec/classes/broker_spec.rb

@@ -6,6 +6,11 @@
     "rspec.puppet.com"
   end
 
+  let :params do
+    { manage_mcollective: false }
+  end
+
+
   on_supported_os.each do |os, facts|
     context "on #{os} " do
       let :facts do

spec/classes/init_spec.rb

@@ -11,6 +11,11 @@
       let :facts do
         facts
       end
+
+      let :params do
+        { manage_mcollective: false }
+      end
+
       context 'with all defaults' do
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_class("choria") }
@@ -70,15 +75,15 @@
       context "repo related tests" do
         context "should not manage the repo if its disabled" do
           let :params do
-            { manage_package_repo: false }
+            { manage_package_repo: false, manage_mcollective: false }
           end
 
           it { is_expected.to_not contain_class("choria::repo") }
         end
 
         context "with manage_package_repo set to true" do
           let :params do
-            { manage_package_repo: true }
+            { manage_package_repo: true, manage_mcollective: false }
           end
 
           if facts[:kernel] == "Linux" and facts[:os]["family"] != "Suse"
@@ -102,9 +107,10 @@
         context "with managed repos", if: (facts[:kernel] == "Linux" and facts[:os]["family"] != "Suse") do
           let(:params) do
             {
-              "manage_package_repo" => true,
-              "nightly_repo" => true,
-              "ensure" => "absent"
+              manage_package_repo: true,
+              nightly_repo: true,
+              ensure: "absent",
+              manage_mcollective: false
             }
           end
 
@@ -117,8 +123,9 @@
         context "with custom repo on CentOS", if: facts[:os]["name"] == "CentOS" do
           let(:params) do
             {
-              "manage_package_repo" => true,
-              "repo_baseurl" => "http://internal-mirror.com/choria"
+              manage_mcollective: false,
+              manage_package_repo: true,
+              repo_baseurl: "http://internal-mirror.com/choria"
             }
           end
 
@@ -132,8 +139,9 @@
         context "when managing an ubuntu bionic node", if: facts[:os]["release"]["major"] == "18.04" do
           let(:params) do
             {
-              "manage_package_repo" => true,
-              "repo_baseurl" => "http://internal-mirror.com/choria"
+              manage_mcollective: false,
+              manage_package_repo: true,
+              repo_baseurl: "http://internal-mirror.com/choria"
             }
           end
 
@@ -149,8 +157,9 @@
         context "when managing an ubuntu xenial node", if: facts[:os]["release"]["major"] == "16.04" do
           let :params do
             {
-              "manage_package_repo" => true,
-              "repo_baseurl" => "http://internal-mirror.com/choria"
+              manage_mcollective: false,
+              manage_package_repo: true,
+              repo_baseurl: "http://internal-mirror.com/choria"
             }
           end
 
@@ -181,7 +190,13 @@
         end
 
         context("with status file configured") do
-          let(:pre_condition) { 'class {"choria": statusfile => "/tmp/status", status_write_interval => 10}' }
+          let :params do
+            {
+              statusfile: "/tmp/status",
+              status_write_interval: 10,
+              manage_mcollective: false
+            }
+          end
 
           it "should support writing the status file" do
             is_expected.to contain_file("/etc/choria/server.conf")
@@ -194,6 +209,7 @@
       context "custom server config", if: facts[:kernel] == "Linux" do
         let :params do
           {
+            manage_mcollective: false,
             server_config: {
               "plugin.choria.security.certname_whitelist": ["user1", "user2"]
             }
@@ -209,6 +225,7 @@
       context "custom logging", if: facts[:kernel] == "Linux" do
         let :params do
          {
+           manage_mcollective: false,
            server_logfile: "/var/log/choria-server.log",
            server_log_level: "debug"
          }
@@ -222,7 +239,10 @@
 
       context "with package set to a specific version" do
         let :params do
-          { version: "1.2.3" }
+          {
+            manage_mcollective: false,
+            version: "1.2.3"
+          }
         end
 
         if facts[:kernel] == "SunOS" and facts[:os]["release"]["major"].to_i == 10
@@ -242,7 +262,10 @@
 
       context "with package ensure set to absent" do
         let :params do
-          { ensure: "absent" }
+          {
+            manage_mcollective: false,
+            ensure: "absent"
+          }
         end
 
         it { should compile.with_all_deps }
@@ -258,7 +281,10 @@
 
       context "with server enabled" do
         let :params do
-          { server: true }
+          {
+            manage_mcollective: false,
+            server: true
+          }
         end
 
         it { should compile.with_all_deps }
@@ -269,7 +295,10 @@
 
       context "with server disabled" do
         let :params do
-          { server: false }
+          {
+            manage_mcollective: false,
+            server: false
+          }
         end
 
         it { should compile.with_all_deps }