Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

https://github.com/twosigma/waiter/issues/1649 + https://github.com/twosigma/waiter/issues/1637 #1672

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions cli/integration/tests/waiter/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,7 @@ def test_show_services_using_token(self):
self.assertEqual(service_description_2, service_2['service-description'])
self.assertEqual('Running', service_1['status'])
self.assertIn(service_2['status'], ['Failing', 'Starting'])
self.assertIn(service_2['scaling-state'], ["stable", "scale-up", "scale-down"])
Copy link
Contributor

@shamsimam shamsimam Oct 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's capitalize the first letter in the output

Actually, let it be. We can fix the string in the api if needed.


# Run show without --json
cp = cli.show(self.waiter_url, token_name)
Expand All @@ -1163,8 +1164,10 @@ def test_show_services_using_token(self):
self.assertIsNotNone(re.search('^# Instances\\s+([12])$', cli.stdout(cp), re.MULTILINE))
self.assertIsNotNone(re.search('^Total Memory\\s+(128|384) MiB$', cli.stdout(cp), re.MULTILINE))
self.assertIsNotNone(re.search('^Total CPUs\\s+0\\.([13])$', cli.stdout(cp), re.MULTILINE))
self.assertIsNotNone(re.search(f'^{service_id_1}.+Running.+Not Current$', cli.stdout(cp), re.MULTILINE))
self.assertIsNotNone(re.search(f'^{service_id_2}.+(Failing|Starting).+Current$',
self.assertIsNotNone(re.search(f'^{service_id_1}.+Running.+Not Current.', cli.stdout(cp), re.MULTILINE))
self.assertIsNotNone(re.search(f'^{service_id_2}.+(Failing|Starting).+Current.',
cli.stdout(cp), re.MULTILINE))
self.assertIsNotNone(re.search(f'^{service_id_2}.+(stable|scale-up|scale-down)$',
cli.stdout(cp), re.MULTILINE))

# Run show without --json and with --no-services
Expand Down Expand Up @@ -2268,6 +2271,10 @@ def test_ssh_token_custom_container(self):
self.__test_ssh(lambda _, instances: instances['active-instances'], admin=True,
container_name='waiter-files')

def test_ssh_token_custom_container_all_users(self):
self.__test_ssh(lambda _, instances: instances['active-instances'], admin=False,
container_name='waiter-files')

def test_ssh_token_invalid_token(self):
token_name = "nonexistent"
cp = cli.ssh(self.waiter_url, token_name)
Expand Down
3 changes: 2 additions & 1 deletion cli/waiter/display.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ def tabulate_token_services(services, token_name, token_etag=None, show_index=Fa
('Last request', format_last_request_time(s)),
('Current?',
format_using_current_token(s, token_etag or s.get('etag', None),
token_name))]
token_name)),
('Scaling State', s['scaling-state'])]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code should be defensive against the value missing and display blank or Unknown. As written the code will throw an error if the scaling-state entry is missing.

if key in column_names or show_index and key == 'Index'])
for index, s in enumerate(services)]
service_table = tabulate(rows, headers='keys', tablefmt='plain')
Expand Down
2 changes: 1 addition & 1 deletion cli/waiter/subcommands/show.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def tabulate_token(cluster_or_cluster_group_name, token, token_name, services, t
last_update_time = format_timestamp_string(token['last-update-time'])
last_update_user = f' ({token["last-update-user"]})' if 'last-update-user' in token else ''
column_names = ['Service Id', 'Cluster', 'Run as user', 'Instances', 'CPUs', 'Memory', 'Version', 'Status', 'Last request',
'Current?']
'Current?', 'Scaling State']
service_table, _ = tabulate_token_services(services, token_name, token_etag=token_etag, column_names=column_names)
return f'\n' \
f'=== {terminal.bold(cluster_or_cluster_group_name)} / {terminal.bold(token_name)} ===\n' \
Expand Down
9 changes: 4 additions & 5 deletions cli/waiter/subcommands/ssh.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from waiter.display import get_user_selection, tabulate_service_instances, tabulate_token_services
from waiter.querying import get_service_id_from_instance_id, get_target_cluster_from_token, print_no_data, \
print_no_services, query_service, query_token, get_services_on_cluster, print_no_instances
from waiter.util import guard_no_cluster, is_admin_enabled, print_info
from waiter.util import guard_no_cluster, print_info

BASH_PATH = '/bin/bash'

Expand Down Expand Up @@ -177,10 +177,9 @@ def register(add_parser):
description='ssh to an instance given the token, service id, or instance id. Working directory '
'will be the log directory.')
parser.add_argument('token-or-service-id-or-instance-id')
if is_admin_enabled():
parser.add_argument('--container-name', '-c',
help='specify the container name you want to ssh into. Defaults to "waiter-app". Has no '
'effect if instance is not k8s pod.')
parser.add_argument('--container-name', '-c',
help='specify the container name you want to ssh into. Defaults to "waiter-app". Has no '
'effect if instance is not k8s pod.')
id_group = parser.add_mutually_exclusive_group(required=False)
id_group.add_argument('--token', '-t', dest='ssh_destination', action='store_const', const=Destination.TOKEN,
default=Destination.TOKEN, help='Default; ssh with token')
Expand Down