(#1886) Clean up `node clean` for merge.

This includes various style changes, and assorted fixes to testing.

Paired-With: Matt Robinson

2 files changed, 69 insertions, 72 deletions

diff --git a/lib/puppet/face/ca.rb b/lib/puppet/face/ca.rb
index e643530f0..00591d637 100644
--- a/lib/puppet/face/ca.rb
+++ b/lib/puppet/face/ca.rb
@@ -6,21 +6,21 @@ Puppet::Face.define(:ca, '0.1.0') do
 
   summary "Local Puppet Certificate Authority management."
 
-  description <<TEXT
-This provides local management of the Puppet Certificate Authority.
+  description <<-TEXT
+    This provides local management of the Puppet Certificate Authority.
 
-You can use this subcommand to sign outstanding certificate requests, list
-and manage local certificates, and inspect the state of the CA.
-TEXT
+    You can use this subcommand to sign outstanding certificate requests, list
+    and manage local certificates, and inspect the state of the CA.
+  TEXT
 
   action :list do
     summary "List certificates and/or certificate requests."
 
-    description <<-end
-This will list the current certificates and certificate signing requests
-in the Puppet CA.  You will also get the fingerprint, and any certificate
-verification failure reported.
-    end
+    description <<-TEXT
+      This will list the current certificates and certificate signing requests
+      in the Puppet CA.  You will also get the fingerprint, and any certificate
+      verification failure reported.
+    TEXT
 
     option "--[no-]all" do
       summary "Include all certificates and requests."
@@ -37,12 +37,12 @@ verification failure reported.
     option "--subject PATTERN" do
       summary "Only list if the subject matches PATTERN."
 
-      description <<TEXT
-Only include certificates or requests where subject matches PATTERN.
+      description <<-TEXT
+        Only include certificates or requests where subject matches PATTERN.
 
-PATTERN is interpreted as a regular expression, allowing complex
-filtering of the content.
-TEXT
+        PATTERN is interpreted as a regular expression, allowing complex
+        filtering of the content.
+      TEXT
     end
 
     when_invoked do |options|
diff --git a/lib/puppet/face/node/clean.rb b/lib/puppet/face/node/clean.rb
index 10d6239ba..a4df1bfaf 100644
--- a/lib/puppet/face/node/clean.rb
+++ b/lib/puppet/face/node/clean.rb
@@ -1,29 +1,29 @@
-Puppet::Indirector::Face.define(:node, '0.0.1') do
+Puppet::Face.define(:node, '0.0.1') do
   action(:clean) do
     option "--[no-]unexport" do
       summary "Unexport exported resources"
     end
-  
+
     summary "Clean up everything a puppetmaster knows about a node"
-    
     arguments "<host1> [<host2> ...]"
-
     description <<-EOT
-This includes
-
- * Signed certificates ($vardir/ssl/ca/signed/node.domain.pem)
- * Cached facts ($vardir/yaml/facts/node.domain.yaml)
- * Cached node stuff ($vardir/yaml/node/node.domain.yaml)
- * Reports ($vardir/reports/node.domain)
- * Stored configs: it can either remove all data from an host in your storedconfig
- database, or with --unexport turn every exported resource supporting ensure to absent
- so that any other host checking out their config can remove those exported configurations.
-
-This will unexport exported resources of a
-host, so that consumers of these resources can remove the exported
-resources and we will safely remove the node from our
-infrastructure.
-EOT
+      This includes
+
+       * Signed certificates ($vardir/ssl/ca/signed/node.domain.pem)
+       * Cached facts ($vardir/yaml/facts/node.domain.yaml)
+       * Cached node stuff ($vardir/yaml/node/node.domain.yaml)
+       * Reports ($vardir/reports/node.domain)
+       * Stored configs: it can either remove all data from an host in your
+       storedconfig database, or with --unexport turn every exported resource
+       supporting ensure to absent so that any other host checking out their
+       config can remove those exported configurations.
+
+      This will unexport exported resources of a
+      host, so that consumers of these resources can remove the exported
+      resources and we will safely remove the node from our
+      infrastructure.
+    EOT
+
     when_invoked do |*args|
       nodes = args[0..-2]
       options = args.last
@@ -39,82 +39,78 @@ EOT
       else
         Puppet::SSL::Host.ca_location = :none
       end
-  
+
       Puppet::Node::Facts.indirection.terminus_class = :yaml
       Puppet::Node::Facts.indirection.cache_class = :yaml
       Puppet::Node.indirection.terminus_class = :yaml
       Puppet::Node.indirection.cache_class = :yaml
 
-      begin
-        nodes.each do |node|
-          node = node.downcase
-          clean_cert(node)
-          clean_cached_facts(node)
-          clean_cached_node(node)
-          clean_reports(node)
-          clean_storeconfigs(node,options[:unexport])
-        end
-      rescue => detail
-        puts detail.backtrace if Puppet[:trace]
-        puts detail.to_s
-      end
+      nodes.each { |node| cleanup(node.downcase, options[:unexport]) }
     end
   end
-  
+
+  def cleanup(node, unexport)
+    clean_cert(node)
+    clean_cached_facts(node)
+    clean_cached_node(node)
+    clean_reports(node)
+    clean_storeconfigs(node, unexport)
+  end
+
   # clean signed cert for +host+
   def clean_cert(node)
-    if Puppet::SSL::Host.ca_location == :local
-      ca.apply(:revoke, :to => [node])
-      ca.apply(:destroy, :to => [node])
-      Puppet.info "%s certificates removed from ca" % node
+    if Puppet::SSL::CertificateAuthority.ca?
+      Puppet::Face[:ca, :current].revoke(node)
+      Puppet::Face[:ca, :current].destroy(node)
+      Puppet.info "#{node} certificates removed from ca"
     else
-      Puppet.info "Not managing %s certs as this host is not a CA" % node
+      Puppet.info "Not managing #{node} certs as this host is not a CA"
     end
   end
 
   # clean facts for +host+
   def clean_cached_facts(node)
     Puppet::Node::Facts.indirection.destroy(node)
-    Puppet.info "%s's facts removed" % node
+    Puppet.info "#{node}'s facts removed"
   end
 
   # clean cached node +host+
   def clean_cached_node(node)
     Puppet::Node.indirection.destroy(node)
-    Puppet.info "%s's cached node removed" % node
+    Puppet.info "#{node}'s cached node removed"
   end
 
   # clean node reports for +host+
   def clean_reports(node)
     Puppet::Transaction::Report.indirection.destroy(node)
-    Puppet.info "%s's reports removed" % node
+    Puppet.info "#{node}'s reports removed"
   end
 
   # clean storeconfig for +node+
-  def clean_storeconfigs(node,do_unexport=false)
+  def clean_storeconfigs(node, do_unexport=false)
     return unless Puppet[:storeconfigs] && Puppet.features.rails?
     require 'puppet/rails'
     Puppet::Rails.connect
     unless rails_node = Puppet::Rails::Host.find_by_name(node)
-      Puppet.notice "No entries found for %s in storedconfigs." % node
+      Puppet.notice "No entries found for #{node} in storedconfigs."
       return
     end
 
     if do_unexport
       unexport(rails_node)
-      Puppet.notice "Force %s's exported resources to absent" % node
+      Puppet.notice "Force #{node}'s exported resources to absent"
       Puppet.warning "Please wait until all other hosts have checked out their configuration before finishing the cleanup with:"
       Puppet.warning "$ puppet node clean #{node}"
     else
       rails_node.destroy
-      Puppet.notice "%s storeconfigs removed" % node
+      Puppet.notice "#{node} storeconfigs removed"
     end
   end
 
   def unexport(node)
     # fetch all exported resource
     query = {:include => {:param_values => :param_name}}
-    query[:conditions] = ["exported=? AND host_id=?", true, node.id]
+    query[:conditions] = [ "exported=? AND host_id=?", true, node.id ]
     Puppet::Rails::Resource.find(:all, query).each do |resource|
       if type_is_ensurable(resource)
         line = 0
@@ -122,7 +118,7 @@ EOT
 
         if ensure_param = resource.param_values.find(
           :first,
-          :conditions => [ 'param_name_id = ?', param_name.id]
+          :conditions => [ 'param_name_id = ?', param_name.id ]
         )
           line = ensure_param.line.to_i
           Puppet::Rails::ParamValue.delete(ensure_param.id);
@@ -134,21 +130,22 @@ EOT
           :line => line,
           :param_name => param_name
         )
-        Puppet.info("%s has been marked as \"absent\"" % resource.name)
+        Puppet.info("#{resource.name} has been marked as \"absent\"")
       end
     end
   end
 
-  def ca
-    @ca ||= Puppet::SSL::CertificateAuthority.instance
-  end
-
   def environment
-    @environemnt ||= Puppet::Node::Environment.new
+    @environment ||= Puppet::Node::Environment.new
   end
 
   def type_is_ensurable(resource)
-      (type=Puppet::Type.type(resource.restype)) && type.validattr?(:ensure) || \
-        (type = environment.known_resource_types.find_definition('',resource.restype)) && type.arguments.keys.include?('ensure')
+    if (type = Puppet::Type.type(resource.restype)) && type.validattr?(:ensure)
+      return true
+    else
+      type = environment.known_resource_types.find_definition('', resource.restype)
+      return true if type && type.arguments.keys.include?('ensure')
+    end
+    return false
   end
-end
\ No newline at end of file
+end