Merge pull request #41 from fhrbek/conn_validator_refactoring

Support for remote puppetdb

README.md

@@ -276,6 +276,13 @@ If true, the module will overwrite the puppet master's routes file to configure
 
 If true, the module will manage the puppet master's storeconfig settings (defaults to true).
 
+####`manage_config`
+If true, the module will store values from puppetdb_server and puppetdb_port parameters in the puppetdb configuration file.
+If false, an existing puppetdb configuration file will be used to retrieve server and port values.
+
+####`strict_validation`
+If true, the module will fail if puppetdb is not reachable, otherwise it will preconfigure puppetdb without checking.
+
 ####`puppet_confdir`
 
 Puppet's config directory (defaults to `/etc/puppet`).

lib/puppet/provider/puppetdb_conn_validator/puppet_https.rb

@@ -1,36 +1,17 @@
-require 'puppet/network/http_pool'
+# See: #10295 for more details.
+#
+# This is a workaround for bug: #4248 whereby ruby files outside of the normal
+# provider/type path do not load until pluginsync has occured on the puppetmaster
+#
+# In this case I'm trying the relative path first, then falling back to normal
+# mechanisms. This should be fixed in future versions of puppet but it looks
+# like we'll need to maintain this for some time perhaps.
+$LOAD_PATH.unshift(File.join(File.dirname(__FILE__),"..","..",".."))
+require 'puppet/util/puppetdb_validator'
 
 # This file contains a provider for the resource type `puppetdb_conn_validator`,
 # which validates the puppetdb connection by attempting an https connection.
 
-# Utility method; attempts to make an https connection to the puppetdb server.
-# This is abstracted out into a method so that it can be called multiple times
-# for retry attempts.
-#
-# @return true if the connection is successful, false otherwise.
-def attempt_connection
-  begin
-    host = resource[:puppetdb_server]
-    port = resource[:puppetdb_port]
-
-    # All that we care about is that we are able to connect successfully via
-    # https, so here we're simpling hitting a somewhat arbitrary low-impact URL
-    # on the puppetdb server.
-    path = "/metrics/mbean/java.lang:type=Memory"
-    headers = {"Accept" => "application/json"}
-    conn = Puppet::Network::HttpPool.http_instance(host, port, true)
-    response = conn.get(path, headers)
-    unless response.kind_of?(Net::HTTPSuccess)
-      Puppet.err "Unable to connect to puppetdb server (#{host}:#{port}): [#{response.code}] #{response.msg}"
-      return false
-    end
-    return true
-  rescue Errno::ECONNREFUSED => e
-    Puppet.notice "Unable to connect to puppetdb server (#{host}:#{port}): #{e.inspect} "
-    return false
-  end
-end
-
 Puppet::Type.type(:puppetdb_conn_validator).provide(:puppet_https) do
   desc "A provider for the resource type `puppetdb_conn_validator`,
         which validates the puppetdb connection by attempting an https
@@ -41,7 +22,7 @@ Puppet::Type.type(:puppetdb_conn_validator).provide(:puppet_https) do
     start_time = Time.now
     timeout = resource[:timeout]
 
-    success = attempt_connection
+    success = validator.attempt_connection
     unless success
       while (Time.now - start_time) < timeout
         # It can take several seconds for the puppetdb server to start up;
@@ -50,7 +31,7 @@ Puppet::Type.type(:puppetdb_conn_validator).provide(:puppet_https) do
         # seconds until the configurable timeout has expired.
         Puppet.notice("Failed to connect to puppetdb; sleeping 2 seconds before retry")
         sleep 2
-        success = attempt_connection
+        success = validator.attempt_connection
       end
     end
 
@@ -65,8 +46,13 @@ Puppet::Type.type(:puppetdb_conn_validator).provide(:puppet_https) do
     # If `#create` is called, that means that `#exists?` returned false, which
     # means that the connection could not be established... so we need to
     # cause a failure here.
-    raise Puppet::Error, "Unable to connect to puppetdb server! (#{resource[:puppetdb_server]}:#{resource[:puppetdb_port]})"
+    raise Puppet::Error, "Unable to connect to puppetdb server! (#{@validator.puppetdb_server}:#{@validator.puppetdb_port})"
   end
 
+  # @api private
+  def validator
+    @validator ||= Puppet::Util::PuppetdbValidator.new(resource[:puppetdb_server], resource[:puppetdb_port])
+  end
 
 end
+

lib/puppet/util/puppetdb_validator.rb

@@ -0,0 +1,39 @@
+require 'puppet/network/http_pool'
+
+module Puppet
+  module Util
+    class PuppetdbValidator
+      attr_reader :puppetdb_server
+      attr_reader :puppetdb_port
+
+      def initialize(puppetdb_server, puppetdb_port)
+        @puppetdb_server = puppetdb_server
+        @puppetdb_port = puppetdb_port
+      end
+
+      # Utility method; attempts to make an https connection to the puppetdb server.
+      # This is abstracted out into a method so that it can be called multiple times
+      # for retry attempts.
+      #
+      # @return true if the connection is successful, false otherwise.
+      def attempt_connection
+        # All that we care about is that we are able to connect successfully via
+        # https, so here we're simpling hitting a somewhat arbitrary low-impact URL
+        # on the puppetdb server.
+        path = "/metrics/mbean/java.lang:type=Memory"
+        headers = {"Accept" => "application/json"}
+        conn = Puppet::Network::HttpPool.http_instance(@puppetdb_server, @puppetdb_port, true)
+        response = conn.get(path, headers)
+        unless response.kind_of?(Net::HTTPSuccess)
+          Puppet.notice "Unable to connect to puppetdb server (#{@puppetdb_server}:#{@puppetdb_port}): [#{response.code}] #{response.msg}"
+          return false
+        end
+        return true
+      rescue Exception => e
+        Puppet.notice "Unable to connect to puppetdb server (#{@puppetdb_server}:#{@puppetdb_port}): #{e.message}"
+        return false
+      end
+    end
+  end
+end
+

manifests/init.pp

@@ -140,7 +140,7 @@ class puppetdb(
   validate_re ($report_ttl_real, ['^(\d)+[s,m,d]$'], "report_ttl is <${report_ttl}> which does not match the regex validation")
 
   if ($manage_redhat_firewall != undef) {
-    notify {'Deprecation notice: `$manage_redhat_firewall` has been deprecated in `puppetdb` class and will be removed in a future versions. Use $open_ssl_listen_port and $open_postgres_port instead.':}
+    notify {'Deprecation notice: `$manage_redhat_firewall` has been deprecated in `puppetdb` class and will be removed in a future version. Use $open_ssl_listen_port and $open_postgres_port instead.':}
   }
 
   class { 'puppetdb::server':

manifests/master/config.pp

@@ -17,6 +17,12 @@
 #                         file to configure it to use puppetdb (defaults to true)
 #   ['manage_storeconfigs'] - If true, the module will manage the puppet master's
 #                         storeconfig settings (defaults to true)
+#   ['manage_config']   - If true, the module will store values from puppetdb_server
+#                         and puppetdb_port parameters in the puppetdb configuration file.
+#                         If false, an existing puppetdb configuration file will be used
+#                         to retrieve server and port values.
+#   ['strict_validation'] - If true, the module will fail if puppetdb is not reachable,
+#                         otherwise it will preconfigure puppetdb without checking.
 #   ['puppet_confdir']  - Puppet's config directory; defaults to /etc/puppet
 #   ['puppet_conf']     - Puppet's config file; defaults to /etc/puppet/puppet.conf
 #   ['puppetdb_version']   - The version of the `puppetdb` package that should
@@ -51,6 +57,8 @@ class puppetdb::master::config(
   $puppetdb_port            = 8081,
   $manage_routes            = true,
   $manage_storeconfigs      = true,
+  $manage_config            = true,
+  $strict_validation        = true,
   $puppet_confdir           = $puppetdb::params::puppet_confdir,
   $puppet_conf              = $puppetdb::params::puppet_conf,
   $puppetdb_version         = $puppetdb::params::puppetdb_version,
@@ -64,27 +72,29 @@ class puppetdb::master::config(
     ensure => $puppetdb_version,
   }
 
-  # Validate the puppetdb connection.  If we can't connect to puppetdb then we
+  if ($strict_validation) {
-  # *must* not perform the other configuration steps, or else
+	  # Validate the puppetdb connection.  If we can't connect to puppetdb then we
-  puppetdb_conn_validator { 'puppetdb_conn':
+	  # *must* not perform the other configuration steps, or else
-    puppetdb_server => $puppetdb_server,
+	  puppetdb_conn_validator { 'puppetdb_conn':
-    puppetdb_port   => $puppetdb_port,
+	    puppetdb_server => $manage_config ? { true => $puppetdb_server, default => undef },
-    timeout         => $puppetdb_startup_timeout,
+	    puppetdb_port   => $manage_config ? { true => $puppetdb_port, default => undef },
-    require         => Package[$terminus_package],
+	    timeout         => $puppetdb_startup_timeout,
-  }
+	    require         => Package[$terminus_package],
+	  }
 
-  # This is a bit of puppet chicanery that allows us to create a
+	  # This is a bit of puppet chicanery that allows us to create a
-  # conditional dependency.  Basically, we're saying that "if the PuppetDB
+	  # conditional dependency.  Basically, we're saying that "if the PuppetDB
-  # service is being managed in this same catalog, it needs to come before
+	  # service is being managed in this same catalog, it needs to come before
-  # this validator."
+	  # this validator."
-  Service<|title == 'puppetdb'|> -> Puppetdb_conn_validator['puppetdb_conn']
+	  Service<|title == $puppetdb::params::puppetdb_service|> -> Puppetdb_conn_validator['puppetdb_conn']
+  }
 
   # Conditionally manage the `routes.yaml` file.  Restart the puppet service
   # if changes are made.
   if ($manage_routes) {
     class { 'puppetdb::master::routes':
       puppet_confdir => $puppet_confdir,
-      require        => Puppetdb_conn_validator['puppetdb_conn'],
+      require        => $strict_validation ? { true => Puppetdb_conn_validator['puppetdb_conn'], default => Package[$terminus_package] },
     }
   }
 
@@ -93,18 +103,20 @@ class puppetdb::master::config(
   # it polls it automatically.
   if ($manage_storeconfigs) {
     class { 'puppetdb::master::storeconfigs':
-      puppet_conf => $puppet_conf,
+	    puppet_conf => $puppet_conf,
-      require     => Puppetdb_conn_validator['puppetdb_conn'],
+      require     => $strict_validation ? { true => Puppetdb_conn_validator['puppetdb_conn'], default => Package[$terminus_package] },
-    }
+   }
   }
 
-  # Manage the `puppetdb.conf` file.  Restart the puppet service if changes
+  if ($manage_config) {
-  # are made.
+	  # Manage the `puppetdb.conf` file.  Restart the puppet service if changes
-  class { 'puppetdb::master::puppetdb_conf':
+	  # are made.
-    server         => $puppetdb_server,
+	  class { 'puppetdb::master::puppetdb_conf':
-    port           => $puppetdb_port,
+	    server         => $puppetdb_server,
-    puppet_confdir => $puppet_confdir,
+	    port           => $puppetdb_port,
-    require        => Puppetdb_conn_validator['puppetdb_conn'],
+	    puppet_confdir => $puppet_confdir,
+	    require        => $strict_validation ? { true => Puppetdb_conn_validator['puppetdb_conn'], default => Package[$terminus_package] },
+	  }
   }
 
   if ($restart_puppet) {
@@ -116,8 +128,14 @@ class puppetdb::master::config(
       }
     }
 
-    Class['puppetdb::master::puppetdb_conf'] ~> Service[$puppet_service_name]
+    if ($manage_config) {
-    Class['puppetdb::master::routes']        ~> Service[$puppet_service_name]
+      Class['puppetdb::master::puppetdb_conf'] ~> Service[$puppet_service_name]
+    }
+
+    if ($manage_routes) {
+      Class['puppetdb::master::routes'] ~> Service[$puppet_service_name]
+    }
+
   }
 
 }

spec/unit/util/puppetdb_validator_spec.rb

@@ -0,0 +1,56 @@
+require 'spec_helper'
+require 'puppet/util/puppetdb_validator'
+
+describe 'Puppet::Util::PuppetdbValidator' do
+
+  before do
+    response_ok = stub()
+    response_ok.stubs(:kind_of?).with(Net::HTTPSuccess).returns(true)
+    response_not_found = stub()
+    response_not_found.stubs(:kind_of?).with(Net::HTTPSuccess).returns(false)
+    response_not_found.stubs(:code).returns(404)
+    response_not_found.stubs(:msg).returns('Not found')
+
+    conn_ok = stub()
+    conn_ok.stubs(:get).with('/metrics/mbean/java.lang:type=Memory', {"Accept" => "application/json"}).returns(response_ok)
+
+    conn_not_found = stub()
+    conn_not_found.stubs(:get).with('/metrics/mbean/java.lang:type=Memory', {"Accept" => "application/json"}).returns(response_not_found)
+
+    Puppet::Network::HttpPool.stubs(:http_instance).raises('Unknown host')
+    Puppet::Network::HttpPool.stubs(:http_instance).with('mypuppetdb.com', 8080, true).raises('Connection refused')
+    Puppet::Network::HttpPool.stubs(:http_instance).with('mypuppetdb.com', 8081, true).returns(conn_ok)
+    Puppet::Network::HttpPool.stubs(:http_instance).with('wrongserver.com', 8081, true).returns(conn_not_found)
+  end
+
+  it 'returns true if connection succeeds' do
+    validator = Puppet::Util::PuppetdbValidator.new('mypuppetdb.com', 8081)
+    validator.attempt_connection.should be_true
+  end
+
+  it 'returns false and issues an appropriate notice if connection is refused' do
+    puppetdb_server = 'mypuppetdb.com'
+    puppetdb_port = 8080
+    validator = Puppet::Util::PuppetdbValidator.new(puppetdb_server, puppetdb_port)
+    Puppet.expects(:notice).with("Unable to connect to puppetdb server (#{puppetdb_server}:#{puppetdb_port}): Connection refused")
+    #Puppet.expects(:notice).with("Unable to connect to puppetdb server (#{puppetdb_server}:#{puppetdb_port}): [404] Not found")
+    validator.attempt_connection.should be_false    
+  end
+
+  it 'returns false and issues an appropriate notice if connection succeeds but puppetdb is not available' do
+    puppetdb_server = 'wrongserver.com'
+    puppetdb_port = 8081
+    validator = Puppet::Util::PuppetdbValidator.new(puppetdb_server, puppetdb_port)
+    Puppet.expects(:notice).with("Unable to connect to puppetdb server (#{puppetdb_server}:#{puppetdb_port}): [404] Not found")
+    validator.attempt_connection.should be_false    
+  end
+
+
+  it 'returns false and issues an appropriate notice if host:port is unreachable or does not exist' do
+    puppetdb_server = 'non-existing.com'
+    puppetdb_port = nil
+    validator = Puppet::Util::PuppetdbValidator.new(puppetdb_server, puppetdb_port)
+    Puppet.expects(:notice).with("Unable to connect to puppetdb server (#{puppetdb_server}:#{puppetdb_port}): Unknown host")
+    validator.attempt_connection.should be_false    
+  end
+end