From f9de28236b726e37977123cea9b4f3a562bfdcdb Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Tue, 7 Apr 2015 10:26:55 -0700 Subject: [PATCH] Auto-expire Redis metadata key via Redis EXPIRE This commit also removed the unnecessary Vmpooler::Janitor lib --- lib/vmpooler.rb | 2 +- lib/vmpooler/janitor.rb | 36 ------------------ lib/vmpooler/pool_manager.rb | 9 +++-- spec/vmpooler/janitor_spec.rb | 71 ----------------------------------- vmpooler | 28 +++++++------- 5 files changed, 19 insertions(+), 127 deletions(-) delete mode 100644 lib/vmpooler/janitor.rb delete mode 100644 spec/vmpooler/janitor_spec.rb diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb index bb84d21..c7bf065 100644 --- a/lib/vmpooler.rb +++ b/lib/vmpooler.rb @@ -11,7 +11,7 @@ module Vmpooler require 'timeout' require 'yaml' - %w( api graphite janitor logger pool_manager vsphere_helper ).each do |lib| + %w( api graphite logger pool_manager vsphere_helper ).each do |lib| begin require "vmpooler/#{lib}" rescue LoadError diff --git a/lib/vmpooler/janitor.rb b/lib/vmpooler/janitor.rb deleted file mode 100644 index f949de8..0000000 --- a/lib/vmpooler/janitor.rb +++ /dev/null @@ -1,36 +0,0 @@ -module Vmpooler - class Janitor - def initialize(logger, redis, data_ttl) - # Load logger library - $logger = logger - - # Connect to Redis - $redis = redis - - # TTL - $data_ttl = data_ttl - end - - def execute! - loop do - find_stale_vms - - sleep(600) - end - end - - def find_stale_vms - $redis.keys('vmpooler__vm__*').each do |key| - data = $redis.hgetall(key) - - if data['destroy'] - lifetime = (Time.now - Time.parse(data['destroy'])) / 60 / 60 - - if lifetime > $data_ttl - $redis.del(key) - end - end - end - end - end -end diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 2fb71f9..19e1ad2 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1,10 +1,8 @@ module Vmpooler class PoolManager - def initialize(config, pools, logger, redis, graphite=nil) + def initialize(config, logger, redis, graphite=nil) $config = config - $pools = pools - # Load logger library $logger = logger @@ -257,6 +255,9 @@ module Vmpooler $redis.hdel('vmpooler__active__' + pool, vm) $redis.hset('vmpooler__vm__' + vm, 'destroy', Time.now) + # Auto-expire metadata key + $redis.expire('vmpooler__vm__' + vm, ($config[:redis]['data_ttl'].to_i * 60 * 60)) + host = $vsphere[pool].find_vm(vm) || $vsphere[pool].find_vm_heavy(vm)[vm] @@ -451,7 +452,7 @@ module Vmpooler $redis.set('vmpooler__tasks__clone', 0) loop do - $pools.each do |pool| + $config[:pools].each do |pool| if ! $threads[pool['name']] check_pool(pool) else diff --git a/spec/vmpooler/janitor_spec.rb b/spec/vmpooler/janitor_spec.rb deleted file mode 100644 index b57e1c2..0000000 --- a/spec/vmpooler/janitor_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -require 'spec_helper' - -describe 'Janitor' do - let(:logger) { double('logger') } - let(:redis) { double('redis') } - let(:data_ttl) { 3 } # number of hours to retain - let(:old_destroy) { '2015-01-30 11:24:30 -0700' } - let(:fut_destroy) { '3015-01-30 11:24:30 -0700' } - - subject { Vmpooler::Janitor.new(logger, redis, data_ttl) } - - before do - allow(redis).to receive(:hgetall).with('key1').and_return({'destroy' => old_destroy}) - allow(redis).to receive(:hgetall).with('key2').and_return({'destroy' => old_destroy}) - allow(redis).to receive(:hgetall).with('key3').and_return({'destroy' => Time.now.to_s}) - allow(redis).to receive(:hgetall).with('key4').and_return({'destroy' => Time.now.to_s}) - allow(redis).to receive(:hgetall).with('key5').and_return({'destroy' => fut_destroy}) - - allow(redis).to receive(:del) - end - - describe '#find_stale_vms' do - - context 'has stale vms' do - - it 'has one key' do - allow(redis).to receive(:keys) {['key1']} - - expect(redis).to receive(:del).with('key1').once - expect(redis).to receive(:hgetall).with('key1').once - expect(redis).not_to receive(:hgetall).with('key2') - - subject.find_stale_vms - end - - it 'has two keys' do - allow(redis).to receive(:keys) {(%w(key1 key2))} - - expect(redis).to receive(:hgetall).twice - expect(redis).to receive(:del).with('key1') - expect(redis).to receive(:del).with('key2') - - subject.find_stale_vms - end - - it 'has 5 keys and 2 stales' do - allow(redis).to receive(:keys) {(%w(key1 key2 key3 key4 key5))} - - - expect(redis).to receive(:hgetall).exactly(5).times - expect(redis).to receive(:del).with('key1') - expect(redis).to receive(:del).with('key2') - expect(redis).not_to receive(:del).with('key3') - expect(redis).not_to receive(:del).with('key4') - expect(redis).not_to receive(:del).with('key5') - - subject.find_stale_vms - end - end - - it 'does not have stale vms' do - allow(redis).to receive(:keys).and_return(['key1']) - allow(redis).to receive(:hgetall).with('key1') {{'destroy' => Time.now.to_s}} - allow(redis).to receive(:del).with('key1') - - expect(redis).not_to receive(:del).with('key1') - - subject.find_stale_vms - end - end -end \ No newline at end of file diff --git a/vmpooler b/vmpooler index 5054def..5d0dd51 100755 --- a/vmpooler +++ b/vmpooler @@ -7,25 +7,23 @@ require 'lib/vmpooler' config = Vmpooler.config redis_host = config[:redis]['server'] -redis_ttl = config[:redis]['data_ttl'] logger_file = config[:config]['logfile'] graphite = config[:graphite]['server'] ? config[:graphite]['server'] : nil api = Thread.new { - thr = Vmpooler::API.new - thr.helpers.configure(config, Vmpooler.new_redis(redis_host)) - thr.helpers.execute! - } -janitor = Thread.new { - Vmpooler::Janitor.new(Vmpooler.new_logger(logger_file), Vmpooler.new_redis(redis_host), redis_ttl).execute! - } + thr = Vmpooler::API.new + thr.helpers.configure(config, Vmpooler.new_redis(redis_host)) + thr.helpers.execute! +} + manager = Thread.new { - Vmpooler::PoolManager.new(config, - config[:pools], - Vmpooler.new_logger(logger_file), - Vmpooler.new_redis(redis_host), - Vmpooler.new_graphite(graphite)).execute! - } + Vmpooler::PoolManager.new( + config, + Vmpooler.new_logger(logger_file), + Vmpooler.new_redis(redis_host), + Vmpooler.new_graphite(graphite) + ).execute! +} -[api, janitor, manager].each { |t| t.join } +[api, manager].each { |t| t.join }