Issue #8185 has been updated by Jeff McCune. Due date set to 07/15/2011 Status changed from Accepted to In Topic Branch Pending Merge Branch set to ticket/8157_better_feedback_for_terminate/8185_list_instances_action
# Topic Branch # I based the topic branch off of `ticket/7511_add_region_option/8157_better_feedback_for_terminate` because of the anticipation of it being merged. The topic branch for this ticket is: `ticket/8157_better_feedback_for_terminate/8185_list_instances_action` When merging into the integration branch, care might be needed to make sure only the relevant two patches are in the topic branch: <pre> >From 2b7533744242610a818e310d98ad8c4a042d94f8 Mon Sep 17 00:00:00 2001 From: Jeff McCune <[email protected]> Date: Sun, 10 Jul 2011 16:52:29 -0700 Subject: [PATCH/Module/cloudpack 1/2] (#8185) Add a central place to set option defaults For #8185 I'm adding a list action. This action does not require any options, but the Fog AWS call needs to have at least the platform option set to 'AWS' In addition, the region option is optional for Fog but should default to 'us-east-1' to prevent Puppet's behavior from changing if Fog's default region changes. This patch adds a new method, #merge_default_options which sets the default platform to AWS and default region to us-east-1. Any actions that send a message to #create_connection should run their options hash through the merge_default_options and re-assign. This is particularly important for the create, terminate and upcoming list actions. Signed-off-by: Jeff McCune <[email protected]> --- lib/puppet/cloudpack.rb | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/puppet/cloudpack.rb b/lib/puppet/cloudpack.rb index 9d98135..9196e92 100644 --- a/lib/puppet/cloudpack.rb +++ b/lib/puppet/cloudpack.rb @@ -5,6 +5,14 @@ require 'puppet/network/http_pool' module Puppet::CloudPack class << self + # Method to set AWS defaults in a central place. Lots of things need these + # defaults, so they all call merge_default_options() to ensure the keys are + # set. + def merge_default_options(options) + default_options = { :region => 'us-east-1', :platform => 'AWS' } + default_options.merge(options) + end + def add_region_option(action) action.option '--region=' do summary "The geographic region of the instance. Defaults to us-east-1." @@ -23,8 +31,8 @@ module Puppet::CloudPack # JJM FIXME We shouldn't have to set the defaults here, but we do because the first # required action may not have it's #before_action evaluated yet. As a result, # the default settings may not be evaluated. - options[:platform] ||= 'AWS' - options[:region] ||= 'us-east-1' + options = Puppet::CloudPack.merge_default_options(options) + regions_response = Puppet::CloudPack.create_connection(options).describe_regions region_names = regions_response.body["regionInfo"].collect { |r| r["regionName"] }.flatten unless region_names.include?(options[:region]) @@ -65,8 +73,7 @@ module Puppet::CloudPack before_action do |action, args, options| # We add these options because it's required in Fog but optional for Cloud Pack # It doesn't feel right to do this here, but I don't know another way yet. - options[:platform] ||= 'AWS' - options[:region] ||= 'us-east-1' + options = Puppet::CloudPack.merge_default_options(options) if Puppet::CloudPack.create_connection(options).images.get(options[:image]).nil? raise ArgumentError, "Unrecognized image name: #{options[:image]}" end @@ -102,7 +109,7 @@ module Puppet::CloudPack before_action do |action, args, options| # We add this option because it's required in Fog but optional for Cloud Pack # It doesn't feel right to do this here, but I don't know another way yet. - options[:platform] ||= 'AWS' + options = Puppet::CloudPack.merge_default_options(options) if Puppet::CloudPack.create_connection(options).key_pairs.get(options[:keypair]).nil? raise ArgumentError, "Unrecognized keypair name: #{options[:keypair]}" end @@ -118,11 +125,10 @@ module Puppet::CloudPack Multiple groups can be specified as a list using ':'. EOT before_action do |action, args, options| - # We add this option because it's required in Fog but optional for Cloud Pack - # It doesn't feel right to do this here, but I don't know another way yet. - options[:platform] ||= 'AWS' options[:group] = options[:group].split(File::PATH_SEPARATOR) unless options[:group].is_a? Array + options = Puppet::CloudPack.merge_default_options(options) + known = Puppet::CloudPack.create_connection(options).security_groups unknown = options[:group].select { |g| known.get(g).nil? } unless unknown.empty? @@ -279,6 +285,7 @@ module Puppet::CloudPack end def create(options) + options = merge_default_options(options) unless options.has_key? :_destroy_server_at_exit options[:_destroy_server_at_exit] = :create end @@ -427,8 +434,8 @@ module Puppet::CloudPack def terminate(server, options) # JJM This isn't ideal, it would be better to set the default in the # option handling block, but I'm not sure how to do this. - options[:platform] ||= 'AWS' - options[:region] ||= 'us-east-1' + options = merge_default_options(options) + print "Connecting to #{options[:platform]} ..." connection = create_connection(options) puts ' Done' -- 1.7.5.4 >From 94f7d8d48a9271b6622cd0740ebb5691f690ebae Mon Sep 17 00:00:00 2001 From: Jeff McCune <[email protected]> Date: Sun, 10 Jul 2011 16:57:11 -0700 Subject: [PATCH/Module/cloudpack 2/2] (#8185) Add list action This change adds a new action to cloud pack. This increases the number of actions that are EC2 specific. Previously, it was just create and terminate. The Faces API is responsible for rendering the output of this action. A simple array of DNS names, one for each running instance in the EC2 region is handed back to Faces. Sample output: $ puppet cloudnode list --region us-west-1 --render-as yaml --- [] $ puppet cloudnode list [null] $ puppet cloudnode list --render-as yaml puppet cloudnode list --render-as yaml --- - ec2-50-19-71-40.compute-1.amazonaws.com Spec tests have also been added validating the input options and output of the action. Signed-off-by: Jeff McCune <[email protected]> --- lib/puppet/cloudpack.rb | 14 +++++++ lib/puppet/face/cloudnode/list.rb | 17 +++++++++ spec/unit/puppet/cloudpack_spec.rb | 22 ++++++++++++ spec/unit/puppet/face/node/list_spec.rb | 57 +++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 0 deletions(-) create mode 100644 lib/puppet/face/cloudnode/list.rb create mode 100644 spec/unit/puppet/face/node/list_spec.rb diff --git a/lib/puppet/cloudpack.rb b/lib/puppet/cloudpack.rb index 9196e92..3f6c393 100644 --- a/lib/puppet/cloudpack.rb +++ b/lib/puppet/cloudpack.rb @@ -139,6 +139,11 @@ module Puppet::CloudPack end + def add_list_options(action) + add_platform_option(action) + add_region_option(action) + end + def add_init_options(action) add_install_options(action) add_classify_options(action) @@ -350,6 +355,15 @@ module Puppet::CloudPack return server.dns_name end + def list(options) + options = merge_default_options(options) + connection = create_connection(options) + servers = connection.servers + # Convert the Fog object into a simple array. + # And return the array to the Faces API for rendering + servers.collect { |i| i.dns_name } + end + def init(server, options) certname = install(server, options) options.delete(:_destroy_server_at_exit) diff --git a/lib/puppet/face/cloudnode/list.rb b/lib/puppet/face/cloudnode/list.rb new file mode 100644 index 0000000..731d109 --- /dev/null +++ b/lib/puppet/face/cloudnode/list.rb @@ -0,0 +1,17 @@ +require 'puppet/cloudpack' + +Puppet::Face.define :cloudnode, '0.0.1' do + action :list do + summary 'List node instances' + description <<-'EOT' + The list action obtains a list of instances from the cloud provider and + displays them on the console output. For EC2 instances, only the instances in + a specific region are provided. + EOT + Puppet::CloudPack.add_list_options(self) + when_invoked do |options| + Puppet::CloudPack.list(options) + end + end +end + diff --git a/spec/unit/puppet/cloudpack_spec.rb b/spec/unit/puppet/cloudpack_spec.rb index ac9c564..afbcebf 100644 --- a/spec/unit/puppet/cloudpack_spec.rb +++ b/spec/unit/puppet/cloudpack_spec.rb @@ -127,6 +127,28 @@ describe Puppet::CloudPack do end end end + + describe '#list' do + describe 'with valid arguments' do + before :all do + @result = subject.list(:platform => 'AWS') + end + it 'should not be empty' do + # We actually get back something like this from Fog: + # ["ec2-368-5-559-12.compute-1.amazonaws.com", "ec2-14-92-246-64.compute-1.amazonaws.com"] + @result.should_not be_empty + end + it "should look like a list of DNS names" do + @result.each do |hostname| + hostname.should match(/^([a-zA-Z0-9-]+)$|^([a-zA-Z0-9-]+\.)+[a-zA-Z0-9-]+$/) + end + end + it "should be a kind of Array" do + @result.should be_a_kind_of(Array) + end + end + end + end describe 'helper functions' do diff --git a/spec/unit/puppet/face/node/list_spec.rb b/spec/unit/puppet/face/node/list_spec.rb new file mode 100644 index 0000000..cf8d2d2 --- /dev/null +++ b/spec/unit/puppet/face/node/list_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' +require 'puppet/cloudpack' + +describe Puppet::Face[:cloudnode, :current] do + before :all do + data = Fog::AWS::Compute::Mock.data['us-east-1'][Fog.credentials[:aws_access_key_id]] + data[:images]['ami-12345'] = { 'imageId' => 'ami-12345' } + data[:key_pairs]['some_keypair'] = { 'keyName' => 'some_keypair' } + end + + before :each do + @options = { + :platform => 'AWS', + :region => 'us-east-1', + } + end + + describe 'option validation' do + describe '(platform)' do + it 'should not require a platform' do + @options.delete(:platform) + # JJM This is absolutely not ideal, but I cannot for the life of me + # figure out how to effectively deal with all of the create_connection + # method calls in the option validation code. + Puppet::CloudPack.stubs(:create_connection).with() do |options| + raise(Exception, "#{options[:platform] == 'AWS'}") + end + expect { subject.list(@options) }.to raise_error Exception, 'true' + end + + it 'should validate the platform' do + @options[:platform] = 'UnsupportedProvider' + expect { subject.list(@options) }.to raise_error ArgumentError, /one of/ + end + end + + describe '(region)' do + it "should not require a region name" do + @options.delete(:region) + # JJM This is absolutely not ideal, but I cannot for the life of me + # figure out how to effectively deal with all of the create_connection + # method calls in the option validation code. + Puppet::CloudPack.stubs(:create_connection).with() do |options| + raise(Exception, "region:#{options[:region]}") + end + expect { subject.list(@options) }.to raise_error Exception, 'region:us-east-1' + end + + it 'should validate the region' do + @options[:region] = 'mars-east-100' + expect { subject.list(@options) }.to raise_error ArgumentError, /one of/ + end + end + + end +end + -- 1.7.5.4 </pre> Email sent to tech for review. ---------------------------------------- Feature #8185: CloudPack should be able to list running instances https://projects.puppetlabs.com/issues/8185 Author: Jeff McCune Status: In Topic Branch Pending Merge Priority: Normal Assignee: Jeff McCune Category: Target version: Keywords: Branch: ticket/8157_better_feedback_for_terminate/8185_list_instances_action Roadmapped: No # Overview # It would be nice to have this listing as a JSON or similar parseable output, but default to a simple one-instance per line listing on STDOUT. The list should allow instance ID's or instance public host names to be displayed easily. Ideally this listing should be filterable -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
