Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AKS] az aks command invoke: Add support for --no-wait #22813

Merged
merged 5 commits into from
Jun 13, 2022

Conversation

FumingZhang
Copy link
Member

Related command

  • az aks command invoke
  • az aks command result

Description

  • Add support for option --no-wait in az aks command invoke
  • Output command id in az aks command invoke
  • Handle empty command result

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@ghost ghost requested review from zhoxing-ms and wangzelin007 June 9, 2022 14:18
@ghost ghost assigned zhoxing-ms Jun 9, 2022
@ghost ghost added this to the Jun 2022 (2022-07-05) milestone Jun 9, 2022
@ghost ghost added the Auto-Assign Auto assign by bot label Jun 9, 2022
@ghost ghost requested a review from yonzhan June 9, 2022 14:19
@ghost ghost added the AKS az aks/acs/openshift label Jun 9, 2022
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 9, 2022

AKS

command_id_regex = re.compile(r'commandResults\/(\w*)\?')
command_id = command_id_regex.findall(command_result_polling_url)[0]
logger.warning("command id: %s", command_id)
return _print_command_result(cmd.cli_ctx, command_result_poller.result(300))
Copy link
Member

Choose a reason for hiding this comment

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

can we condition this on no_wait parameter?

if no_wait==true
print command id
instruct how to fetch result with "az aks command result"
else
...

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, a sample screenshot
image

f"with exitcode={commandResult.exit_code}{colorama.Style.RESET_ALL}")
print(commandResult.logs)
return
if commandResult:
Copy link
Member

Choose a reason for hiding this comment

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

let's drop condition here.

Copy link
Member Author

@FumingZhang FumingZhang Jun 10, 2022

Choose a reason for hiding this comment

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

For the situation where the command is running, the HTTP status code of the response is 202, at this time the commandResult will be parsed as None (see SDK), resulting in the following exception

image

Comment on lines +2747 to +2748
print(f"{colorama.Fore.BLUE}command is in {commandResult.provisioning_state} state{colorama.Style.RESET_ALL}")
return
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed a misleading colon (:).

Copy link
Member Author

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

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

For those commands executed without the additional --no-wait option, the results are the same as before

  • command result with short execution time
    Screenshot 2022-06-10 155423
  • command result with long execution time (>300s)
    Screenshot 2022-06-10 161038

For commands with the --no-wait option, the results are as follows

  • command result with short execution time
    Screenshot 2022-06-10 155532
  • command result with long execution time (>300s)
    Screenshot 2022-06-10 155939
    • after the command completes
      Screenshot 2022-06-10 161110

Comment on lines 2703 to 2718
def _aks_command_result_in_progess_helper(client, resource_group_name, name, command_id):
# pylint: disable=unused-argument
def command_result_direct_response_handler(pipeline_response, *args, **kwargs):
deserialized_data = pipeline_response.context.get("deserialized_data", {})
if deserialized_data:
provisioning_state = deserialized_data.get("properties", {}).get("provisioningState", None)
started_at = deserialized_data.get("properties", {}).get("startedAt", None)
print(f"command id: {command_id}, started at: {started_at}, status: {provisioning_state}")
print(
f"Please use command \"az aks command result -g {resource_group_name} -n {name} -i {command_id}\" "
"to get the future execution result"
)
else:
print(f"failed to fetch command result for command id: {command_id}")
client.get_command_result(resource_group_name, name, command_id, cls=command_result_direct_response_handler)
return
Copy link
Member Author

Choose a reason for hiding this comment

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

Since it is declared in swagger that the 202 HTTP status code is returned for the command in running state and no response parsing is performed, the response is manually parsed here in a relatively ugly way.

Comment on lines +2697 to +2699
if commandResult is None:
_aks_command_result_in_progess_helper(client, resource_group_name, name, command_id)
return
Copy link
Member Author

Choose a reason for hiding this comment

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

The SDK will not parse the response result of the query corresponding to the command that is still in running state.

Comment on lines +2682 to +2686
command_result_polling_url = command_result_poller.polling_method()._initial_response.http_response.headers[
"location"
]
command_id_regex = re.compile(r"commandResults\/(\w*)\?")
command_id = command_id_regex.findall(command_result_polling_url)[0]
Copy link
Member Author

Choose a reason for hiding this comment

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

The initial response content of the request is empty. The API path to query command result can only be obtained through the location field in the header of the response.

@FumingZhang FumingZhang changed the title {AKS} Update aks command commands [AKS] az aks command invoke: Add support for --no-wait Jun 13, 2022
@FumingZhang FumingZhang marked this pull request as ready for review June 13, 2022 06:30
@zhoxing-ms zhoxing-ms merged commit e348ddf into Azure:dev Jun 13, 2022
@FumingZhang FumingZhang deleted the fuming/aks-run-0609 branch June 13, 2022 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AKS az aks/acs/openshift Auto-Assign Auto assign by bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants