diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index f1d05cc..140eaff 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -5,6 +5,7 @@ require 'vmpooler/util/parsing' module Vmpooler class API class V1 < Sinatra::Base + api_version = '1' api_prefix = "/api/v#{api_version}" @@ -336,9 +337,9 @@ module Vmpooler end def too_many_requested?(payload) - payload&.each do |_poolname, count| + payload&.each do |poolname, count| next unless count.to_i > config['max_ondemand_instances_per_request'] - + metrics.increment('ondemandrequest.toomanyrequests.' + poolname) return true end false @@ -362,6 +363,7 @@ module Vmpooler if backend.exists("vmpooler__odrequest__#{request_id}") result['message'] = "request_id '#{request_id}' has already been created" status 409 + metrics.increment('ondemandrequest.generate.duplicaterequests') return result end @@ -385,6 +387,7 @@ module Vmpooler result['domain'] = config['domain'] if config['domain'] result[:ok] = true + metrics.increment('ondemandrequest.generate.success') result end @@ -976,7 +979,7 @@ module Vmpooler if request_hash['status'] == 'ready' result['ready'] = true - get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,_count| + Parsing.get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,_count| instances = backend.smembers("vmpooler__#{request_id}__#{platform_alias}__#{pool}") result[platform_alias] = { 'hostname': instances } end @@ -989,7 +992,7 @@ module Vmpooler result['message'] = 'The request has been deleted' status 200 else - get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,count| + Parsing.get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,count| instance_count = backend.scard("vmpooler__#{request_id}__#{platform_alias}__#{pool}") result[platform_alias] = { 'ready': instance_count.to_s, @@ -1015,7 +1018,7 @@ module Vmpooler else backend.hset("vmpooler__odrequest__#{request_id}", 'status', 'deleted') - get_platform_pool_count(platforms) do |platform_alias,pool,_count| + Parsing.get_platform_pool_count(platforms) do |platform_alias,pool,_count| backend.smembers("vmpooler__#{request_id}__#{platform_alias}__#{pool}")&.each do |vm| backend.smove("vmpooler__running__#{pool}", "vmpooler__completed__#{pool}", vm) end diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 8457c96..b5f97a6 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -1501,7 +1501,7 @@ module Vmpooler def vms_ready?(request_id, redis) catch :request_not_ready do request_hash = redis.hgetall("vmpooler__odrequest__#{request_id}") - get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,count| + Parsing.get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,count| pools_filled = redis.scard("vmpooler__#{request_id}__#{platform_alias}__#{pool}") throw :request_not_ready unless pools_filled.to_i == count.to_i end @@ -1553,7 +1553,7 @@ module Vmpooler def remove_vms_for_failed_request(request_id, expiration_ttl, redis) request_hash = redis.hgetall("vmpooler__odrequest__#{request_id}") - get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,_count| + Parsing.get_platform_pool_count(request_hash['requested']) do |platform_alias,pool,_count| pools_filled = redis.smembers("vmpooler__#{request_id}__#{platform_alias}__#{pool}") redis.pipelined do pools_filled&.each do |vm| diff --git a/lib/vmpooler/util/parsing.rb b/lib/vmpooler/util/parsing.rb index 89ed9ca..a44b716 100644 --- a/lib/vmpooler/util/parsing.rb +++ b/lib/vmpooler/util/parsing.rb @@ -1,11 +1,11 @@ # utility class shared between apps module Vmpooler class Parsing - def get_platform_pool_count(requested, &block) + def self.get_platform_pool_count(requested, &block) requested_platforms = requested.split(',') requested_platforms.each do |platform| platform_alias, pool, count = platform.split(':') - #raise ArgumentError if platform_alias.nil? || pool.nil? || count.nil? + raise ArgumentError if platform_alias.nil? || pool.nil? || count.nil? yield platform_alias, pool, count end end diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index e886138..8e75f45 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -4872,6 +4872,14 @@ EOT end end + it 'fails if the request is not valid' do + redis_connection_pool.with do |redis| + request_id = "#{request_id}-wrong" + create_ondemand_request_for_test(request_id, current_time.to_i, "#{pool}:5", redis) + expect{subject.vms_ready?(request_id, redis)}.to raise_error(ArgumentError) + end + end + it 'returns false when vms for request_id are not ready' do redis_connection_pool.with do |redis| result = subject.vms_ready?(request_id, redis)