Feature #1886

Add node cleanup capability

Added by micah - over 4 years ago. Updated 14 days ago.

Status:ClosedStart date:01/23/2009
Priority:NormalDue date:
Assignee:-% Done:

80%

Category:newfeature
Target version:2.7.10
Affected Puppet version:0.25.0 Branch:https://github.com/puppetlabs/puppet/pull/92
Keywords:communitypatch

Description

There really isn’t a good mechanism to clean up when a node no longer exits. You can remove it from your node definition, but there is a lot of junk hanging around that should be cleaned up too. It would be really cool if the ability to wipe a node was built into puppetmaster so you could just simply do something like:

puppetmaster --clean node
puppetmaster --clean node.domain.name

and this would clean up everything that puppet can find on that node. The things that I can think of that need to get cleaned up are:

  1. Signed certificates ($vardir/ssl/ca/signed/node.domain.pem)
  2. Cached facts ($vardir/yaml/facts/node.domain.yaml)
  3. Cached node stuff ($vardir/yaml/node/node.domain.yaml)
  4. Reports ($vardir/reports/node.domain)
  5. Stored configs. I found a script on the wiki that takes care of this:
#!/usr/bin/env ruby

## Usage: sudo ./kill_node_in_storeconfigs_db.rb 

require 'puppet/rails'

Puppet[:config] = "/etc/puppet/puppet.conf"
Puppet.parse_config
pm_conf = Puppet.settings.instance_variable_get(:@values)[:puppetmasterd]

adapter = pm_conf[:dbadapter]
args = {:adapter => adapter, :log_level => pm_conf[:rails_loglevel]}

case adapter
  when "sqlite3":
    args[:dbfile] = pm_conf[:dblocation]
  when "mysql", "postgresql":
    args[:host]     = pm_conf[:dbserver] unless pm_conf[:dbserver].empty?
    args[:username] = pm_conf[:dbuser] unless pm_conf[:dbuser].empty?
    args[:password] = pm_conf[:dbpassword] unless pm_conf[:dbpassword].empty?
    args[:database] = pm_conf[:dbname]
  else
    raise ArgumentError, "Invalid db adapter %s" % adapter
end

ActiveRecord::Base.establish_connection(args)

if @host = Puppet::Rails::Host.find_by_name(ARGV[0].strip)
  print "Killing #{ARGV[0]}..." 
  $stdout.flush
  @host.destroy 
  puts "done."
else
  puts "Can't find host #{ARGV[0]}."
end


Sure, I can do all this manually, but this is a feature request because its a little annoying and you forget all the pieces.

Related issues

Related to Puppet - Bug #11330: puppet node clean does not remove records from inventory_... Accepted 12/11/2011
Duplicated by Puppet - Feature #4539: Puppet should have the capability to "clean" nodes like c... Duplicate 08/15/2010

History

#1 Updated by James Turnbull over 4 years ago

  • Status changed from Unreviewed to Accepted
  • Assignee set to Brice Figureau

#2 Updated by Udo Waechter over 4 years ago

micah wrote:

  1. Stored configs. I found a script on the wiki that takes care of this: It would be really great if exported resources will be unexported. I am sure that this will not work for most of the resources, but those that have a “ensure” parameter should be set to their opposite:

so:

@@file{“/bla/bla”: ensure => present, }

should become

@@file{“/bla/bla”: ensure => absent, }

I guess this increases the complexity of this ticket, but I think that it definetly should be there.

#3 Updated by Luke Kanies over 4 years ago

do wrote:

micah wrote:

  1. Stored configs. I found a script on the wiki that takes care of this: It would be really great if exported resources will be unexported. I am sure that this will not work for most of the resources, but those that have a “ensure” parameter should be set to their opposite:

so:

@@file{“/bla/bla”: ensure => present, }

should become

@@file{“/bla/bla”: ensure => absent, }

I guess this increases the complexity of this ticket, but I think that it definetly should be there.

Yeah, this is a pretty significant change; while I can see why you want this, I’d say that most cases that use exported resources should also use resource purging.

#4 Updated by Udo Waechter over 4 years ago

luke wrote:

do wrote:

micah wrote:

  1. Stored configs. I found a script on the wiki that takes care of this: It would be really great if exported resources will be unexported. I am sure that this will not work for most of the resources, but those that have a “ensure” parameter should be set to their opposite:

so:

@@file{“/bla/bla”: ensure => present, }

should become

@@file{“/bla/bla”: ensure => absent, }

I guess this increases the complexity of this ticket, but I think that it definetly should be there.

Yeah, this is a pretty significant change; while I can see why you want this, I’d say that most cases that use exported resources should also use resource purging.

Yes, we have that in our modules (we have anticlasses for all classes that we do define). Unfortunately the day comes when a node dies without having the last chance to be put into such a class and run puppet for the last time.

We have this with about 4 hosts now, and their exported resources are spread over 4 or 5 other hosts. Now, I need to delete the stored configs, afterwards I need to log in to all of those machines and delete the resources by hand. Long tme since I did such things, thanks to puppet :)

#5 Updated by micah - about 4 years ago

  • Affected Puppet version changed from 0.24.7 to 0.25.0

#6 Updated by R.I. Pienaar about 4 years ago

It would be great – at least for stored procs – if we could just have a setting in puppet.conf that defines a max age for a node in the exported resources database.

So if I feel in my environmnet it is ok if I dont hear from a node for 2 days that probably its been retired so I am happy to purge it then the exported resource database should just get cleared.

I wouldnt think a default policy of always clear older than n days would be acceptable, by default no auto aging should happen. But as an optional setting I think it would simplify things a lot and make the whole system more self managing.

In a larger context if a node{} had some property that you could set analogous to ensure => absent the master should then clean certs, nuke exported resources and everything else mentioned.

#7 Updated by James Turnbull about 4 years ago

  • Target version set to 4

#8 Updated by Ohad Levy almost 4 years ago

volcane volcane wrote:

It would be great – at least for stored procs – if we could just have a setting in puppet.conf that defines a max age for a node in the exported resources database.

So if I feel in my environmnet it is ok if I dont hear from a node for 2 days that probably its been retired so I am happy to purge it then the exported resource database should just get cleared.

I wouldnt think a default policy of always clear older than n days would be acceptable, by default no auto aging should happen. But as an optional setting I think it would simplify things a lot and make the whole system more self managing.

In a larger context if a node{} had some property that you could set analogous to ensure => absent the master should then clean certs, nuke exported resources and everything else mentioned.

I would add a disable functionality, this will not erase the data, but will not include it either.

#9 Updated by Peter Meier over 2 years ago

  • Branch set to https://github.com/duritong/puppet/tree/ticket/2.6.x/1886

#10 Updated by Matt Robinson over 2 years ago

  • Keywords set to communitypatch

#11 Updated by Paul Berry over 2 years ago

  • Status changed from Accepted to Code Insufficient
  • Assignee changed from Brice Figureau to Peter Meier

See mailing list discussion.

#12 Updated by Dis Connect over 2 years ago

Paul Berry wrote:

See mailing list discussion.

er, what discussion? Got a link? (FYI the file above doesn’t work anymore.)

#13 Updated by Daniel Pittman over 2 years ago

  • Status changed from Code Insufficient to Needs Decision
  • Assignee changed from Peter Meier to Daniel Pittman

The discussion thread is here: http://groups.google.com/group/puppet-dev/browse_thread/thread/317e930bddf35f71/77baa3d53ed2ce30?lnk=gst&q=%231886#77baa3d53ed2ce30

At this point the functionality has been folded, at least in prototype state, into lak’s tree in the interfaces tools, and the bulk of issues blocking this entering as a new application are resolved.

We need to identify which of these paths is the long term strategy for this, and which branch it targets; James mentions 2.6, R&D feel that this is a very large change compared to our general policies on maintenance branches, but we recognise the need for a similar tool.

Sorry for the long delay in getting this up to date, and we would welcome any pointers to more current versions of the code if anyone wants to contribute them; I think we have the latest, but would be happy to be more certain. :)

#14 Updated by Nigel Kersten over 2 years ago

  • Target version changed from 4 to 2.7.x

We’re targeting this at Statler.

Have we reached agreement on what “this” actually means however?

#15 Updated by Daniel Pittman over 2 years ago

No, not at this point in time. We need to make a couple of decisions:

  1. Are we shipping “interfaces”, so that it would make sense to have ‘puppet node clean’ as an action?
  2. Do we want to ship an interim solution in 2.6, where having a separate node cleaner might make sense?

I think the answers are yes, and no, meaning that “this” is “use the interfaces stuff to deliver this functionality.

#16 Updated by Nigel Kersten over 2 years ago

  • Status changed from Needs Decision to Accepted
  • Priority changed from Low to Normal

Yes, I agree. We should retarget this discussion at the desired kinds of cleanup commands that people need.

#17 Updated by Peter Meier about 2 years ago

  1. Are we shipping “interfaces”, so that it would make sense to have ‘puppet node clean’ as an action?
  2. Do we want to ship an interim solution in 2.6, where having a separate node cleaner might make sense?

I think the answers are yes, and no, meaning that “this” is “use the interfaces stuff to deliver this functionality.

+1, I hoped that we could work @ the PuppetCamp on integrating it with the interfaces. I will ping the various people there…

#18 Updated by Nigel Kersten about 2 years ago

Reminder to those watching to chase people up at Puppet Camp.

#19 Updated by Peter Meier about 2 years ago

Nigel Kersten wrote:

Reminder to those watching to chase people up at Puppet Camp.

I had the idea to finish the implementation with the new “faces” feature in the hacking session.

#20 Updated by Peter Meier about 2 years ago

  • Branch changed from https://github.com/duritong/puppet/tree/ticket/2.6.x/1886 to https://github.com/duritong/puppet/tree/feature/2.7.x/1886

Patch for 2.7.x up @ https://github.com/duritong/puppet/tree/feature/2.7.x/1886

#21 Updated by Daniel Pittman about 2 years ago

Branch changed from https://github.com/duritong/puppet/tree/ticket/2.6.x/1886 to https://github.com/duritong/puppet/tree/feature/2.7.x/1886

Patch for 2.7.x up @ https://github.com/duritong/puppet/tree/feature/2.7.x/1886

I reviewed the code, and I am absolutely happy to see it merge in the current state.

#22 Updated by James Turnbull about 2 years ago

  • Status changed from Accepted to In Topic Branch Pending Review

#23 Updated by Pieter van de Bruggen almost 2 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release
  • Target version changed from 2.7.x to 2.7.3
  • Branch changed from https://github.com/duritong/puppet/tree/feature/2.7.x/1886 to 2.7.x

This was merged into 2.7.x as commit:ecca900.

#24 Updated by Pieter van de Bruggen almost 2 years ago

  • Status changed from Merged - Pending Release to Tests Insufficient
  • % Done changed from 0 to 80

So that this context isn’t lost:

The merged code caused failures in our CI environment (https://jenkins.puppetlabs.com/job/Puppet%20Specs%20(2.7.x)/61/) that we had difficulty reproducing; consequently, cleaning of storeconfigs has been temporarily backed out, pending better isolated testing.

#25 Updated by Peter Meier almost 2 years ago

The merged code caused failures in our CI environment (https://jenkins.puppetlabs.com/job/Puppet%20Specs%20(2.7.x)/61/) that we had difficulty reproducing; consequently, cleaning of storeconfigs has been temporarily backed out, pending better isolated testing.

So the one thing that pops up is that it fails on ruby >=1.8.7 and with these rubies it uses activerecord >= 3.0, which seems to not get properly mocked. rails 3.x (and so activerecord 3.x) requires ruby >= 1.8.7. This means that probably on the 1.8.5 installation there is an activerecord 2.3.x installed, which is also what I used to test things on 1.8.7, as I didn’t remember that rails 3 got officially supported by puppet for storedconfigs. Sounds like a reasonable thing to look at, not? So using rails >= 3 for puppet stored configs is offically supported?

#26 Updated by Michael Stahnke almost 2 years ago

  • Target version changed from 2.7.3 to 2.7.x

Moving to 2.7.x as it doesn’t look like this was fully fixed at 2.7.3 rc1 timeframe.

#27 Updated by Peter Meier almost 2 years ago

Michael Stahnke wrote:

Moving to 2.7.x as it doesn’t look like this was fully fixed at 2.7.3 rc1 timeframe.

Sent a new pull request.

#28 Updated by Peter Meier almost 2 years ago

  • Target version changed from 2.7.x to 2.7.4

setting the target version to 2.7.4 so it will hopefully be merged in the next release.

#29 Updated by Matthaus Owens almost 2 years ago

  • Status changed from Tests Insufficient to In Topic Branch Pending Review
  • Target version changed from 2.7.4 to 2.7.5

This patch requires review from our dev team. Targeting at 2.7.5 which should release in early October.

#30 Updated by Michael Stahnke over 1 year ago

  • Target version changed from 2.7.5 to 2.7.x

This does not look like it got reviewed before cutting 2.7.6rc1. Moving to 2.7.x.

#31 Updated by Peter Meier over 1 year ago

What’s the status of the pending review? Can we get that reviewed before we enter another release cycle? Thx!

#32 Updated by Josh Cooper over 1 year ago

  • Branch changed from 2.7.x to https://github.com/puppetlabs/puppet/pull/92

Set branch

#33 Updated by Rob Terhaar over 1 year ago

I recently found that node cleanup does not remove nodes from the inventory tables as well: #11330

#34 Updated by Daniel Pittman over 1 year ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release
  • Target version changed from 2.7.x to 2.7.10

The StoreConfig change has been merged, and should be running through the test suite. Unless something crops up there, this should be done.

#35 Updated by Michael Stahnke over 1 year ago

  • Status changed from Merged - Pending Release to Closed

released in 2.7.10rc1

#36 Updated by Daniel Pittman 14 days ago

  • Assignee deleted (Daniel Pittman)

Also available in: Atom PDF