From 4df970b21fd835e4e46133780b95f997e706602e Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Mon, 21 Sep 2020 14:39:10 -0500 Subject: [PATCH 1/2] (maint) Fix for ABS PR#306 that includes json responses Before this change ABS sometiimes returned a string of JSON with escaped quotes, that had to be parsed again With this change, floaty should accept both the legacy and the new full JSON responses. --- lib/vmfloaty/abs.rb | 28 +++++++++++++++++++++++----- spec/vmfloaty/abs_spec.rb | 6 +++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/vmfloaty/abs.rb b/lib/vmfloaty/abs.rb index 6b5c0e1..d75a658 100644 --- a/lib/vmfloaty/abs.rb +++ b/lib/vmfloaty/abs.rb @@ -77,8 +77,10 @@ def self.get_active_requests(verbose, url, user) requests.each do |req| next if req == 'null' - if valid_json?(req) + if valid_json?(req) # legacy ABS had another JSON string always-be-scheduling/pull/306 req_hash = JSON.parse(req) + elsif req.is_a?(Hash) + req_hash = req else FloatyLogger.warn "Warning: couldn't parse request returned from abs/status/queue" next @@ -170,7 +172,11 @@ def self.list(verbose, url, os_filter = nil) res_body = JSON.parse(res.body) if res_body.key?('vmpooler_platforms') os_list << '*** VMPOOLER Pools ***' - os_list += JSON.parse(res_body['vmpooler_platforms']) + if res_body['vmpooler_platforms'].is_a?(Hash) + os_list += res_body['vmpooler_platforms'] + else + os_list += JSON.parse(res_body['vmpooler_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + end end end @@ -180,7 +186,11 @@ def self.list(verbose, url, os_filter = nil) if res_body.key?('ondemand_vmpooler_platforms') && res_body['ondemand_vmpooler_platforms'] != '[]' os_list << '' os_list << '*** VMPOOLER ONDEMAND Pools ***' - os_list += JSON.parse(res_body['ondemand_vmpooler_platforms']) + if res_body['ondemand_vmpooler_platforms'].is_a?(Hash) + os_list += res_body['ondemand_vmpooler_platforms'] + else + os_list += JSON.parse(res_body['ondemand_vmpooler_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + end end end @@ -190,7 +200,11 @@ def self.list(verbose, url, os_filter = nil) if res_body.key?('nspooler_platforms') os_list << '' os_list << '*** NSPOOLER Pools ***' - os_list += JSON.parse(res_body['nspooler_platforms']) + if res_body['nspooler_platforms'].is_a?(Hash) + os_list += res_body['nspooler_platforms'] + else + os_list += JSON.parse(res_body['nspooler_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + end end end @@ -200,7 +214,11 @@ def self.list(verbose, url, os_filter = nil) if res_body.key?('aws_platforms') os_list << '' os_list << '*** AWS Pools ***' - os_list += JSON.parse(res_body['aws_platforms']) + if res_body['aws_platforms'].is_a?(Hash) + os_list += res_body['aws_platforms'] + else + os_list += JSON.parse(res_body['aws_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + end end end diff --git a/spec/vmfloaty/abs_spec.rb b/spec/vmfloaty/abs_spec.rb index c075c56..5e82993 100644 --- a/spec/vmfloaty/abs_spec.rb +++ b/spec/vmfloaty/abs_spec.rb @@ -71,9 +71,9 @@ # rubocop:disable Layout/LineLength @active_requests_response = ' [ - "{ \"state\":\"allocated\",\"last_processed\":\"2019-12-16 23:00:34 +0000\",\"allocated_resources\":[{\"hostname\":\"take-this.delivery.puppetlabs.net\",\"type\":\"win-2012r2-x86_64\",\"engine\":\"vmpooler\"}],\"audit_log\":{\"2019-12-13 16:45:29 +0000\":\"Allocated take-this.delivery.puppetlabs.net for job 1576255517241\"},\"request\":{\"resources\":{\"win-2012r2-x86_64\":1},\"job\":{\"id\":\"1576255517241\",\"tags\":{\"user\":\"test-user\"},\"user\":\"test-user\",\"time-received\":1576255519},\"priority\":1}}", + { "state":"allocated","last_processed":"2019-12-16 23:00:34 +0000","allocated_resources":[{"hostname":"take-this.delivery.puppetlabs.net","type":"win-2012r2-x86_64","engine":"vmpooler"}],"audit_log":{"2019-12-13 16:45:29 +0000":"Allocated take-this.delivery.puppetlabs.net for job 1576255517241"},"request":{"resources":{"win-2012r2-x86_64":1},"job":{"id":"1576255517241","tags":{"user":"test-user"},"user":"test-user","time-received":1576255519},"priority":1}}, "null", - "{\"state\":\"allocated\",\"last_processed\":\"2019-12-16 23:00:34 +0000\",\"allocated_resources\":[{\"hostname\":\"not-this.delivery.puppetlabs.net\",\"type\":\"win-2012r2-x86_64\",\"engine\":\"vmpooler\"}],\"audit_log\":{\"2019-12-13 16:46:14 +0000\":\"Allocated not-this.delivery.puppetlabs.net for job 1576255565159\"},\"request\":{\"resources\":{\"win-2012r2-x86_64\":1},\"job\":{\"id\":\"1576255565159\",\"tags\":{\"user\":\"not-test-user\"},\"user\":\"not-test-user\",\"time-received\":1576255566},\"priority\":1}}" + {"state":"allocated","last_processed":"2019-12-16 23:00:34 +0000","allocated_resources":[{"hostname":"not-this.delivery.puppetlabs.net","type":"win-2012r2-x86_64","engine":"vmpooler"}],"audit_log":{"2019-12-13 16:46:14 +0000":"Allocated not-this.delivery.puppetlabs.net for job 1576255565159"},"request":{"resources":{"win-2012r2-x86_64":1},"job":{"id":"1576255565159","tags":{"user":"not-test-user"},"user":"not-test-user","time-received":1576255566},"priority":1}} ]' # rubocop:enable Layout/LineLength @token = 'utpg2i2xswor6h8ttjhu3d47z53yy47y' @@ -101,7 +101,7 @@ # rubocop:disable Layout/LineLength @active_requests_response = ' [ - "{ \"state\":\"allocated\", \"last_processed\":\"2020-01-17 22:29:13 +0000\", \"allocated_resources\":[{\"hostname\":\"craggy-chord.delivery.puppetlabs.net\", \"type\":\"centos-7-x86_64\", \"engine\":\"vmpooler\"}, {\"hostname\":\"visible-revival.delivery.puppetlabs.net\", \"type\":\"centos-7-x86_64\", \"engine\":\"vmpooler\"}], \"audit_log\":{\"2020-01-17 22:28:45 +0000\":\"Allocated craggy-chord.delivery.puppetlabs.net, visible-revival.delivery.puppetlabs.net for job 1579300120799\"}, \"request\":{\"resources\":{\"centos-7-x86_64\":2}, \"job\":{\"id\":\"1579300120799\", \"tags\":{\"user\":\"test-user\"}, \"user\":\"test-user\", \"time-received\":1579300120}, \"priority\":3}}" + { "state":"allocated", "last_processed":"2020-01-17 22:29:13 +0000", "allocated_resources":[{"hostname":"craggy-chord.delivery.puppetlabs.net", "type":"centos-7-x86_64", "engine":"vmpooler"}, {"hostname":"visible-revival.delivery.puppetlabs.net", "type":"centos-7-x86_64", "engine":"vmpooler"}], "audit_log":{"2020-01-17 22:28:45 +0000":"Allocated craggy-chord.delivery.puppetlabs.net, visible-revival.delivery.puppetlabs.net for job 1579300120799"}, "request":{"resources":{"centos-7-x86_64":2}, "job":{"id":"1579300120799", "tags":{"user":"test-user"}, "user":"test-user", "time-received":1579300120}, "priority":3}} ]' @return_request = { '{"job_id":"1579300120799","hosts":{"hostname":"craggy-chord.delivery.puppetlabs.net","type":"centos-7-x86_64","engine":"vmpooler"},{"hostname":"visible-revival.delivery.puppetlabs.net","type":"centos-7-x86_64","engine":"vmpooler"}}'=>true } # rubocop:enable Layout/LineLength From cd7c0fae134747607a4299a881e80689223fe49e Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Mon, 21 Sep 2020 15:20:22 -0500 Subject: [PATCH 2/2] Adding spec tests for #list --- lib/vmfloaty/abs.rb | 24 ++++++++++----------- spec/vmfloaty/abs_spec.rb | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/lib/vmfloaty/abs.rb b/lib/vmfloaty/abs.rb index d75a658..2426c32 100644 --- a/lib/vmfloaty/abs.rb +++ b/lib/vmfloaty/abs.rb @@ -172,10 +172,10 @@ def self.list(verbose, url, os_filter = nil) res_body = JSON.parse(res.body) if res_body.key?('vmpooler_platforms') os_list << '*** VMPOOLER Pools ***' - if res_body['vmpooler_platforms'].is_a?(Hash) - os_list += res_body['vmpooler_platforms'] - else + if res_body['vmpooler_platforms'].is_a?(String) os_list += JSON.parse(res_body['vmpooler_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + else + os_list += res_body['vmpooler_platforms'] end end end @@ -186,10 +186,10 @@ def self.list(verbose, url, os_filter = nil) if res_body.key?('ondemand_vmpooler_platforms') && res_body['ondemand_vmpooler_platforms'] != '[]' os_list << '' os_list << '*** VMPOOLER ONDEMAND Pools ***' - if res_body['ondemand_vmpooler_platforms'].is_a?(Hash) - os_list += res_body['ondemand_vmpooler_platforms'] - else + if res_body['ondemand_vmpooler_platforms'].is_a?(String) os_list += JSON.parse(res_body['ondemand_vmpooler_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + else + os_list += res_body['ondemand_vmpooler_platforms'] end end end @@ -200,10 +200,10 @@ def self.list(verbose, url, os_filter = nil) if res_body.key?('nspooler_platforms') os_list << '' os_list << '*** NSPOOLER Pools ***' - if res_body['nspooler_platforms'].is_a?(Hash) - os_list += res_body['nspooler_platforms'] - else + if res_body['nspooler_platforms'].is_a?(String) os_list += JSON.parse(res_body['nspooler_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + else + os_list += res_body['nspooler_platforms'] end end end @@ -214,10 +214,10 @@ def self.list(verbose, url, os_filter = nil) if res_body.key?('aws_platforms') os_list << '' os_list << '*** AWS Pools ***' - if res_body['aws_platforms'].is_a?(Hash) - os_list += res_body['aws_platforms'] - else + if res_body['aws_platforms'].is_a?(String) os_list += JSON.parse(res_body['aws_platforms']) # legacy ABS had another JSON string always-be-scheduling/pull/306 + else + os_list += res_body['aws_platforms'] end end end diff --git a/spec/vmfloaty/abs_spec.rb b/spec/vmfloaty/abs_spec.rb index 5e82993..40804cf 100644 --- a/spec/vmfloaty/abs_spec.rb +++ b/spec/vmfloaty/abs_spec.rb @@ -9,6 +9,50 @@ before :each do end + describe '#list' do + it 'skips empty platforms and lists aws' do + stub_request(:get, "http://foo/status/platforms/vmpooler"). + to_return(:status => 200, :body => "", :headers => {}) + stub_request(:get, "http://foo/status/platforms/ondemand_vmpooler"). + to_return(:status => 200, :body => "", :headers => {}) + stub_request(:get, "http://foo/status/platforms/nspooler"). + to_return(:status => 200, :body => "", :headers => {}) + body = '{ + "aws_platforms": [ + "amazon-6-x86_64", + "amazon-7-x86_64", + "amazon-7-arm64", + "centos-7-x86-64-west", + "redhat-8-arm64" + ] + }' + stub_request(:get, "http://foo/status/platforms/aws"). + to_return(:status => 200, :body => body, :headers => {}) + + + results = ABS.list(false, "http://foo") + + expect(results).to include("amazon-6-x86_64", "amazon-7-x86_64", "amazon-7-arm64", "centos-7-x86-64-west", "redhat-8-arm64") + end + it 'legacy JSON string, prior to PR 306' do + stub_request(:get, "http://foo/status/platforms/vmpooler"). + to_return(:status => 200, :body => "", :headers => {}) + stub_request(:get, "http://foo/status/platforms/ondemand_vmpooler"). + to_return(:status => 200, :body => "", :headers => {}) + stub_request(:get, "http://foo/status/platforms/nspooler"). + to_return(:status => 200, :body => "", :headers => {}) + body = '{ + "aws_platforms": "[\"amazon-6-x86_64\",\"amazon-7-x86_64\",\"amazon-7-arm64\",\"centos-7-x86-64-west\",\"redhat-8-arm64\"]" + }' + stub_request(:get, "http://foo/status/platforms/aws"). + to_return(:status => 200, :body => body, :headers => {}) + + results = ABS.list(false, "http://foo") + + expect(results).to include("amazon-6-x86_64", "amazon-7-x86_64", "amazon-7-arm64", "centos-7-x86-64-west", "redhat-8-arm64") + end + end + describe '#format' do it 'returns an hash formatted like a vmpooler return, plus the job_id' do job_id = "generated_by_floaty_12345"