From d518d0244c98c639d1501d16b27ae68cf91fbf0a Mon Sep 17 00:00:00 2001 From: Alex Kubacki Date: Thu, 19 Dec 2024 15:07:25 -0500 Subject: [PATCH] Quality of life requests (#360) * Error for zero environments * Support --env as alias for --environment * Unset even if a value is passed * return container profile for service * Bump minor version * Fixes * Show all backups by default. --- Gemfile.lock | 2 +- lib/aptible/cli/helpers/environment.rb | 16 +++++++---- lib/aptible/cli/resource_formatter.rb | 1 + lib/aptible/cli/subcommands/apps.rb | 14 ++++++---- lib/aptible/cli/subcommands/backup.rb | 11 ++++---- lib/aptible/cli/subcommands/config.rb | 2 +- lib/aptible/cli/subcommands/db.rb | 28 +++++++++---------- lib/aptible/cli/subcommands/environment.rb | 4 +-- lib/aptible/cli/subcommands/log_drain.rb | 6 ++-- lib/aptible/cli/subcommands/maintenance.rb | 4 +-- lib/aptible/cli/subcommands/metric_drain.rb | 12 ++++---- lib/aptible/cli/version.rb | 2 +- spec/aptible/cli/subcommands/apps_spec.rb | 8 ++++-- spec/aptible/cli/subcommands/config_spec.rb | 11 ++++++++ spec/aptible/cli/subcommands/services_spec.rb | 8 ++++++ 15 files changed, 81 insertions(+), 48 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 6729b126..4c4a5ee9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - aptible-cli (0.24.0) + aptible-cli (0.24.1) activesupport (>= 4.0, < 6.0) aptible-api (~> 1.6.5) aptible-auth (~> 1.2.5) diff --git a/lib/aptible/cli/helpers/environment.rb b/lib/aptible/cli/helpers/environment.rb index 7e2910a3..bc8fd230 100644 --- a/lib/aptible/cli/helpers/environment.rb +++ b/lib/aptible/cli/helpers/environment.rb @@ -37,11 +37,17 @@ def environment_from_handle(handle) def ensure_default_environment environments = Aptible::Api::Account.all(token: fetch_token) - return environments.first if environments.count == 1 - - raise Thor::Error, <<-ERR.gsub(/\s+/, ' ').strip - Multiple environments available, please specify with --environment - ERR + case environments.count + when 0 + e = 'No environments. Go to https://app.aptible.com/ to proceed' + raise Thor::Error, e + when 1 + return environments.first + else + raise Thor::Error, <<-ERR.gsub(/\s+/, ' ').strip + Multiple environments available, please specify with --environment or --env + ERR + end end end end diff --git a/lib/aptible/cli/resource_formatter.rb b/lib/aptible/cli/resource_formatter.rb index e3fe2833..b5c290c9 100644 --- a/lib/aptible/cli/resource_formatter.rb +++ b/lib/aptible/cli/resource_formatter.rb @@ -157,6 +157,7 @@ def inject_service(node, service, app) node.value('command', service.command || 'CMD') node.value('container_count', service.container_count) node.value('container_size', service.container_memory_limit_mb) + node.value('container_profile', service.instance_class.to_s[/[a-z]/]) attach_app(node, app) end diff --git a/lib/aptible/cli/subcommands/apps.rb b/lib/aptible/cli/subcommands/apps.rb index 4cf1ee5d..b8ce5f38 100644 --- a/lib/aptible/cli/subcommands/apps.rb +++ b/lib/aptible/cli/subcommands/apps.rb @@ -9,7 +9,7 @@ def self.included(thor) include Helpers::Token desc 'apps', 'List all applications' - option :environment + option :environment, aliases: '--env' def apps Formatter.render(Renderer.current) do |root| root.grouped_keyed_list( @@ -28,7 +28,7 @@ def apps end desc 'apps:create HANDLE', 'Create a new application' - option :environment + option :environment, aliases: '--env' define_method 'apps:create' do |handle| environment = ensure_environment(options) app = environment.create_app(handle: handle) @@ -69,9 +69,11 @@ def apps raise Thor::Error, m end - if container_count.nil? && container_size.nil? - raise Thor::Error, - 'Provide at least --container-count or --container-size' + if container_count.nil? && container_size.nil? && + container_profile.nil? + m = 'Provide at least --container-count, --container-size, ' \ + 'or --container-profile' + raise Thor::Error, m end # We don't validate any parameters here: API will do that for us. @@ -104,7 +106,7 @@ def apps ' ENVIRONMENT_HANDLE]', 'Rename an app handle. In order'\ ' for the new app handle to appear in log drain and metric'\ ' drain destinations, you must restart the app.' - option :environment + option :environment, aliases: '--env' define_method 'apps:rename' do |old_handle, new_handle| env = ensure_environment(options) app = ensure_app(options.merge(app: old_handle)) diff --git a/lib/aptible/cli/subcommands/backup.rb b/lib/aptible/cli/subcommands/backup.rb index 23c39c64..6762edd2 100644 --- a/lib/aptible/cli/subcommands/backup.rb +++ b/lib/aptible/cli/subcommands/backup.rb @@ -14,7 +14,8 @@ def self.included(thor) '[--key-arn KEY_ARN]', 'Restore a backup' option :handle, desc: 'a name to use for the new database' - option :environment, desc: 'a different environment to restore to' + option :environment, aliases: '--env', + desc: 'a different environment to restore to' option :container_size, type: :numeric option :size, type: :numeric option :disk_size, type: :numeric @@ -68,9 +69,9 @@ def self.included(thor) end desc 'backup:list DB_HANDLE', 'List backups for a database' - option :environment + option :environment, aliases: '--env' option :max_age, - default: '1mo', + default: '99y', desc: 'Limit backups returned (example usage: 1w, 1y, etc.)' define_method 'backup:list' do |handle| age = ChronicDuration.parse(options[:max_age]) @@ -95,8 +96,8 @@ def self.included(thor) desc 'backup:orphaned', 'List backups associated with ' \ 'deprovisioned databases' - option :environment - option :max_age, default: '1y', + option :environment, aliases: '--env' + option :max_age, default: '99y', desc: 'Limit backups returned '\ '(example usage: 1w, 1y, etc.)' define_method 'backup:orphaned' do diff --git a/lib/aptible/cli/subcommands/config.rb b/lib/aptible/cli/subcommands/config.rb index f1b1bae9..cc0f5d93 100644 --- a/lib/aptible/cli/subcommands/config.rb +++ b/lib/aptible/cli/subcommands/config.rb @@ -1,5 +1,4 @@ require 'shellwords' - module Aptible module CLI module Subcommands @@ -72,6 +71,7 @@ def config # FIXME: define_method - ?! Seriously, WTF Thor. app = ensure_app(options) env = Hash[args.map do |arg| + arg = arg.split('=')[0] validate_env_key!(arg) [arg, ''] end] diff --git a/lib/aptible/cli/subcommands/db.rb b/lib/aptible/cli/subcommands/db.rb index 95a94bc5..764c653c 100644 --- a/lib/aptible/cli/subcommands/db.rb +++ b/lib/aptible/cli/subcommands/db.rb @@ -14,7 +14,7 @@ def self.included(thor) include Term::ANSIColor desc 'db:list', 'List all databases' - option :environment + option :environment, aliases: '--env' define_method 'db:list' do Formatter.render(Renderer.current) do |root| root.grouped_keyed_list( @@ -63,7 +63,7 @@ def self.included(thor) option :disk_size, default: 10, type: :numeric option :size, type: :numeric option :key_arn, type: :string - option :environment + option :environment, aliases: '--env' option :container_profile, type: :string, desc: 'Examples: m c r' option :iops, type: :numeric @@ -126,7 +126,7 @@ def self.included(thor) end desc 'db:clone SOURCE DEST', 'Clone a database to create a new one' - option :environment + option :environment, aliases: '--env' define_method 'db:clone' do |source_handle, dest_handle| # TODO: Deprecate + recommend backup source = ensure_database(options.merge(db: source_handle)) @@ -139,7 +139,7 @@ def self.included(thor) '[--container-profile PROFILE] [--iops IOPS] ' \ '[--logical --version VERSION] [--key-arn KEY_ARN]', 'Create a replica/follower of a database' - option :environment + option :environment, aliases: '--env' option :container_size, type: :numeric option :size, type: :numeric option :disk_size, type: :numeric @@ -191,7 +191,7 @@ def self.included(thor) desc 'db:dump HANDLE [pg_dump options]', 'Dump a remote database to file' - option :environment + option :environment, aliases: '--env' define_method 'db:dump' do |handle, *dump_options| database = ensure_database(options.merge(db: handle)) with_postgres_tunnel(database) do |url| @@ -204,7 +204,7 @@ def self.included(thor) desc 'db:execute HANDLE SQL_FILE [--on-error-stop]', 'Executes sql against a database' - option :environment + option :environment, aliases: '--env' option :on_error_stop, type: :boolean define_method 'db:execute' do |handle, sql_path| database = ensure_database(options.merge(db: handle)) @@ -217,7 +217,7 @@ def self.included(thor) end desc 'db:tunnel HANDLE', 'Create a local tunnel to a database' - option :environment + option :environment, aliases: '--env' option :port, type: :numeric option :type, type: :string define_method 'db:tunnel' do |handle| @@ -262,7 +262,7 @@ def self.included(thor) end desc 'db:deprovision HANDLE', 'Deprovision a database' - option :environment + option :environment, aliases: '--env' define_method 'db:deprovision' do |handle| database = ensure_database(options.merge(db: handle)) CLI.logger.info "Deprovisioning #{database.handle}..." @@ -278,7 +278,7 @@ def self.included(thor) end desc 'db:backup HANDLE', 'Backup a database' - option :environment + option :environment, aliases: '--env' define_method 'db:backup' do |handle| database = ensure_database(options.merge(db: handle)) CLI.logger.info "Backing up #{database.handle}..." @@ -287,7 +287,7 @@ def self.included(thor) end desc 'db:reload HANDLE', 'Reload a database' - option :environment + option :environment, aliases: '--env' define_method 'db:reload' do |handle| database = ensure_database(options.merge(db: handle)) CLI.logger.info "Reloading #{database.handle}..." @@ -300,7 +300,7 @@ def self.included(thor) '[--container-profile PROFILE] [--iops IOPS] ' \ '[--volume-type [gp2, gp3]]', 'Restart a database' - option :environment + option :environment, aliases: '--env' option :container_size, type: :numeric option :container_profile, type: :string, desc: 'Examples: m c r' @@ -335,7 +335,7 @@ def self.included(thor) desc 'db:modify HANDLE ' \ '[--iops IOPS] [--volume-type [gp2, gp3]]', 'Modify a database disk' - option :environment + option :environment, aliases: '--env' option :iops, type: :numeric option :volume_type define_method 'db:modify' do |handle| @@ -354,7 +354,7 @@ def self.included(thor) end desc 'db:url HANDLE', 'Display a database URL' - option :environment + option :environment, aliases: '--env' option :type, type: :string define_method 'db:url' do |handle| database = ensure_database(options.merge(db: handle)) @@ -371,7 +371,7 @@ def self.included(thor) ' ENVIRONMENT_HANDLE]', 'Rename a database handle. In order'\ ' for the new database handle to appear in log drain and'\ ' metric drain destinations, you must reload the database.' - option :environment + option :environment, aliases: '--env' define_method 'db:rename' do |old_handle, new_handle| env = ensure_environment(options) db = ensure_database(options.merge(db: old_handle)) diff --git a/lib/aptible/cli/subcommands/environment.rb b/lib/aptible/cli/subcommands/environment.rb index 9cdc4076..280dfec8 100644 --- a/lib/aptible/cli/subcommands/environment.rb +++ b/lib/aptible/cli/subcommands/environment.rb @@ -8,7 +8,7 @@ def self.included(thor) include Helpers::Token desc 'environment:list', 'List all environments' - option :environment + option :environment, aliases: '--env' define_method 'environment:list' do Formatter.render(Renderer.current) do |root| root.keyed_list( @@ -25,7 +25,7 @@ def self.included(thor) desc 'environment:ca_cert', 'Retrieve the CA certificate associated with the environment' - option :environment + option :environment, aliases: '--env' define_method 'environment:ca_cert' do Formatter.render(Renderer.current) do |root| root.grouped_keyed_list( diff --git a/lib/aptible/cli/subcommands/log_drain.rb b/lib/aptible/cli/subcommands/log_drain.rb index 952cec2a..bf2ce060 100644 --- a/lib/aptible/cli/subcommands/log_drain.rb +++ b/lib/aptible/cli/subcommands/log_drain.rb @@ -19,11 +19,11 @@ def self.drain_options option :drain_databases, default: true, type: :boolean option :drain_ephemeral_sessions, default: true, type: :boolean option :drain_proxies, default: true, type: :boolean - option :environment + option :environment, aliases: '--env' end desc 'log_drain:list', 'List all Log Drains' - option :environment + option :environment, aliases: '--env' define_method 'log_drain:list' do Formatter.render(Renderer.current) do |root| root.grouped_keyed_list( @@ -137,7 +137,7 @@ def self.drain_options desc 'log_drain:deprovision HANDLE --environment ENVIRONMENT', 'Deprovisions a log drain' - option :environment + option :environment, aliases: '--env' define_method 'log_drain:deprovision' do |handle| account = ensure_environment(options) drain = ensure_log_drain(account, handle) diff --git a/lib/aptible/cli/subcommands/maintenance.rb b/lib/aptible/cli/subcommands/maintenance.rb index 9cf9a165..aad21511 100644 --- a/lib/aptible/cli/subcommands/maintenance.rb +++ b/lib/aptible/cli/subcommands/maintenance.rb @@ -11,7 +11,7 @@ def self.included(thor) desc 'maintenance:apps', 'List Apps impacted by maintenance schedules where '\ 'restarts are required' - option :environment + option :environment, aliases: '--env' define_method 'maintenance:apps' do found_maintenance = false m = maintenance_apps @@ -46,7 +46,7 @@ def self.included(thor) desc 'maintenance:dbs', 'List Databases impacted by maintenance schedules where '\ 'restarts are required' - option :environment + option :environment, aliases: '--env' define_method 'maintenance:dbs' do found_maintenance = false m = maintenance_databases diff --git a/lib/aptible/cli/subcommands/metric_drain.rb b/lib/aptible/cli/subcommands/metric_drain.rb index f8a8653b..9e2a82d8 100644 --- a/lib/aptible/cli/subcommands/metric_drain.rb +++ b/lib/aptible/cli/subcommands/metric_drain.rb @@ -18,7 +18,7 @@ def self.included(thor) include Helpers::MetricDrain desc 'metric_drain:list', 'List all Metric Drains' - option :environment + option :environment, aliases: '--env' define_method 'metric_drain:list' do Formatter.render(Renderer.current) do |root| root.grouped_keyed_list( @@ -40,7 +40,7 @@ def self.included(thor) '--db DATABASE_HANDLE --environment ENVIRONMENT', 'Create an InfluxDB Metric Drain' option :db, type: :string - option :environment + option :environment, aliases: '--env' define_method 'metric_drain:create:influxdb' do |handle| account = ensure_environment(options) @@ -66,7 +66,7 @@ def self.included(thor) option :password, type: :string option :url, type: :string option :db, type: :string - option :environment + option :environment, aliases: '--env' define_method 'metric_drain:create:influxdb:custom' do |handle| account = ensure_environment(options) @@ -95,7 +95,7 @@ def self.included(thor) option :org, type: :string option :token, type: :string option :url, type: :string - option :environment + option :environment, aliases: '--env' define_method 'metric_drain:create:influxdb:customv2' do |handle| account = ensure_environment(options) @@ -121,7 +121,7 @@ def self.included(thor) 'Create a Datadog Metric Drain' option :api_key, type: :string option :site, type: :string - option :environment + option :environment, aliases: '--env' define_method 'metric_drain:create:datadog' do |handle| account = ensure_environment(options) @@ -150,7 +150,7 @@ def self.included(thor) desc 'metric_drain:deprovision HANDLE --environment ENVIRONMENT', 'Deprovisions a Metric Drain' - option :environment + option :environment, aliases: '--env' define_method 'metric_drain:deprovision' do |handle| account = ensure_environment(options) drain = ensure_metric_drain(account, handle) diff --git a/lib/aptible/cli/version.rb b/lib/aptible/cli/version.rb index b095620a..2f50f554 100644 --- a/lib/aptible/cli/version.rb +++ b/lib/aptible/cli/version.rb @@ -1,5 +1,5 @@ module Aptible module CLI - VERSION = '0.24.0'.freeze + VERSION = '0.24.1'.freeze end end diff --git a/spec/aptible/cli/subcommands/apps_spec.rb b/spec/aptible/cli/subcommands/apps_spec.rb index 937b7c7e..612ef46a 100644 --- a/spec/aptible/cli/subcommands/apps_spec.rb +++ b/spec/aptible/cli/subcommands/apps_spec.rb @@ -93,11 +93,13 @@ def explain s1 = Fabricate( :service, - app: app, process_type: 's1', command: 'true', container_count: 2 + app: app, process_type: 's1', command: 'true', container_count: 2, + instance_class: 'm5' ) s2 = Fabricate( :service, - app: app, process_type: 's2', container_memory_limit_mb: 2048 + app: app, process_type: 's2', container_memory_limit_mb: 2048, + instance_class: 'r5' ) expected_json = [ @@ -118,6 +120,7 @@ def explain 'id' => s1.id, 'command' => s1.command, 'container_count' => s1.container_count, + 'container_profile' => 'm', 'container_size' => s1.container_memory_limit_mb, 'created_at' => fmt_time(s1.created_at) }, @@ -126,6 +129,7 @@ def explain 'id' => s2.id, 'command' => 'CMD', 'container_count' => s2.container_count, + 'container_profile' => 'r', 'container_size' => s2.container_memory_limit_mb, 'created_at' => fmt_time(s2.created_at) } diff --git a/spec/aptible/cli/subcommands/config_spec.rb b/spec/aptible/cli/subcommands/config_spec.rb index 2af99a86..1b1bc315 100644 --- a/spec/aptible/cli/subcommands/config_spec.rb +++ b/spec/aptible/cli/subcommands/config_spec.rb @@ -109,6 +109,17 @@ subject.send('config:unset', 'FOO') end + it 'unsets environment variables even if the user passes a value' do + expect(app).to receive(:create_operation!) + .with(type: 'configure', env: { 'FOO' => '' }) + .and_return(operation) + + expect(subject).to receive(:attach_to_operation_logs) + .with(operation) + + subject.send('config:unset', 'FOO=whoops') + end + it 'rejects environment variables that start with -' do expect { subject.send('config:rm', '-foo') } .to raise_error(/invalid argument/im) diff --git a/spec/aptible/cli/subcommands/services_spec.rb b/spec/aptible/cli/subcommands/services_spec.rb index 026adfc7..d1bbd5e3 100644 --- a/spec/aptible/cli/subcommands/services_spec.rb +++ b/spec/aptible/cli/subcommands/services_spec.rb @@ -46,6 +46,14 @@ expect(captured_output_text.split("\n")).to include('Container Count: 3') end + it 'lists container profile' do + Fabricate(:service, app: app, instance_class: 'u7') + subject.send('services') + + expect(captured_output_text.split("\n")) + .to include('Container Profile: u') + end + it 'lists multiple services' do Fabricate(:service, app: app, process_type: 'foo') Fabricate(:service, app: app, process_type: 'bar')