Skip to content
Browse files

Bogus servers should not be killable (fixes #250)

* style guide corrections ('&&', not 'and'; don't use single-letter variable names)
* also fixed a thing where security groups enumeration would die if a bogus server existed that's bit me before
* reviewed other knife cluster commands and made them not tolerate bogosity either
  • Loading branch information...
1 parent d9a1b6b commit 5ae6b3db351aa6fc85c261e0471b91525d98d822 Philip (flip) Kromer committed
View
1 lib/chef/knife/cluster_bootstrap.rb
@@ -62,7 +62,6 @@ def perform_execution(target)
ensure_common_environment(target)
# Execute across all servers in parallel
Ironfan.parallel(target.values) {|computer| run_bootstrap(computer)}
-# threads = target.servers.map{ |server| Thread.new(server) { |svr| run_bootstrap(svr, svr.public_hostname) } }
end
def confirm_execution(target)
View
2 lib/chef/knife/cluster_start.rb
@@ -24,7 +24,7 @@ class ClusterStart < Ironfan::Script
import_banner_and_options(Ironfan::Script)
def relevant?(server)
- server.stopped?
+ (not bogus?) && stopped?
end
def perform_execution(target)
View
2 lib/chef/knife/cluster_stop.rb
@@ -24,7 +24,7 @@ class ClusterStop < Ironfan::Script
import_banner_and_options(Ironfan::Script)
def relevant?(server)
- server.running?
+ (not bogus?) && server.running?
end
def perform_execution(target)
View
11 lib/chef/knife/ironfan_knife_common.rb
@@ -84,19 +84,18 @@ def confirm_execution(*args)
#
def get_relevant_slice( *predicate )
full_target = get_slice( *predicate )
- display(full_target) do |m|
- rel = relevant?(m)
- { :relevant? => (rel ? "[blue]#{rel}[reset]" : '-' ) }
+ display(full_target) do |mach|
+ rel = relevant?(mach)
+ { :relevant? => (rel ? "[green]#{rel}[reset]" : '-' ) }
end
- full_target.select{|m| relevant?(m) }
+ full_target.select{|mach| relevant?(mach) }
end
# passes target to Broker::Conductor#display, will show headings in server slice
# tables based on the --verbose flag
def display(target, display_style=nil, &block)
display_style ||= (config[:verbosity] == 0 ? :default : :expanded)
-# target.display(ui, display_style, &block)
- broker.display(target, display_style)
+ broker.display(target, display_style, &block)
end
#
View
8 lib/ironfan/broker.rb
@@ -31,11 +31,15 @@ def discover!(cluster)
end
def display(computers,style)
- defined_data = computers.map {|m| m.to_display(style) }
+ defined_data = computers.map do |mach|
+ hsh = mach.to_display(style)
+ hsh.merge!(yield(mach)) if block_given?
+ hsh
+ end
if defined_data.empty?
ui.info "Nothing to report"
else
- headings = defined_data.map{|r| r.keys}.flatten.uniq
+ headings = defined_data.map{|hsh| hsh.keys }.flatten.uniq
Formatador.display_compact_table(defined_data, headings.to_a)
end
end
View
72 lib/ironfan/broker/computer.rb
@@ -17,11 +17,11 @@ def initialize(*args)
super
providers[:chef] ||= Ironfan::Provider::ChefServer
return unless server
- providers[:iaas] = server.selected_cloud.provider
- volumes = server.volumes.values
- volumes += server.implied_volumes
- volumes.each { |v| self.drive v.name, :volume => v }
- rescue StandardError => err ; err.polish("#{self.class} on #{args}'") rescue nil ; raise
+ providers[:iaas] = server.selected_cloud.provider
+ volumes = server.volumes.values
+ volumes += server.implied_volumes
+ volumes.each{|vol| self.drive vol.name, :volume => vol }
+ rescue StandardError => err ; err.polish("#{self.class} on '#{args.inspect}'") rescue nil ; raise
end
def name
@@ -38,11 +38,11 @@ def correlate
res.expected_ids(self).each do |id|
next unless res.recall? id
- recalled = res.recall id
- recalled.users << self
+ recalled = res.recall id
+ recalled.users << self
- target = res.resource_type.to_s
- target += "__#{id}" if res.multiple?
+ target = res.resource_type.to_s
+ target += "__#{id}" if res.multiple?
self[target.to_sym] = recalled
recalled.on_correlate(self)
@@ -130,14 +130,6 @@ def chef_client_script_content
@chef_client_script_content = Ironfan.safely{ File.read(script_filename) }
end
-# def ensure_resource(type)
-# if type.multiple?
-# existing = resources[type.resource_id]
-# else
-#
-# end
-# end
-
#
# Display
#
@@ -200,7 +192,7 @@ def machine=(value)
self[:machine] = value
end
def dns_name ; machine? ? machine.dns_name : nil ; end
- def bootstrap_distro ; (server && server.selected_cloud) ? server.selected_cloud.bootstrap_distro : nil ; end
+ def bootstrap_distro ; (server && server.selected_cloud) ? server.selected_cloud.bootstrap_distro : nil ; end
def keypair
resources[:keypair]
@@ -225,16 +217,16 @@ def client?
not client.nil?
end
def created?
- machine? and machine.created?
+ machine? && machine.created?
end
def machine?
not machine.nil?
end
def killable?
- not permanent? and (node? || client? || created?)
+ (not bogus?) && (not permanent?) && (node? || client? || created?)
end
def launchable?
- not bogus? and not created?
+ (not bogus?) && (not created?)
end
def node?
not node.nil?
@@ -245,13 +237,13 @@ def permanent?
[true, :true, 'true'].include? server.selected_cloud.permanent
end
def running?
- machine? and machine.running?
+ machine? && machine.running?
end
def server?
not server.nil?
end
def stopped?
- machine? and machine.stopped?
+ machine? && machine.stopped?
end
# @return [Boolean] true if machine is likely to be reachable by ssh
@@ -295,15 +287,15 @@ def correlate
def validate
computers = self
- values.each{|c| c.validate }
+ values.each{|c| c.validate }
values.map {|c| c.providers.values}.flatten.uniq.each {|p| p.validate computers }
end
def group_action(verb)
computers = self
- provider_keys = values.map {|c| c.chosen_providers({ :providers => :iaas })}.flatten.uniq
- providers = provider_keys.map { |pk| values.map { |c| c.providers[pk] } }.flatten.compact.uniq
- providers.each { |p| p.send(verb, computers) }
+ provider_keys = values.map{|c| c.chosen_providers({ :providers => :iaas })}.flatten.uniq
+ providers = provider_keys.map{|pk| values.map{|c| c.providers[pk] } }.flatten.compact.uniq
+ providers.each{|p| p.send(verb, computers) }
end
def prepare
@@ -318,19 +310,19 @@ def aggregate
# Manipulation
#
def kill(options={})
- Ironfan.parallel(values) {|c| c.kill(options) }
+ Ironfan.parallel(values){|cc| cc.kill(options) }
end
def launch
- Ironfan.parallel(values) {|c| c.launch }
+ Ironfan.parallel(values){|cc| cc.launch }
end
def save(options={})
- Ironfan.parallel(values) {|c| c.save(options) }
+ Ironfan.parallel(values){|cc| cc.save(options) }
end
def start
- Ironfan.parallel(values) {|c| c.start }
+ Ironfan.parallel(values){|cc| cc.start }
end
def stop
- Ironfan.parallel(values) {|c| c.stop }
+ Ironfan.parallel(values){|cc| cc.stop }
end
def environments
@@ -351,7 +343,7 @@ def create_expected!
# Return the selection inside another Computers collection
def select(&block)
result = empty_copy
- values.select(&block).each{|m| result << m}
+ values.select(&block).each{|mach| result << mach}
result
end
@@ -363,16 +355,14 @@ def empty_copy
# Find all selected computers, as well as any bogus computers from discovery
def slice(facet_name=nil, slice_indexes=nil)
- return self if (facet_name.nil? and slice_indexes.nil?)
- result = empty_copy
+ return self if (facet_name.nil? && slice_indexes.nil?)
slice_array = build_slice_array(slice_indexes)
- each do |m|
- result << m if (m.bogus? or ( # bogus computer or
- ( m.server.facet_name == facet_name ) and # facet match and
- ( slice_array.include? m.server.index or # index match or
- slice_indexes.nil? ) ) ) # no indexes specified
+ select do |mach|
+ mach.bogus? or (
+ # facet match, and index match (or no indexes specified)
+ (mach.server.facet_name == facet_name) &&
+ (slice_array.include?(mach.server.index) || slice_indexes.nil?))
end
- result
end
def build_slice_array(slice_indexes)
View
3 lib/ironfan/provider/ec2/security_group.rb
@@ -19,6 +19,7 @@ def self.shared?() true; end
def self.multiple?() true; end
def self.resource_type() :security_group; end
def self.expected_ids(computer)
+ return unless computer.server
ec2 = computer.server.cloud(:ec2)
ec2.security_groups.keys.map { |name| group_name_with_vpc(name,ec2.vpc) }.uniq
end
@@ -73,7 +74,7 @@ def self.prepare!(computers)
# First, deduce the list of all groups to which at least one instance belongs
# We'll use this later to decide whether to create groups, or authorize access,
# using a VPC security group or an EC2 security group.
- groups_that_should_exist = computers.map { |c| expected_ids(c) }.flatten.sort.uniq
+ groups_that_should_exist = computers.map{|comp| expected_ids(comp) }.flatten.compact.sort.uniq
groups_to_create << groups_that_should_exist
computers.select { |computer| Ec2.applicable computer }.each do |computer|
2 notes
@@ -1 +1 @@
-Subproject commit f607be7ab5801a0719746cd2b4d23f44b2cbddba
+Subproject commit c99cf8812dc46202ca92b9021418eed8ef676357

0 comments on commit 5ae6b3d

Please sign in to comment.
Something went wrong with that request. Please try again.