Check each ondemand request to see if it is ready when moving an instance to ready

This commit updates how ondemand requests are checked to include a check to see if the request is ready when an instance is added to the ready instances set. Without this change the delay between a check for the instances being ready and the last instance becoming availalbe could cause the API to show all instances are ready, but the request not be considered ready yet.

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)
-        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
+        check_ondemand_request_ready(request_id, redis, score)
       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']

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,40 +4920,62 @@ EOT
           expect(result).to eq(1)
         end
       end
+    end
+  end
 
-      context 'when the request is ready' do
-        before(:each) do
-          expect(subject).to receive(:vms_ready?).and_return(true)
-        end
+  describe '#check_ondemand_request_ready' do
+    let(:score) { current_time.to_f }
+    before(:each) do
+      config[:config]['ondemand_request_ttl'] = 5
+    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)
-          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)
-          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)
-          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
 
-      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(subject).to receive(:request_expired?).with(request_id, Float, redis).and_return(true)
-            subject.check_ondemand_requests_ready(redis)
-          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_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_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_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
+
+    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_request_ready(request_id, redis)
         end
       end
     end