add some metrics to the od request generation

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

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|

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

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)