From c5b9e1d184e122add1172c8382ac2b38ed8f716a Mon Sep 17 00:00:00 2001 From: Jesse Scott Date: Tue, 22 Sep 2020 12:07:11 -0700 Subject: [PATCH 1/7] Update completion scripts for `service` subcommands --- extras/completions/floaty.bash | 24 ++++++++++++++++-------- extras/completions/floaty.zsh | 20 +++++++++++++------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/extras/completions/floaty.bash b/extras/completions/floaty.bash index 0c18c49..17a5b24 100644 --- a/extras/completions/floaty.bash +++ b/extras/completions/floaty.bash @@ -2,29 +2,37 @@ _vmfloaty() { - local cur prev subcommands template_subcommands hostname_subcommands + local cur prev commands template_arg_commands hostname_arg_commands service_subcommands + COMPREPLY=() cur="${COMP_WORDS[COMP_CWORD]}" prev="${COMP_WORDS[COMP_CWORD-1]}" - subcommands="delete get help list modify query revert service snapshot ssh status summary token" - template_subcommands="get ssh" - hostname_subcommands="delete modify query revert snapshot" + commands="delete get help list modify query revert service snapshot ssh status summary token" + template_arg_commands="get ssh" + hostname_arg_commands="delete modify query revert snapshot" + service_subcommands="types examples" if [[ $cur == -* ]] ; then # TODO: option completion COMPREPLY=() - elif [[ $template_subcommands =~ (^| )$prev($| ) ]] ; then + elif [[ $template_arg_commands =~ (^| )$prev($| ) ]] ; then if [[ -z "$_vmfloaty_avail_templates" ]] ; then + # TODO: need a --hostnameonly equivalent here because the section headers of + # `floaty list` are adding some spurious entries (including files in current + # directory because part of the headers is `**` which is getting expanded) _vmfloaty_avail_templates=$(floaty list 2>/dev/null) fi COMPREPLY=( $(compgen -W "${_vmfloaty_avail_templates}" -- "${cur}") ) - elif [[ $hostname_subcommands =~ (^| )$prev($| ) ]] ; then + elif [[ $hostname_arg_commands =~ (^| )$prev($| ) ]] ; then _vmfloaty_active_hostnames=$(floaty list --active --hostnameonly 2>/dev/null) COMPREPLY=( $(compgen -W "${_vmfloaty_active_hostnames}" -- "${cur}") ) - else - COMPREPLY=( $(compgen -W "${subcommands}" -- "${cur}") ) + elif [[ "service" == $prev ]] ; then + COMPREPLY=( $(compgen -W "${service_subcommands}" -- "${cur}") ) + elif [[ $1 == $prev ]] ; then + # only show top level commands we are at root + COMPREPLY=( $(compgen -W "${commands}" -- "${cur}") ) fi } complete -F _vmfloaty floaty diff --git a/extras/completions/floaty.zsh b/extras/completions/floaty.zsh index 77edf17..1dbc292 100644 --- a/extras/completions/floaty.zsh +++ b/extras/completions/floaty.zsh @@ -1,26 +1,32 @@ _floaty() { - local line subcommands template_subcommands hostname_subcommands + local line commands template_arg_commands hostname_arg_commands service_subcommands - subcommands="delete get help list modify query revert snapshot ssh status summary token" + commands="delete get help list modify query revert service snapshot ssh status summary token" - template_subcommands=("get" "ssh") - hostname_subcommands=("delete" "modify" "query" "revert" "snapshot") + template_arg_commands=("get" "ssh") + hostname_arg_commands=("delete" "modify" "query" "revert" "snapshot") + service_subcommands=("types" "examples") _arguments -C \ - "1: :(${subcommands})" \ + "1: :(${commands})" \ "*::arg:->args" - if ((template_subcommands[(Ie)$line[1]])); then + if ((template_arg_commands[(Ie)$line[1]])); then _floaty_template_sub - elif ((hostname_subcommands[(Ie)$line[1]])); then + elif ((hostname_arg_commands[(Ie)$line[1]])); then _floaty_hostname_sub + elif [[ "service" == $line[1] ]]; then + _arguments "1: :(${service_subcommands})" fi } _floaty_template_sub() { if [[ -z "$_vmfloaty_avail_templates" ]] ; then + # TODO: need a --hostnameonly equivalent here because the section headers of + # `floaty list` are adding some spurious entries (including files in current + # directory because part of the headers is `**` which is getting expanded) _vmfloaty_avail_templates=$(floaty list 2>/dev/null) fi From 5c8aa13081a6f00bebc80d56a9f4ec149ae22b3f Mon Sep 17 00:00:00 2001 From: Christopher Thorn Date: Wed, 23 Sep 2020 15:06:24 -0700 Subject: [PATCH 2/7] (maint) Add vmpooler_fallback to the get service check Previously the vmpooler_fallback setting was not being recognizied, this PR will fix it. --- lib/vmfloaty/utils.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/vmfloaty/utils.rb b/lib/vmfloaty/utils.rb index a569415..5f43345 100644 --- a/lib/vmfloaty/utils.rb +++ b/lib/vmfloaty/utils.rb @@ -254,6 +254,7 @@ class Utils 'url' => config['url'], 'user' => config['user'], 'token' => config['token'], + 'vmpooler_fallback' => config['vmpooler_fallback'], 'type' => config['type'] || 'vmpooler', } From 3a60ffbdbd58daaa964c4b62bfa92e991681090c Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Thu, 24 Sep 2020 09:46:22 -0500 Subject: [PATCH 3/7] (maint) Don't require config file for list --active Will fallback to printing ABS information, which is less than what you get if vmpooler_fallback is available --- lib/vmfloaty/utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmfloaty/utils.rb b/lib/vmfloaty/utils.rb index a569415..5a2775b 100644 --- a/lib/vmfloaty/utils.rb +++ b/lib/vmfloaty/utils.rb @@ -116,7 +116,7 @@ class Utils output_target.puts "- [JobID:#{host_data['request']['job']['id']}] <#{host_data['state']}>" host_data['allocated_resources'].each do |allocated_resources, _i| - if allocated_resources['engine'] == "vmpooler" + if allocated_resources['engine'] == "vmpooler" && service.config["vmpooler_fallback"] vmpooler_service = service.clone vmpooler_service.silent = true vmpooler_service.maybe_use_vmpooler From 6deadb22674e8a1b0bee4a77b3547b161f78ce14 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Thu, 24 Sep 2020 10:00:15 -0500 Subject: [PATCH 4/7] Fix tests --- spec/vmfloaty/utils_spec.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/spec/vmfloaty/utils_spec.rb b/spec/vmfloaty/utils_spec.rb index e26e0f1..a90d3b1 100644 --- a/spec/vmfloaty/utils_spec.rb +++ b/spec/vmfloaty/utils_spec.rb @@ -5,6 +5,11 @@ require 'json' require 'commander/command' require_relative '../../lib/vmfloaty/utils' +# allow changing config in service for tests +class Service + attr_writer :config +end + describe Utils do describe '#standardize_hostnames' do before :each do @@ -441,7 +446,7 @@ describe Utils do end let(:default_output_first_line) { "- [JobID:#{hostname}] " } - let(:default_output_second_line) { " - #{fqdn} (#{template}, 7.67/48 hours, user: bob, role: agent)" } + let(:default_output_second_line) { " - #{fqdn} (#{template})" } it 'prints output with job id, host, and template' do expect(STDOUT).to receive(:puts).with(default_output_first_line) @@ -450,6 +455,16 @@ describe Utils do subject end + it 'prints more information when vmpooler_fallback is set output with job id, host, template, lifetime, user and role' do + fallback = {'vmpooler_fallback' => 'vmpooler'} + service.config.merge! fallback + default_output_second_line=" - #{fqdn} (#{template}, 7.67/48 hours, user: bob, role: agent)" + expect(STDOUT).to receive(:puts).with(default_output_first_line) + expect(STDOUT).to receive(:puts).with(default_output_second_line) + + subject + end + context 'when print_to_stderr option is true' do let(:print_to_stderr) { true } @@ -529,7 +544,7 @@ describe Utils do end let(:default_output_first_line) { "- [JobID:#{hostname}] " } - let(:default_output_second_line) { " - #{fqdn} (#{template}, 7.67/48 hours, user: bob, role: agent)" } + let(:default_output_second_line) { " - #{fqdn} (#{template})" } let(:default_output_third_line) { " - #{fqdn_ns} (#{template_ns})" } it 'prints output with job id, host, and template' do From fbd98f93aefc722a3821401a07bbbef1ff15b89d Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Fri, 9 Oct 2020 10:18:53 -0500 Subject: [PATCH 5/7] (maint) Fix bug with detecting ABS service Before this change the code would try to check if an object was ABS by using is_a? which is only for instances of a class. It also compared with ABS.class which returns the Class class. Now fixed by comparing the object to the static class --- lib/vmfloaty/service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmfloaty/service.rb b/lib/vmfloaty/service.rb index a0303c4..ee41648 100644 --- a/lib/vmfloaty/service.rb +++ b/lib/vmfloaty/service.rb @@ -139,7 +139,7 @@ class Service # some methods do not exist for ABS, and if possible should target the Pooler service def maybe_use_vmpooler - if @service_object.is_a?(ABS.class) + if @service_object == ABS # this is not an instance if !self.silent FloatyLogger.info "The service in use is ABS, but the requested method should run against vmpooler directly, using fallback_vmpooler config from ~/.vmfloaty.yml" self.silent = true From e0fac0bb6c9ee7771f506b1a98cd9a9d0eba793f Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Fri, 9 Oct 2020 10:23:23 -0500 Subject: [PATCH 6/7] (maint) Add more uniqueness to jobid Before this change there was a non zero chance that two requests could be received at the same millisecond and have the same jobid. Added the username as a prefix to the job id --- lib/vmfloaty/abs.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vmfloaty/abs.rb b/lib/vmfloaty/abs.rb index 2426c32..1d1755a 100644 --- a/lib/vmfloaty/abs.rb +++ b/lib/vmfloaty/abs.rb @@ -248,7 +248,7 @@ class ABS conn = Http.get_conn(verbose, url) conn.headers['X-AUTH-TOKEN'] = token if token - saved_job_id = DateTime.now.strftime('%Q') + saved_job_id = user + "-" + DateTime.now.strftime('%Q') vmpooler_config = Utils.get_vmpooler_service_config(config['vmpooler_fallback']) req_obj = { :resources => os_types, From 671623bc4f13d77ff91c2b755560520cdd6c5f0f Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Fri, 9 Oct 2020 10:37:16 -0500 Subject: [PATCH 7/7] handle ctrl-c and term signal and return useful message on how to query ABS for the state of the request or to delete it --- lib/vmfloaty/abs.rb | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/vmfloaty/abs.rb b/lib/vmfloaty/abs.rb index 1d1755a..4eb657e 100644 --- a/lib/vmfloaty/abs.rb +++ b/lib/vmfloaty/abs.rb @@ -284,15 +284,20 @@ class ABS validate_queue_status_response(res.status, res.body, "Initial request", verbose) - (1..retries).each do |i| - queue_place, res_body = check_queue(conn, saved_job_id, req_obj, verbose) - return translated(res_body, saved_job_id) if res_body + begin + (1..retries).each do |i| + queue_place, res_body = check_queue(conn, saved_job_id, req_obj, verbose) + return translated(res_body, saved_job_id) if res_body - sleep_seconds = 10 if i >= 10 - sleep_seconds = i if i < 10 - FloatyLogger.info "Waiting #{sleep_seconds} seconds to check if ABS request has been filled. Queue Position: #{queue_place}... (x#{i})" + sleep_seconds = 10 if i >= 10 + sleep_seconds = i if i < 10 + FloatyLogger.info "Waiting #{sleep_seconds} seconds to check if ABS request has been filled. Queue Position: #{queue_place}... (x#{i})" - sleep(sleep_seconds) + sleep(sleep_seconds) + end + rescue SystemExit, Interrupt + FloatyLogger.info "\n\nFloaty interrupted, you can query the state of your request via\n1) `floaty query #{saved_job_id}` or delete it via\n2) `floaty delete #{saved_job_id}`" + exit 1 end nil end