From c6fdc8d6fdbc9259cd9259537c142e63fa596173 Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Wed, 5 Aug 2020 15:42:27 -0700 Subject: [PATCH] (POOLER-186) Fix template alias evaluation with backend weight of 0 This change fixes template alias evaluation to ensure that the correct data is set when generating on demand requests for pools that have a backend weight configured for a value of 0. Without this change vmpooler will return an empty selection in api for template alias evaluation. To support this change tests are added that first reproduced the failure, and then verified that it is resolved with the addition of the patch. Additionally, test coverage is added to ensure that code paths that include pickup gem usage are covered. --- lib/vmpooler/api/v1.rb | 14 +++++------ spec/integration/api/v1/ondemandvm_spec.rb | 28 ++++++++++++++++++---- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/lib/vmpooler/api/v1.rb b/lib/vmpooler/api/v1.rb index 3146cc6..066af86 100644 --- a/lib/vmpooler/api/v1.rb +++ b/lib/vmpooler/api/v1.rb @@ -89,18 +89,16 @@ module Vmpooler template_backends += aliases weighted_pools = get_pool_weights(template_backends) - pickup = Pickup.new(weighted_pools) if weighted_pools.count == template_backends.count - count.to_i.times do - if pickup + if weighted_pools.count > 1 && weighted_pools.count == template_backends.count + pickup = Pickup.new(weighted_pools) + count.to_i.times do selection << pickup.pick - else + end + else + count.to_i.times do selection << template_backends.sample end end - else - count.to_i.times do - selection << template - end end count_selection(selection) diff --git a/spec/integration/api/v1/ondemandvm_spec.rb b/spec/integration/api/v1/ondemandvm_spec.rb index 37b315c..a361f9f 100644 --- a/spec/integration/api/v1/ondemandvm_spec.rb +++ b/spec/integration/api/v1/ondemandvm_spec.rb @@ -24,15 +24,19 @@ describe Vmpooler::API::V1 do 'vm_lifetime_auth' => 2, 'max_ondemand_instances_per_request' => 50, 'backend_weight' => { - 'compute1' => 5 + 'compute1' => 5, + 'compute2' => 0 } }, pools: [ - {'name' => 'pool1', 'size' => 0}, - {'name' => 'pool2', 'size' => 0, 'clone_target' => 'compute1'}, + {'name' => 'pool1', 'size' => 0, 'clone_target' => 'compute1'}, + {'name' => 'pool2', 'size' => 0, 'clone_target' => 'compute2'}, {'name' => 'pool3', 'size' => 0, 'clone_target' => 'compute1'} ], - alias: { 'poolone' => ['pool1'] }, + alias: { + 'poolone' => ['pool1'], + 'pool2' => ['pool1'] + }, pool_names: [ 'pool1', 'pool2', 'pool3', 'poolone' ] } } @@ -92,6 +96,22 @@ describe Vmpooler::API::V1 do post "#{prefix}/ondemandvm", '{"poolone":"1"}' end + context 'with a backend of 0 weight' do + before(:each) do + config[:config]['backend_weight']['compute1'] = 0 + end + + it 'sets the platform string in redis for the request to indicate the selected platforms' do + expect(redis).to receive(:hset).with("vmpooler__odrequest__#{uuid}", 'requested', 'pool1:pool1:1') + post "#{prefix}/ondemandvm", '{"pool1":"1"}' + end + end + + it 'sets the platform string in redis for the request to indicate the selected platforms using weight' do + expect(redis).to receive(:hset).with("vmpooler__odrequest__#{uuid}", 'requested', 'pool2:pool1:1') + post "#{prefix}/ondemandvm", '{"pool2":"1"}' + end + context 'with domain set in the config' do let(:domain) { 'example.com' } before(:each) do