Merge pull request #357 from smarterclayton/better_app_create_flow

Run domain setup and config setup during app create

lib/rhc/auth/token.rb

@@ -16,8 +16,10 @@ def initialize(opt, auth=nil, store=nil)
 
     def to_request(request)
       if token
+        debug "Using token authentication"
         (request[:headers] ||= {})['authorization'] = "Bearer #{token}"
       elsif auth and (!@allows_tokens or @can_get_token == false)
+        debug "Bypassing token auth"
         auth.to_request(request)
       end
       request
@@ -56,6 +58,7 @@ def token_rejected(response, client)
           if has_token
             raise RHC::Rest::TokenExpiredOrInvalid, "Your authorization token is expired or invalid."
           end
+          debug "Cannot authenticate via token or password, exiting"
           return false
         end
 
@@ -78,6 +81,7 @@ def token_rejected(response, client)
 
         return auth.retry_auth?(response, client) unless @can_get_token
 
+        debug "Creating a new authorization token"
         if auth_token = client.new_session(:auth => auth)
           @fetch_once = true
           save(auth_token.token)

lib/rhc/command_runner.rb

@@ -101,7 +101,7 @@ def run!
     end
 
     def provided_arguments
-      @args[0, @args.find_index { |arg| arg.start_with?('-') } || @args.length]
+      @args[0, @args.find_index { |arg| arg != '--' and arg.start_with?('-') } || @args.length]
     end
 
     def global_option(*args, &block)

lib/rhc/commands.rb

@@ -34,7 +34,11 @@ def parse_options_and_call_procs *args
         opts
       end
 
-      remaining = opts.parse! args
+      # Separate option lists with '--'
+      remaining = args.split('--').map{ |a| opts.parse!(a) }.inject([]) do |arr, sub| 
+        arr << '--' unless arr.empty?
+        arr.concat(sub)
+      end
 
       _, config_path = proxy_options.find{ |arg| arg[0] == :config }
       clean, _ = proxy_options.find{ |arg| arg[0] == :clean }
@@ -266,32 +270,42 @@ def self.fill_arguments(cmd, options, args_metadata, options_metadata, args)
           end
           raise ArgumentError.new("Missing required option '#{arg}'.") if option_meta[:required] && options[arg].nil?
         end
-        # process args
-        arg_slots = [].fill(nil, 0, args_metadata.length)
-        fill_args = args.reverse
-        args_metadata.each_with_index do |arg_meta, i|
-          # check options
-          option = arg_meta[:option_symbol]
-          context_helper = arg_meta[:context_helper]
+
+        available = args.dup
+        slots = Array.new(args_metadata.length)
+        args_metadata.each_with_index do |arg, i|
+          option = arg[:option_symbol]
+          context_helper = arg[:context_helper]
 
           value = options.__hash__[option] if option
-          value = fill_args.pop if value.nil?
           value = cmd.send(context_helper) if value.nil? and context_helper
 
-          if arg_meta[:arg_type] == :list
-            fill_args.push(value) unless value.nil?
-            value = fill_args.reverse
-            fill_args = []
-          elsif value.nil?
-            raise ArgumentError.new("Missing required argument '#{arg_meta[:name]}'.") if fill_args.empty?
+          if value.nil?
+            value = 
+              if arg[:arg_type] == :list
+                all = []
+                while available.first && available.first != '--'
+                  all << available.shift
+                end
+                available.shift if available.first == '--'
+                all
+              else
+                available.shift
+              end
+          end
+
+          if value.nil?
+            raise ArgumentError, "Missing required argument '#{arg[:name]}'." unless arg[:optional]
+            break if available.empty?
+          else
+            slots[i] = value
+            options.__hash__[option] = value if option
           end
-          arg_slots[i] = value
-          options.__hash__[option] = value if option
         end
 
-        raise ArgumentError.new("Too many arguments passed in: #{fill_args.reverse.join(" ")}") unless fill_args.empty?
+        raise ArgumentError, "Too many arguments passed in: #{available.reverse.join(" ")}" unless available.empty?
 
-        arg_slots
+        slots
       end
 
       def self.commands

lib/rhc/commands/app.rb

@@ -9,6 +9,7 @@ class App < Base
     description "Creates and controls an OpenShift application.  To see the list of all applications use the rhc domain show command.  Note that delete is not reversible and will stop your application and then remove the application and repo from the remote server. No local changes are made."
     syntax "<action>"
     default_action :help
+    suppress_wizard
 
     summary "Create an application"
     description <<-DESC
@@ -38,19 +39,24 @@ class App < Base
 
       DESC
     syntax "<name> <cartridge> [-n namespace]"
-    option ["-n", "--namespace NAME"], "Namespace for the application", :context => :namespace_context
-    option ["-g", "--gear-size size"], "Gear size controls how much memory and CPU your cartridges can use."
+    option ["-n", "--namespace NAME"], "Namespace for the application"
+    option ["-g", "--gear-size SIZE"], "Gear size controls how much memory and CPU your cartridges can use."
     option ["-s", "--scaling"], "Enable scaling for the web cartridge."
-    option ["-r", "--repo dir"], "Path to the Git repository (defaults to ./$app_name)"
+    option ["-r", "--repo DIR"], "Path to the Git repository (defaults to ./$app_name)"
     option ["--from-code URL"], "URL to a Git repository that will become the initial contents of the application"
     option ["--[no-]git"], "Skip creating the local Git repository."
     option ["--nogit"], "DEPRECATED: Skip creating the local Git repository.", :deprecated => {:key => :git, :value => false}
     option ["--[no-]dns"], "Skip waiting for the application DNS name to resolve. Must be used in combination with --no-git"
-    option ["--enable-jenkins [server_name]"], "Enable Jenkins builds for this application (will create a Jenkins application if not already available). The default name will be 'jenkins' if not specified."
-    argument :name, "Name for your application", ["-a", "--app name"]
-    argument :cartridges, "The web framework this application should use", ["-t", "--type cartridge"], :arg_type => :list
+    option ['--no-keys'], "Skip checking SSH keys during app creation", :hide => true
+    option ["--enable-jenkins [NAME]"], "Enable Jenkins builds for this application (will create a Jenkins application if not already available). The default name will be 'jenkins' if not specified."
+    argument :name, "Name for your application", ["-a", "--app NAME"], :optional => true
+    argument :cartridges, "The web framework this application should use", ["-t", "--type CARTRIDGE"], :optional => true, :arg_type => :list
     #argument :additional_cartridges, "A list of other cartridges such as databases you wish to add. Cartridges can also be added later using 'rhc cartridge add'", [], :arg_type => :list
     def create(name, cartridges)
+      check_config!
+
+      check_name!(name)
+
       cartridges = check_cartridges(cartridges, &require_one_web_cart)
 
       options.default \
@@ -59,8 +65,7 @@ def create(name, cartridges)
 
       raise ArgumentError, "You have named both your main application and your Jenkins application '#{name}'. In order to continue you'll need to specify a different name with --enable-jenkins or choose a different application name." if jenkins_app_name == name && enable_jenkins?
 
-      raise RHC::Rest::DomainNotFoundException.new("No domains found. Please create a domain with 'rhc domain create <namespace>' before creating applications.") if rest_client.domains.empty?
-      rest_domain = rest_client.find_domain(options.namespace)
+      rest_domain = check_domain!
       rest_app = nil
 
       cart_names = cartridges.collect do |c|
@@ -121,6 +126,9 @@ def create(name, cartridges)
         end if build_app_exists
       end
 
+      debug "Checking SSH keys through the wizard"
+      check_sshkeys! unless options.no_keys
+
       if options.dns
         paragraph do
           say "Waiting for your DNS name to be available ... "
@@ -139,9 +147,6 @@ def create(name, cartridges)
 
         if options.git
           paragraph do
-            debug "Checking SSH keys through the wizard"
-            check_sshkeys! unless options.noprompt
-
             say "Downloading the application Git repository ..."
             paragraph do
               begin
@@ -335,9 +340,40 @@ def require_one_web_cart
       end
 
       def check_sshkeys!
+        return unless interactive?
         RHC::SSHWizard.new(rest_client, config, options).run
       end
 
+      def check_name!(name)
+        return unless name.blank?
+
+        paragraph{ say "When creating an application, you must provide a name and a cartridge from the list below:" }
+        paragraph{ list_cartridges(standalone_cartridges) }
+
+        raise ArgumentError, "Please specify the name of the application and the web cartridge to install"
+      end
+
+      def check_config!
+        return if not interactive? or (!options.clean && config.has_local_config?) or (options.server && (options.rhlogin || options.token))
+        RHC::EmbeddedWizard.new(config, options).run
+      end
+
+      def check_domain!
+        if options.namespace
+          rest_client.find_domain(options.namespace)
+        else
+          if rest_client.domains.empty?
+            raise RHC::Rest::DomainNotFoundException, "No domains found. Please create a domain with 'rhc domain create <namespace>' before creating applications." unless interactive?
+            RHC::DomainWizard.new(config, options, rest_client).run
+          end
+          domain = rest_client.domains.first
+          raise RHC::Rest::DomainNotFoundException, "No domains found. Please create a domain with 'rhc domain create <namespace>' before creating applications." unless domain
+          options.namespace = domain.id
+          domain
+        end
+      end
+
+
       def gear_groups_for_app(app_name)
         rest_client.find_application_gear_groups(options.namespace, app_name)
       end

lib/rhc/commands/base.rb

@@ -32,6 +32,7 @@ def rest_client(opts={})
       @rest_client ||= begin
           auth = RHC::Auth::Basic.new(options)
           auth = RHC::Auth::Token.new(options, auth, token_store) if (options.use_authorization_tokens || options.token) && !(options.rhlogin && options.password)
+          debug "Authenticating with #{auth.class}"
           client_from_options(:auth => auth)
         end
     end
@@ -129,17 +130,16 @@ def self.option(switches, description, options={})
                           }
     end
 
-    def self.argument(name, description, switches, options={})
+    def self.argument(name, description, switches=[], options={})
       arg_type = options[:arg_type]
-      raise ArgumentError("Only the last argument descriptor for an action can be a list") if arg_type == :list and list_argument_defined?
-      list_argument_defined true if arg_type == :list
 
       option_symbol = Commander::Runner.switch_to_sym(switches.last)
       args_metadata << {:name => name,
                         :description => description,
                         :switches => switches,
                         :context_helper => options[:context],
                         :option_symbol => option_symbol,
+                        :optional => options[:optional],
                         :arg_type => arg_type}
     end
 
@@ -149,12 +149,6 @@ def self.default_action(action)
     end
 
     private
-      def self.list_argument_defined(bool)
-        options[:list_argument_defined] = bool
-      end
-      def self.list_argument_defined?
-        options[:list_argument_defined]
-      end
       def self.options_metadata
         options[:options] ||= []
       end

lib/rhc/commands/domain.rb

@@ -89,15 +89,16 @@ def status
     def delete(namespace)
       domain = rest_client.find_domain namespace
 
-      say "Deleting domain '#{namespace}'"
+      say "Deleting domain '#{namespace}' ... "
 
       begin
         domain.destroy
       rescue RHC::Rest::ClientErrorException #FIXME: I am insufficiently specific
         raise RHC::Exception.new("Domain contains applications. Delete applications first.", 128)
       end
 
-      results { say "Success!" }
+      success "deleted"
+
       0
     end
   end

lib/rhc/commands/sshkey.rb

@@ -69,10 +69,12 @@ def add(name, key)
     summary 'Remove SSH key from your account'
     syntax '<name>'
     alias_action :delete
-    argument :name, 'SSH key to remove', []
+    argument :name, 'Name of SSH key to remove'
     def remove(name)
+      say "Removing the key '#{name} ... "
       rest_client.delete_key(name)
-      results { say "SSH key '#{name}' has been removed" }
+
+      success "removed"
 
       0
     end

lib/rhc/core_ext.rb

@@ -23,6 +23,23 @@ def to_json(options=nil)
   end
 end
 
+class Array
+  # From rails
+  def split(value = nil)
+    using_block = block_given?
+
+    inject([[]]) do |results, element|
+      if (using_block && yield(element)) || (value == element)
+        results << []
+      else
+        results.last << element
+      end
+
+      results
+    end
+  end  
+end
+
 class File
   def chunk(chunk_size=1024)
     yield read(chunk_size) until eof?

lib/rhc/helpers.rb

@@ -180,6 +180,10 @@ def certificate_file(file)
     # Output helpers
     #
 
+    def interactive?
+      $stdout.tty? and not options.noprompt
+    end
+
     def debug(msg)
       $stderr.puts "DEBUG: #{msg}" if debug?
     end

lib/rhc/rest/mock.rb

@@ -120,7 +120,7 @@ def stub_add_authorization(params)
         to_return(new_authorization(params))
     end
     def stub_no_keys
-      stub_api_request(:get, 'broker/rest/user/keys', mock_user_auth).to_return(no_keys)
+      stub_api_request(:get, 'broker/rest/user/keys', mock_user_auth).to_return(empty_keys)
     end
     def stub_mock_ssh_keys(name='test')
       stub_api_request(:get, 'broker/rest/user/keys', mock_user_auth).
@@ -159,7 +159,7 @@ def stub_one_key(name)
         })
     end
     def stub_no_domains
-      stub_api_request(:get, 'broker/rest/domains', mock_user_auth).to_return(no_domains)
+      stub_api_request(:get, 'broker/rest/domains', mock_user_auth).to_return(empty_domains)
     end
     def stub_one_domain(name)
       stub_api_request(:get, 'broker/rest/domains', mock_user_auth).
@@ -221,10 +221,10 @@ def test_and_raise
       EOM
     end
 
-    def no_keys
+    def empty_keys
       empty_response_list('keys')
     end
-    def no_domains
+    def empty_domains
       empty_response_list('domains')
     end