Merge branch 'pooler_158' of github.com:mattkirby/vmpooler into pooler_158

docs/API.md

@@ -816,6 +816,7 @@ An authentication token is required in order to request instances on demand when
 Responses:
 * 201 - Provisioning request accepted
 * 400 - Payload contains invalid JSON and cannot be parsed
+* 401 - No auth token provided, or provided auth token is not valid, and auth is enabled
 * 403 - Request exceeds the configured per pool maximum
 * 404 - A pool was requested, which is not available in the running configuration, or an unknown error occurred.
 * 409 - A request of the matching ID has already been created
@@ -879,6 +880,7 @@ Deleting a ondemand request will delete any instances created for the request an
 
 Responses:
 * 200 - The API request was sucessful. A message will indicate if the request has already been deleted.
+* 401 - No auth token provided, or provided auth token is not valid, and auth is enabled
 * 404 - The request can not be found, or an unknown error occurred.
 ```
 $ curl -X DELETE https://vmpooler.example.com/api/v1/ondemandvm/e3ff6271-d201-4f31-a315-d17f4e15863a

lib/vmpooler.rb

@@ -78,7 +78,7 @@ module Vmpooler
     parsed_config[:config]['vm_lifetime_auth'] = string_to_int(ENV['VM_LIFETIME_AUTH']) if ENV['VM_LIFETIME_AUTH']
     parsed_config[:config]['max_tries'] = string_to_int(ENV['MAX_TRIES']) if ENV['MAX_TRIES']
     parsed_config[:config]['retry_factor'] = string_to_int(ENV['RETRY_FACTOR']) if ENV['RETRY_FACTOR']
-    parsed_config[:config]['create_folders'] = ENV['CREATE_FOLDERS'] if ENV['CREATE_FOLDERS']
+    parsed_config[:config]['create_folders'] = true?(ENV['CREATE_FOLDERS']) if ENV['CREATE_FOLDERS']
     parsed_config[:config]['create_template_delta_disks'] = ENV['CREATE_TEMPLATE_DELTA_DISKS'] if ENV['CREATE_TEMPLATE_DELTA_DISKS']
     set_linked_clone(parsed_config)
     parsed_config[:config]['experimental_features'] = ENV['EXPERIMENTAL_FEATURES'] if ENV['EXPERIMENTAL_FEATURES']

lib/vmpooler/api/v1.rb

@@ -838,6 +838,33 @@ module Vmpooler
       JSON.pretty_generate(result)
     end
 
+    post "#{api_prefix}/ondemandvm/:template/?" do
+      content_type :json
+      result = { 'ok' => false }
+
+      need_token! if Vmpooler::API.settings.config[:auth]
+
+      payload = extract_templates_from_query_params(params[:template])
+
+      if payload
+        invalid = invalid_templates(payload.reject { |k, _v| k == 'request_id' })
+        if invalid.empty?
+          result = generate_ondemand_request(payload)
+        else
+          result[:bad_templates] = invalid
+          invalid.each do |bad_template|
+            metrics.increment('ondemandrequest.invalid.' + bad_template)
+          end
+          status 404
+        end
+      else
+        metrics.increment('ondemandrequest.invalid.unknown')
+        status 404
+      end
+
+      JSON.pretty_generate(result)
+    end
+
     get "#{api_prefix}/ondemandvm/:requestid/?" do
       content_type :json
 

lib/vmpooler/pool_manager.rb

@@ -153,6 +153,7 @@ module Vmpooler
           redis.sadd("vmpooler__#{request_id}__#{pool_alias}__#{pool}", vm)
         end
         move_vm_queue(pool, vm, 'pending', 'running', redis)
+        check_ondemand_request_ready(request_id, redis)
       else
         redis.smove('vmpooler__pending__' + pool, 'vmpooler__ready__' + pool, vm)
       end
@@ -1486,22 +1487,30 @@ module Vmpooler
     end
 
     def check_ondemand_requests_ready(redis)
-      # default expiration is one month to ensure the data does not stay in redis forever
-      default_expiration = 259_200_0
       in_progress_requests = redis.zrange('vmpooler__provisioning__processing', 0, -1, with_scores: true)
       in_progress_requests&.each do |request_id, score|
-        next if request_expired?(request_id, score, redis)
+        check_ondemand_request_ready(request_id, redis, score)
-        next unless vms_ready?(request_id, redis)
-
-        redis.multi
-        redis.hset("vmpooler__odrequest__#{request_id}", 'status', 'ready')
-        redis.expire("vmpooler__odrequest__#{request_id}", default_expiration)
-        redis.zrem('vmpooler__provisioning__processing', request_id)
-        redis.exec
       end
       in_progress_requests.length
     end
 
+    def check_ondemand_request_ready(request_id, redis, score = nil)
+      # default expiration is one month to ensure the data does not stay in redis forever
+      default_expiration = 259_200_0
+      processing_key = 'vmpooler__provisioning__processing'
+      ondemand_hash_key = "vmpooler__odrequest__#{request_id}"
+      score ||= redis.zscore(processing_key, request_id)
+      return if request_expired?(request_id, score, redis)
+
+      return unless vms_ready?(request_id, redis)
+
+      redis.multi
+      redis.hset(ondemand_hash_key, 'status', 'ready')
+      redis.expire(ondemand_hash_key, default_expiration)
+      redis.zrem(processing_key, request_id)
+      redis.exec
+    end
+
     def request_expired?(request_id, score, redis)
       delta = Time.now.to_i - score.to_i
       ondemand_request_ttl = $config[:config]['ondemand_request_ttl']

lib/vmpooler/providers/vsphere.rb

@@ -348,7 +348,7 @@ module Vmpooler
 
             begin
               vm_target_folder = find_vm_folder(pool_name, connection)
-              vm_target_folder = create_folder(connection, target_folder_path, target_datacenter_name) if vm_target_folder.nil? && @config[:config].key?('create_folders') && (@config[:config]['create_folders'] == true)
+              vm_target_folder ||= create_folder(connection, target_folder_path, target_datacenter_name) if @config[:config].key?('create_folders') && (@config[:config]['create_folders'] == true)
             rescue StandardError
               if @config[:config].key?('create_folders') && (@config[:config]['create_folders'] == true)
                 vm_target_folder = create_folder(connection, target_folder_path, target_datacenter_name)
@@ -356,6 +356,7 @@ module Vmpooler
                 raise
               end
             end
+            raise ArgumentError, "Can not find the configured folder for #{pool_name} #{target_folder_path}" unless vm_target_folder
 
             # Create the new VM
             new_vm_object = template_vm_object.CloneVM_Task(

spec/unit/pool_manager_spec.rb

@@ -315,6 +315,11 @@ EOT
 
     context 'with request_id' do
       context 'with a pending request' do
+        before(:each) do
+          allow(subject).to receive(:check_ondemand_request_ready)
+          config[:config]['ondemand_request_ttl'] = 20
+        end
+
         it 'sets the vm as active' do
           redis_connection_pool.with do |redis|
             expect(Time).to receive(:now).and_return(current_time).at_least(:once)
@@ -386,6 +391,8 @@ EOT
         let(:platforms_string) { "#{platform_alias}:#{pool}:1" }
         let(:score) { current_time.to_i }
         before(:each) do
+          config[:config]['ondemand_request_ttl'] = 20
+          allow(subject).to receive(:check_ondemand_request_ready)
           redis_connection_pool.with do |redis|
             create_ondemand_request_for_test(request_id, score, platforms_string, redis, user, token)
           end
@@ -4888,7 +4895,6 @@ EOT
   end
 
   describe '#check_ondemand_requests_ready' do
-
     before(:each) do
       config[:config]['ondemand_request_ttl'] = 5
     end
@@ -4914,30 +4920,52 @@ EOT
           expect(result).to eq(1)
         end
       end
+    end
+  end
+
+  describe '#check_ondemand_request_ready' do
+    let(:score) { current_time.to_f }
+    before(:each) do
+      config[:config]['ondemand_request_ttl'] = 5
+    end
 
     context 'when the request is ready' do
       before(:each) do
         expect(subject).to receive(:vms_ready?).and_return(true)
+        redis_connection_pool.with do |redis|
+          expect(redis).to receive(:zscore).and_return(score)
+        end
       end
 
       it 'sets the request as ready' do
         redis_connection_pool.with do |redis|
           expect(redis).to receive(:hset).with("vmpooler__odrequest__#{request_id}", 'status', 'ready')
-            subject.check_ondemand_requests_ready(redis)
+          subject.check_ondemand_request_ready(request_id, redis)
         end
       end
 
       it 'marks the ondemand request hash key for expiration in one month' do
         redis_connection_pool.with do |redis|
           expect(redis).to receive(:expire).with("vmpooler__odrequest__#{request_id}", 2592000)
-            subject.check_ondemand_requests_ready(redis)
+          subject.check_ondemand_request_ready(request_id, redis)
         end
       end
 
       it 'removes the request from processing' do
         redis_connection_pool.with do |redis|
           expect(redis).to receive(:zrem).with('vmpooler__provisioning__processing', request_id)
-            subject.check_ondemand_requests_ready(redis)
+          subject.check_ondemand_request_ready(request_id, redis)
+        end
+      end
+    end
+
+    context 'with the score provided' do
+      it 'should not request the score' do
+        redis_connection_pool.with do |redis|
+          expect(redis).to_not receive(:zscore)
+          expect(subject).to receive(:vms_ready?).and_return(true)
+          expect(redis).to receive(:zrem).with('vmpooler__provisioning__processing', request_id)
+          subject.check_ondemand_request_ready(request_id, redis, score)
         end
       end
     end
@@ -4945,9 +4973,9 @@ EOT
     context 'when a request has taken too long to be filled' do
       it 'should return true for request_expired?' do
         redis_connection_pool.with do |redis|
+          expect(redis).to receive(:zscore).and_return(score)
           expect(subject).to receive(:request_expired?).with(request_id, Float, redis).and_return(true)
-            subject.check_ondemand_requests_ready(redis)
+          subject.check_ondemand_request_ready(request_id, redis)
-          end
         end
       end
     end

spec/unit/providers/vsphere_spec.rb

@@ -646,6 +646,18 @@ EOT
       end
     end
 
+    context 'when create_vm_folder returns nil' do
+      before(:each) do
+        template_vm = new_template_object
+        allow(subject).to receive(:find_template_vm).and_return(new_template_object)
+        expect(subject).to receive(:find_vm_folder).and_return(nil)
+      end
+
+      it 'should raise an error' do
+        expect{ subject.create_vm(poolname, vmname) }.to raise_error(ArgumentError)
+      end
+    end
+
     context 'Given a successful creation' do
       let(:folder_object) { mock_RbVmomi_VIM_Folder({ :name => 'pool1'}) }
       before(:each) do