From 03ab91b8a125f6fc5e89d1775714459a595f1422 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Wed, 10 Dec 2025 11:15:51 -0500 Subject: [PATCH 01/17] Extract ECS monitoring data collection to ServiceViewCollector --- .../ecs/monitorexpressgatewayservice.py | 573 +----------- .../ecs/serviceviewcollector.py | 538 ++++++++++++ .../ecs/test_monitorexpressgatewayservice.py | 611 ------------- .../ecs/test_serviceviewcollector.py | 830 ++++++++++++++++++ 4 files changed, 1379 insertions(+), 1173 deletions(-) create mode 100644 awscli/customizations/ecs/serviceviewcollector.py create mode 100644 tests/unit/customizations/ecs/test_serviceviewcollector.py diff --git a/awscli/customizations/ecs/monitorexpressgatewayservice.py b/awscli/customizations/ecs/monitorexpressgatewayservice.py index 3004fe69ecc9..e946c010dce3 100644 --- a/awscli/customizations/ecs/monitorexpressgatewayservice.py +++ b/awscli/customizations/ecs/monitorexpressgatewayservice.py @@ -18,7 +18,10 @@ allowing users to track resource creation progress, deployment status, and service health through an interactive command-line interface with live updates and visual indicators. -The module implements two primary monitoring modes: +The data collection logic is handled by ServiceViewCollector, which parses AWS resources and +formats monitoring output. This module focuses on display and user interaction. + +The module implements two resource view modes: - RESOURCE: Displays all resources associated with the service - DEPLOYMENT: Shows resources that have changed in the most recent deployment @@ -40,23 +43,15 @@ import sys import threading import time -from functools import reduce from botocore.exceptions import ClientError from awscli.customizations.commands import BasicCommand from awscli.customizations.ecs.exceptions import MonitoringError -from awscli.customizations.ecs.expressgateway.managedresource import ( - ManagedResource, -) -from awscli.customizations.ecs.expressgateway.managedresourcegroup import ( - ManagedResourceGroup, -) from awscli.customizations.ecs.prompt_toolkit_display import Display +from awscli.customizations.ecs.serviceviewcollector import ServiceViewCollector from awscli.customizations.utils import uni_print -TIMESTAMP_FORMAT = "%Y-%m-%dT%H:%M:%SZ" - class ECSMonitorExpressGatewayService(BasicCommand): """AWS CLI command for monitoring ECS Express Gateway Service deployments. @@ -192,17 +187,18 @@ def __init__( timeout_minutes=30, display=None, use_color=True, + collector=None, ): self._client = client self.service_arn = service_arn self.mode = mode self.timeout_minutes = timeout_minutes - self.last_described_gateway_service_response = None - self.last_execution_time = 0 - self.cached_monitor_result = None self.start_time = time.time() self.use_color = use_color self.display = display or Display() + self.collector = collector or ServiceViewCollector( + client, service_arn, mode, use_color + ) @staticmethod def is_monitoring_available(): @@ -213,9 +209,7 @@ def exec(self): """Start monitoring the express gateway service with progress display.""" def monitor_service(spinner_char): - return self._monitor_express_gateway_service( - spinner_char, self.service_arn, self.mode - ) + return self.collector.get_current_view(spinner_char) asyncio.run(self._execute_with_progress_async(monitor_service, 100)) @@ -226,12 +220,7 @@ async def _execute_with_progress_async( spinner_chars = "⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏" spinner_index = 0 - # Initialize with basic service resource - service_resource = ManagedResource("Service", self.service_arn) - initial_output = service_resource.get_status_string( - spinner_char="{SPINNER}", use_color=self.use_color - ) - current_output = initial_output + current_output = "Waiting for initial data" async def update_data(): nonlocal current_output @@ -287,543 +276,3 @@ async def update_spinner(): spinner_task.cancel() final_output = current_output.replace("{SPINNER}", "") uni_print(final_output + "\nMonitoring Complete!\n") - - def _monitor_express_gateway_service( - self, spinner_char, service_arn, mode, execution_refresh_millis=5000 - ): - """Monitor service status and return formatted output. - - Args: - spinner_char (char): Character to print representing progress (unused with single spinner) - execution_refresh_millis (int): Refresh interval in milliseconds - service_arn (str): Service ARN to monitor - mode (str): Monitoring mode ('RESOURCE' or 'DEPLOYMENT') - - Returns: - str: Formatted status output - """ - current_time = time.time() - - if ( - current_time - self.last_execution_time - >= execution_refresh_millis / 1000.0 - ): - try: - describe_gateway_service_response = ( - self._client.describe_express_gateway_service( - serviceArn=service_arn - ) - ) - if not describe_gateway_service_response: - self.cached_monitor_result = ( - None, - "Trying to describe gateway service", - ) - elif ( - not ( - service := describe_gateway_service_response.get( - "service" - ) - ) - or not service.get("serviceArn") - or not service.get("activeConfigurations") - ): - self.cached_monitor_result = ( - None, - "Trying to describe gateway service", - ) - else: - self.last_described_gateway_service_response = ( - describe_gateway_service_response - ) - described_gateway_service = ( - describe_gateway_service_response.get("service") - ) - - if mode == "DEPLOYMENT": - managed_resources, info = self._diff_service_view( - described_gateway_service - ) - else: - managed_resources, info = self._combined_service_view( - described_gateway_service - ) - - service_resources = [ - self._parse_cluster(described_gateway_service), - self._parse_service(described_gateway_service), - ] - if managed_resources: - service_resources.append(managed_resources) - service_resource = ManagedResourceGroup( - resources=service_resources - ) - self._update_cached_monitor_results(service_resource, info) - except ClientError as e: - if ( - e.response.get('Error', {}).get('Code') - == 'InvalidParameterException' - ): - error_message = e.response.get('Error', {}).get( - 'Message', '' - ) - if ( - "Cannot call DescribeServiceRevisions for a service that is INACTIVE" - in error_message - ): - empty_resource_group = ManagedResourceGroup() - self._update_cached_monitor_results( - empty_resource_group, "Service is inactive" - ) - else: - raise - else: - raise - - self.last_execution_time = current_time - - if not self.cached_monitor_result: - return "Waiting for initial data" - else: - # Generate the output every iteration. This allow the underlying resources to utilize spinners - service_resource, info = self.cached_monitor_result - status_string = ( - service_resource.get_status_string( - spinner_char=spinner_char, use_color=self.use_color - ) - if service_resource - else None - ) - - output = "\n".join([x for x in [status_string, info] if x]) - return output - - def _diff_service_view(self, describe_gateway_service_response): - """Generate diff view showing changes in the latest deployment. - - Computes differences between source and target service revisions to show - what resources are being updated or disassociated in the current deployment. - - Args: - describe_gateway_service_response (dict): Service description from API - - Returns: - tuple: (resources, info_output) where: - - resources (ManagedResourceGroup): Diff view of resources - - info_output (str): Informational messages - """ - service_arn = describe_gateway_service_response.get("serviceArn") - list_service_deployments_response = ( - self._client.list_service_deployments( - service=service_arn, maxResults=1 - ) - ) - listed_service_deployments = self._validate_and_parse_response( - list_service_deployments_response, - "ListServiceDeployments", - expected_field="serviceDeployments", - ) - if ( - not listed_service_deployments - or "serviceDeploymentArn" not in listed_service_deployments[0] - ): - return ( - None, - "Waiting for a deployment to start", - ) - - deployment_arn = listed_service_deployments[0].get( - "serviceDeploymentArn" - ) - - describe_service_deployments_response = ( - self._client.describe_service_deployments( - serviceDeploymentArns=[deployment_arn] - ) - ) - described_service_deployments = self._validate_and_parse_response( - describe_service_deployments_response, - "DescribeServiceDeployments", - expected_field="serviceDeployments", - eventually_consistent=True, - ) - described_service_deployment = described_service_deployments[0] - if ( - not described_service_deployment - or not described_service_deployment.get("targetServiceRevision") - ): - return ( - None, - "Waiting for a deployment to start", - ) - - target_sr = described_service_deployment.get( - "targetServiceRevision" - ).get("arn") - - target_sr_resources_list, described_target_sr_list = ( - self._describe_and_parse_service_revisions([target_sr]) - ) - if len(target_sr_resources_list) != 1: - return (None, "Trying to describe service revisions") - target_sr_resources = target_sr_resources_list[0] - described_target_sr = described_target_sr_list[0] - - task_def_arn = described_target_sr.get("taskDefinition") - if "sourceServiceRevisions" in described_service_deployment: - source_sr_resources, _ = ( - self._describe_and_parse_service_revisions( - [ - sr.get("arn") - for sr in described_service_deployment.get( - "sourceServiceRevisions" - ) - ] - ) - ) - if len(source_sr_resources) != len( - described_service_deployment.get("sourceServiceRevisions") - ): - return (None, "Trying to describe service revisions)") - source_sr_resources_combined = reduce( - lambda x, y: x.combine(y), source_sr_resources - ) - else: - source_sr_resources_combined = ManagedResourceGroup() - - updating_resources, disassociating_resources = ( - target_sr_resources.diff(source_sr_resources_combined) - ) - updating_resources.resource_type = "Updating" - disassociating_resources.resource_type = "Disassociating" - service_resources = ManagedResourceGroup( - resource_type="Deployment", - identifier=deployment_arn, - status=described_service_deployment.get("status"), - reason=described_service_deployment.get("statusReason"), - resources=[ - ManagedResource( - resource_type="TargetServiceRevision", identifier=target_sr - ), - ManagedResource( - resource_type="TaskDefinition", identifier=task_def_arn - ), - updating_resources, - disassociating_resources, - ], - ) - return service_resources, None - - def _combined_service_view(self, describe_gateway_service_response): - """Generate combined view of all active service resources. - - Extracts and combines resources from all active service configurations, - resolving conflicts by taking the version with the latest timestamp. - - Args: - describe_gateway_service_response (dict): Service description from API - - Returns: - tuple: (resources, info_output) where: - - resources (ManagedResourceGroup): Combined view of all resources - - info_output (str): Informational messages - """ - service_revision_arns = [ - config.get("serviceRevisionArn") - for config in describe_gateway_service_response.get( - "activeConfigurations" - ) - ] - service_revision_resources, _ = ( - self._describe_and_parse_service_revisions(service_revision_arns) - ) - - if len(service_revision_resources) != len(service_revision_arns): - return (None, "Trying to describe service revisions") - - service_resource = reduce( - lambda x, y: x.combine(y), service_revision_resources - ) - - return service_resource, None - - def _update_cached_monitor_results(self, resource, info): - """Update cached monitoring results with new data. - - Args: - resource: New resource data (replaces existing if provided) - info: New info message (always replaces existing) - """ - if not self.cached_monitor_result: - self.cached_monitor_result = (resource, info) - else: - self.cached_monitor_result = ( - resource or self.cached_monitor_result[0], - info, - ) - - def _validate_and_parse_response( - self, - response, - operation_name, - expected_field=None, - eventually_consistent=False, - ): - """Validate API response and extract expected field. - - Args: - response: API response to validate - operation_name: Name of the operation for error messages - expected_field: Field to extract from response (optional) - eventually_consistent: Whether to filter out MISSING failures - - Returns: - Extracted field value or None if no expected_field specified - - Raises: - MonitoringError: If response is invalid or missing required fields - """ - if not response: - raise MonitoringError(f"{operation_name} response is empty") - - self._parse_failures(response, operation_name, eventually_consistent) - - if not expected_field: - return None - - if response.get(expected_field) is None: - raise MonitoringError( - f"{operation_name} response is missing {expected_field}" - ) - return response.get(expected_field) - - def _parse_failures(self, response, operation_name, eventually_consistent): - """Parse and raise errors for API response failures. - - Args: - response: API response to check for failures - operation_name: Name of the operation for error messages - eventually_consistent: Whether to filter out MISSING failures for eventually consistent operations - - Raises: - MonitoringError: If failures are found in the response - """ - failures = response.get("failures") - - if not failures: - return - - if any(not f.get('arn') or not f.get('reason') for f in failures): - raise MonitoringError( - "Invalid failure response: missing arn or reason" - ) - - if eventually_consistent: - failures = [ - failure - for failure in failures - if failure.get("reason") != "MISSING" - ] - - if not failures: - return - - failure_msgs = [ - f"{f['arn']} failed with {f['reason']}" for f in failures - ] - joined_msgs = '\n'.join(failure_msgs) - raise MonitoringError(f"{operation_name}:\n{joined_msgs}") - - def _describe_and_parse_service_revisions(self, arns): - """Describe and parse service revisions into managed resources. - - Args: - arns (list): List of service revision ARNs to describe - - Returns: - tuple: (parsed_resources, described_revisions) where: - - parsed_resources (list): List of ManagedResourceGroup objects - - described_revisions (list): Raw API response data - """ - # API supports up to 20 arns, DescribeExpressGatewayService should never return more than 5 - describe_service_revisions_response = ( - self._client.describe_service_revisions(serviceRevisionArns=arns) - ) - described_service_revisions = self._validate_and_parse_response( - describe_service_revisions_response, - "DescribeServiceRevisions", - expected_field="serviceRevisions", - eventually_consistent=True, - ) - - return [ - self._parse_ecs_managed_resources(sr) - for sr in described_service_revisions - ], described_service_revisions - - def _parse_cluster(self, service): - return ManagedResource("Cluster", service.get("cluster")) - - def _parse_service(self, service): - service_arn = service.get("serviceArn") - cluster = service.get("cluster") - describe_service_response = self._client.describe_services( - cluster=cluster, services=[service_arn] - ) - described_service = self._validate_and_parse_response( - describe_service_response, "DescribeServices", "services" - )[0] - return ManagedResource( - "Service", - service.get("serviceArn"), - additional_info=described_service - and described_service.get("events")[0].get("message") - if described_service.get("events") - else None, - ) - - def _parse_ecs_managed_resources(self, service_revision): - managed_resources = service_revision.get("ecsManagedResources") - if not managed_resources: - return ManagedResourceGroup() - - parsed_resources = [] - if "ingressPaths" in managed_resources: - parsed_resources.append( - ManagedResourceGroup( - resource_type="IngressPaths", - resources=[ - self._parse_ingress_path_resources(ingress_path) - for ingress_path in managed_resources.get( - "ingressPaths" - ) - ], - ) - ) - if "autoScaling" in managed_resources: - parsed_resources.append( - self._parse_auto_scaling_configuration( - managed_resources.get("autoScaling") - ) - ) - if "metricAlarms" in managed_resources: - parsed_resources.append( - self._parse_metric_alarms( - managed_resources.get("metricAlarms") - ) - ) - if "serviceSecurityGroups" in managed_resources: - parsed_resources.append( - self._parse_service_security_groups( - managed_resources.get("serviceSecurityGroups") - ) - ) - if "logGroups" in managed_resources: - parsed_resources.append( - self._parse_log_groups(managed_resources.get("logGroups")) - ) - return ManagedResourceGroup(resources=parsed_resources) - - def _parse_ingress_path_resources(self, ingress_path): - resources = [] - if ingress_path.get("loadBalancer"): - resources.append( - self._parse_managed_resource( - ingress_path.get("loadBalancer"), "LoadBalancer" - ) - ) - if ingress_path.get("loadBalancerSecurityGroups"): - resources.extend( - self._parse_managed_resource_list( - ingress_path.get("loadBalancerSecurityGroups"), - "LoadBalancerSecurityGroup", - ) - ) - if ingress_path.get("certificate"): - resources.append( - self._parse_managed_resource( - ingress_path.get("certificate"), "Certificate" - ) - ) - if ingress_path.get("listener"): - resources.append( - self._parse_managed_resource( - ingress_path.get("listener"), "Listener" - ) - ) - if ingress_path.get("rule"): - resources.append( - self._parse_managed_resource(ingress_path.get("rule"), "Rule") - ) - if ingress_path.get("targetGroups"): - resources.extend( - self._parse_managed_resource_list( - ingress_path.get("targetGroups"), "TargetGroup" - ) - ) - return ManagedResourceGroup( - resource_type="IngressPath", - identifier=ingress_path.get("endpoint"), - resources=resources, - ) - - def _parse_auto_scaling_configuration(self, auto_scaling_configuration): - resources = [] - if auto_scaling_configuration.get("scalableTarget"): - resources.append( - self._parse_managed_resource( - auto_scaling_configuration.get("scalableTarget"), - "ScalableTarget", - ) - ) - if auto_scaling_configuration.get("applicationAutoScalingPolicies"): - resources.extend( - self._parse_managed_resource_list( - auto_scaling_configuration.get( - "applicationAutoScalingPolicies" - ), - "ApplicationAutoScalingPolicy", - ) - ) - return ManagedResourceGroup( - resource_type="AutoScalingConfiguration", resources=resources - ) - - def _parse_metric_alarms(self, metric_alarms): - return ManagedResourceGroup( - resource_type="MetricAlarms", - resources=self._parse_managed_resource_list( - metric_alarms, "MetricAlarm" - ), - ) - - def _parse_service_security_groups(self, service_security_groups): - return ManagedResourceGroup( - resource_type="ServiceSecurityGroups", - resources=self._parse_managed_resource_list( - service_security_groups, "SecurityGroup" - ), - ) - - def _parse_log_groups(self, logs_groups): - return ManagedResourceGroup( - resource_type="LogGroups", - resources=self._parse_managed_resource_list( - logs_groups, "LogGroup" - ), - ) - - def _parse_managed_resource(self, resource, resource_type): - return ManagedResource( - resource_type, - resource.get("arn"), - status=resource.get("status"), - updated_at=resource.get("updatedAt"), - reason=resource.get("statusReason"), - ) - - def _parse_managed_resource_list(self, data_list, resource_type): - return [ - self._parse_managed_resource(data, resource_type) - for data in data_list - ] diff --git a/awscli/customizations/ecs/serviceviewcollector.py b/awscli/customizations/ecs/serviceviewcollector.py new file mode 100644 index 000000000000..a9c28732d86f --- /dev/null +++ b/awscli/customizations/ecs/serviceviewcollector.py @@ -0,0 +1,538 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +"""Service view collector for ECS Express Gateway Service monitoring. + +This module provides business logic for collecting and formatting +ECS Express Gateway Service monitoring data. +""" + +import time +from functools import reduce + +from botocore.exceptions import ClientError + +from awscli.customizations.ecs.exceptions import MonitoringError +from awscli.customizations.ecs.expressgateway.managedresource import ( + ManagedResource, +) +from awscli.customizations.ecs.expressgateway.managedresourcegroup import ( + ManagedResourceGroup, +) + + +class ServiceViewCollector: + """Collects and formats ECS Express Gateway Service monitoring data. + + Responsible for: + - Making ECS API calls + - Parsing resource data + - Formatting output strings + - Caching responses + + Args: + client: ECS client for API calls + service_arn (str): ARN of the service to monitor + mode (str): Monitoring mode - 'RESOURCE' or 'DEPLOYMENT' + use_color (bool): Whether to use color in output + """ + + def __init__(self, client, service_arn, mode, use_color=True): + self._client = client + self.service_arn = service_arn + self.mode = mode + self.use_color = use_color + self.last_described_gateway_service_response = None + self.last_execution_time = 0 + self.cached_monitor_result = None + + def get_current_view( + self, spinner_char="{SPINNER}", execution_refresh_millis=5000 + ): + """Get current monitoring view as formatted string. + + Args: + spinner_char (str): Character for progress indication + execution_refresh_millis (int): Refresh interval in milliseconds + + Returns: + str: Formatted monitoring output + """ + current_time = time.time() + + if ( + current_time - self.last_execution_time + >= execution_refresh_millis / 1000.0 + ): + try: + describe_gateway_service_response = ( + self._client.describe_express_gateway_service( + serviceArn=self.service_arn + ) + ) + if not describe_gateway_service_response: + self.cached_monitor_result = ( + None, + "Trying to describe gateway service", + ) + elif ( + not ( + service := describe_gateway_service_response.get( + "service" + ) + ) + or not service.get("serviceArn") + or service.get("activeConfigurations") is None + ): + self.cached_monitor_result = ( + None, + "Trying to describe gateway service", + ) + else: + self.last_described_gateway_service_response = ( + describe_gateway_service_response + ) + described_gateway_service = ( + describe_gateway_service_response.get("service") + ) + + if self.mode == "DEPLOYMENT": + managed_resources, info = self._diff_service_view( + described_gateway_service + ) + else: + managed_resources, info = self._combined_service_view( + described_gateway_service + ) + + service_resources = [ + self._parse_cluster(described_gateway_service), + self._parse_service(described_gateway_service), + ] + if managed_resources: + service_resources.append(managed_resources) + service_resource = ManagedResourceGroup( + resources=service_resources + ) + self._update_cached_monitor_results(service_resource, info) + except ClientError as e: + if ( + e.response.get('Error', {}).get('Code') + == 'InvalidParameterException' + ): + error_message = e.response.get('Error', {}).get( + 'Message', '' + ) + if ( + "Cannot call DescribeServiceRevisions for a service that is INACTIVE" + in error_message + ): + empty_resource_group = ManagedResourceGroup() + self._update_cached_monitor_results( + empty_resource_group, "Service is inactive" + ) + else: + raise + else: + raise + + self.last_execution_time = current_time + + if not self.cached_monitor_result: + return "Waiting for initial data" + else: + service_resource, info = self.cached_monitor_result + status_string = ( + service_resource.get_status_string( + spinner_char=spinner_char, use_color=self.use_color + ) + if service_resource + else None + ) + + output = "\n".join([x for x in [status_string, info] if x]) + return output + + def _diff_service_view(self, describe_gateway_service_response): + """Generate diff view showing changes in the latest deployment.""" + service_arn = describe_gateway_service_response.get("serviceArn") + list_service_deployments_response = ( + self._client.list_service_deployments( + service=service_arn, maxResults=1 + ) + ) + listed_service_deployments = self._validate_and_parse_response( + list_service_deployments_response, + "ListServiceDeployments", + expected_field="serviceDeployments", + ) + if ( + not listed_service_deployments + or "serviceDeploymentArn" not in listed_service_deployments[0] + ): + return ( + None, + "Waiting for a deployment to start", + ) + + deployment_arn = listed_service_deployments[0].get( + "serviceDeploymentArn" + ) + + describe_service_deployments_response = ( + self._client.describe_service_deployments( + serviceDeploymentArns=[deployment_arn] + ) + ) + described_service_deployments = self._validate_and_parse_response( + describe_service_deployments_response, + "DescribeServiceDeployments", + expected_field="serviceDeployments", + eventually_consistent=True, + ) + if not described_service_deployments: + return (None, "Waiting for a deployment to start") + + described_service_deployment = described_service_deployments[0] + if ( + not described_service_deployment + or not described_service_deployment.get("targetServiceRevision") + ): + return ( + None, + "Waiting for a deployment to start", + ) + + target_sr = described_service_deployment.get( + "targetServiceRevision" + ).get("arn") + + target_sr_resources_list, described_target_sr_list = ( + self._describe_and_parse_service_revisions([target_sr]) + ) + if len(target_sr_resources_list) != 1: + return (None, "Trying to describe service revisions") + target_sr_resources = target_sr_resources_list[0] + described_target_sr = described_target_sr_list[0] + + task_def_arn = described_target_sr.get("taskDefinition") + if "sourceServiceRevisions" in described_service_deployment: + source_sr_resources, _ = ( + self._describe_and_parse_service_revisions( + [ + sr.get("arn") + for sr in described_service_deployment.get( + "sourceServiceRevisions" + ) + ] + ) + ) + if len(source_sr_resources) != len( + described_service_deployment.get("sourceServiceRevisions") + ): + return (None, "Trying to describe service revisions)") + source_sr_resources_combined = reduce( + lambda x, y: x.combine(y), source_sr_resources + ) + else: + source_sr_resources_combined = ManagedResourceGroup() + + updating_resources, disassociating_resources = ( + target_sr_resources.compare_resource_sets( + source_sr_resources_combined + ) + ) + updating_resources.resource_type = "Updating" + disassociating_resources.resource_type = "Disassociating" + service_resources = ManagedResourceGroup( + resource_type="Deployment", + identifier=deployment_arn, + status=described_service_deployment.get("status"), + reason=described_service_deployment.get("statusReason"), + resources=[ + ManagedResource( + resource_type="TargetServiceRevision", identifier=target_sr + ), + ManagedResource( + resource_type="TaskDefinition", identifier=task_def_arn + ), + updating_resources, + disassociating_resources, + ], + ) + return service_resources, None + + def _combined_service_view(self, describe_gateway_service_response): + """Generate combined view of all active service resources.""" + service_revision_arns = [ + config.get("serviceRevisionArn") + for config in describe_gateway_service_response.get( + "activeConfigurations" + ) + ] + service_revision_resources, _ = ( + self._describe_and_parse_service_revisions(service_revision_arns) + ) + + if len(service_revision_resources) != len(service_revision_arns): + return (None, "Trying to describe service revisions") + + service_resource = reduce( + lambda x, y: x.combine(y), service_revision_resources + ) + + return service_resource, None + + def _update_cached_monitor_results(self, resource, info): + """Update cached monitoring results with new data.""" + if not self.cached_monitor_result: + self.cached_monitor_result = (resource, info) + else: + self.cached_monitor_result = ( + resource or self.cached_monitor_result[0], + info, + ) + + def _validate_and_parse_response( + self, + response, + operation_name, + expected_field=None, + eventually_consistent=False, + ): + """Validate API response and extract expected field.""" + if not response: + raise MonitoringError(f"{operation_name} response is empty") + + self._parse_failures(response, operation_name, eventually_consistent) + + if not expected_field: + return None + + if response.get(expected_field) is None: + raise MonitoringError( + f"{operation_name} response is missing {expected_field}" + ) + return response.get(expected_field) + + def _parse_failures(self, response, operation_name, eventually_consistent): + """Parse and raise errors for API response failures.""" + failures = response.get("failures") + + if not failures: + return + + if any(not f.get('arn') or not f.get('reason') for f in failures): + raise MonitoringError( + "Invalid failure response: missing arn or reason" + ) + + if eventually_consistent: + failures = [ + failure + for failure in failures + if failure.get("reason") != "MISSING" + ] + + if not failures: + return + + failure_msgs = [ + f"{f['arn']} failed with {f['reason']}" for f in failures + ] + joined_msgs = '\n'.join(failure_msgs) + raise MonitoringError(f"{operation_name}:\n{joined_msgs}") + + def _describe_and_parse_service_revisions(self, arns): + """Describe and parse service revisions into managed resources.""" + describe_service_revisions_response = ( + self._client.describe_service_revisions(serviceRevisionArns=arns) + ) + described_service_revisions = self._validate_and_parse_response( + describe_service_revisions_response, + "DescribeServiceRevisions", + expected_field="serviceRevisions", + eventually_consistent=True, + ) + + return [ + self._parse_ecs_managed_resources(sr) + for sr in described_service_revisions + ], described_service_revisions + + def _parse_cluster(self, service): + return ManagedResource("Cluster", service.get("cluster")) + + def _parse_service(self, service): + service_arn = service.get("serviceArn") + cluster = service.get("cluster") + describe_service_response = self._client.describe_services( + cluster=cluster, services=[service_arn] + ) + described_service = self._validate_and_parse_response( + describe_service_response, "DescribeServices", "services" + )[0] + return ManagedResource( + "Service", + service.get("serviceArn"), + additional_info=described_service + and described_service.get("events")[0].get("message") + if described_service.get("events") + else None, + ) + + def _parse_ecs_managed_resources(self, service_revision): + managed_resources = service_revision.get("ecsManagedResources") + if not managed_resources: + return ManagedResourceGroup() + + parsed_resources = [] + if "ingressPaths" in managed_resources: + parsed_resources.append( + ManagedResourceGroup( + resource_type="IngressPaths", + resources=[ + self._parse_ingress_path_resources(ingress_path) + for ingress_path in managed_resources.get( + "ingressPaths" + ) + ], + ) + ) + if "autoScaling" in managed_resources: + parsed_resources.append( + self._parse_auto_scaling_configuration( + managed_resources.get("autoScaling") + ) + ) + if "metricAlarms" in managed_resources: + parsed_resources.append( + self._parse_metric_alarms( + managed_resources.get("metricAlarms") + ) + ) + if "serviceSecurityGroups" in managed_resources: + parsed_resources.append( + self._parse_service_security_groups( + managed_resources.get("serviceSecurityGroups") + ) + ) + if "logGroups" in managed_resources: + parsed_resources.append( + self._parse_log_groups(managed_resources.get("logGroups")) + ) + return ManagedResourceGroup(resources=parsed_resources) + + def _parse_ingress_path_resources(self, ingress_path): + resources = [] + if ingress_path.get("loadBalancer"): + resources.append( + self._parse_managed_resource( + ingress_path.get("loadBalancer"), "LoadBalancer" + ) + ) + if ingress_path.get("loadBalancerSecurityGroups"): + resources.extend( + self._parse_managed_resource_list( + ingress_path.get("loadBalancerSecurityGroups"), + "LoadBalancerSecurityGroup", + ) + ) + if ingress_path.get("certificate"): + resources.append( + self._parse_managed_resource( + ingress_path.get("certificate"), "Certificate" + ) + ) + if ingress_path.get("listener"): + resources.append( + self._parse_managed_resource( + ingress_path.get("listener"), "Listener" + ) + ) + if ingress_path.get("rule"): + resources.append( + self._parse_managed_resource(ingress_path.get("rule"), "Rule") + ) + if ingress_path.get("targetGroups"): + resources.extend( + self._parse_managed_resource_list( + ingress_path.get("targetGroups"), "TargetGroup" + ) + ) + return ManagedResourceGroup( + resource_type="IngressPath", + identifier=ingress_path.get("endpoint"), + resources=resources, + ) + + def _parse_auto_scaling_configuration(self, auto_scaling_configuration): + resources = [] + if auto_scaling_configuration.get("scalableTarget"): + resources.append( + self._parse_managed_resource( + auto_scaling_configuration.get("scalableTarget"), + "ScalableTarget", + ) + ) + if auto_scaling_configuration.get("applicationAutoScalingPolicies"): + resources.extend( + self._parse_managed_resource_list( + auto_scaling_configuration.get( + "applicationAutoScalingPolicies" + ), + "ApplicationAutoScalingPolicy", + ) + ) + return ManagedResourceGroup( + resource_type="AutoScalingConfiguration", resources=resources + ) + + def _parse_metric_alarms(self, metric_alarms): + return ManagedResourceGroup( + resource_type="MetricAlarms", + resources=self._parse_managed_resource_list( + metric_alarms, "MetricAlarm" + ), + ) + + def _parse_service_security_groups(self, service_security_groups): + return ManagedResourceGroup( + resource_type="ServiceSecurityGroups", + resources=self._parse_managed_resource_list( + service_security_groups, "SecurityGroup" + ), + ) + + def _parse_log_groups(self, logs_groups): + return ManagedResourceGroup( + resource_type="LogGroups", + resources=self._parse_managed_resource_list( + logs_groups, "LogGroup" + ), + ) + + def _parse_managed_resource(self, resource, resource_type): + return ManagedResource( + resource_type, + resource.get("arn"), + status=resource.get("status"), + updated_at=resource.get("updatedAt"), + reason=resource.get("statusReason"), + ) + + def _parse_managed_resource_list(self, data_list, resource_type): + return [ + self._parse_managed_resource(data, resource_type) + for data in data_list + ] diff --git a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py index 37d9fa0a63e1..e0677a8e3451 100644 --- a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py @@ -123,617 +123,6 @@ def test_is_monitoring_available_without_tty(self, mock_isatty): ECSExpressGatewayServiceWatcher.is_monitoring_available() is False ) - def setup_method(self): - self.app_session = create_app_session(output=DummyOutput()) - self.app_session.__enter__() - self.mock_client = Mock() - self.service_arn = ( - "arn:aws:ecs:us-west-2:123456789012:service/my-cluster/my-service" - ) - - def teardown_method(self): - if hasattr(self, 'app_session'): - self.app_session.__exit__(None, None, None) - - def _create_watcher_with_mocks(self, resource_view="RESOURCE", timeout=1): - """Helper to create watcher with mocked display""" - mock_display = Mock() - mock_display.has_terminal.return_value = True - mock_display._check_keypress.return_value = None - mock_display._restore_terminal.return_value = None - mock_display.display.return_value = None - - watcher = ECSExpressGatewayServiceWatcher( - self.mock_client, - self.service_arn, - resource_view, - timeout_minutes=timeout, - display=mock_display, - ) - - # Mock exec to call the monitoring method once and print output - original_monitor = watcher._monitor_express_gateway_service - - def mock_exec(): - try: - output = original_monitor("⠋", self.service_arn, resource_view) - print(output) - print("Monitoring Complete!") - except Exception as e: - # Re-raise expected exceptions - if isinstance(e, (ClientError, MonitoringError)): - raise - # For other exceptions, just print and complete - print("Monitoring Complete!") - - watcher.exec = mock_exec - return watcher - - @patch('time.sleep') - def test_exec_successful_all_mode_monitoring(self, mock_sleep, capsys): - """Test successful monitoring in RESOURCE mode with resource parsing""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [ - { - "arn": "rev-arn", - "ecsManagedResources": { - "ingressPaths": [ - { - "endpoint": "https://api.example.com", - "loadBalancer": { - "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/my-lb/1234567890abcdef", - "status": "ACTIVE", - }, - "targetGroups": [ - { - "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/my-tg/1234567890abcdef", - "status": "HEALTHY", - } - ], - } - ], - "serviceSecurityGroups": [ - { - "arn": "arn:aws:ec2:us-west-2:123456789012:security-group/sg-1234567890abcdef0", - "status": "ACTIVE", - } - ], - "logGroups": [ - { - "arn": "arn:aws:logs:us-west-2:123456789012:log-group:/aws/ecs/my-service", - "status": "ACTIVE", - } - ], - }, - } - ] - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Running"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - # Verify parsed resources appear in output - assert "Cluster" in output_text - assert "Service" in output_text - assert "IngressPath" in output_text - assert "LoadBalancer" in output_text - assert "TargetGroup" in output_text - assert "SecurityGroup" in output_text - assert "LogGroup" in output_text - - # Specific identifiers - assert "https://api.example.com" in output_text # IngressPath endpoint - assert "my-lb" in output_text # LoadBalancer identifier - assert "my-tg" in output_text # TargetGroup identifier - assert ( - "sg-1234567890abcdef0" in output_text - ) # SecurityGroup identifier - assert "/aws/ecs/my-service" in output_text # LogGroup identifier - - # Status values - assert "ACTIVE" in output_text # LoadBalancer and SecurityGroup status - assert "HEALTHY" in output_text # TargetGroup status - - @patch('time.sleep') - def test_exec_successful_delta_mode_with_deployment( - self, mock_sleep, capsys - ): - """Test DEPLOYMENT mode executes successfully""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [], - } - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Service running"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - - # Verify DEPLOYMENT mode executes successfully - assert captured.out - - @patch('time.sleep') - def test_exec_combined_view_multiple_revisions(self, mock_sleep, capsys): - """Test RESOURCE mode combines multiple service revisions correctly""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - # Multiple active configurations (combined view) - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [ - {"serviceRevisionArn": "rev-1"}, - {"serviceRevisionArn": "rev-2"}, - ], - } - } - - # Mock multiple revisions with different resources - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [ - { - "arn": "rev-1", - "ecsManagedResources": { - "ingressPaths": [ - { - "endpoint": "https://api.example.com", - "loadBalancer": { - "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/api-lb/1234", - "status": "ACTIVE", - }, - } - ], - "serviceSecurityGroups": [ - { - "arn": "arn:aws:ec2:us-west-2:123456789012:security-group/sg-api123", - "status": "ACTIVE", - } - ], - }, - }, - { - "arn": "rev-2", - "ecsManagedResources": { - "ingressPaths": [ - { - "endpoint": "https://web.example.com", - "loadBalancer": { - "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/web-lb/5678", - "status": "CREATING", - }, - } - ], - "logGroups": [ - { - "arn": "arn:aws:logs:us-west-2:123456789012:log-group:/aws/ecs/web-logs", - "status": "ACTIVE", - } - ], - }, - }, - ] - } - - self.mock_client.describe_services.return_value = { - "services": [ - {"events": [{"message": "Multiple revisions active"}]} - ] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - # Verify combined view shows resources from both revisions - # Resource types from both revisions - assert "IngressPath" in output_text - assert "LoadBalancer" in output_text - assert "SecurityGroup" in output_text # From rev-1 - assert "LogGroup" in output_text # From rev-2 - - # Specific identifiers from both revisions - assert "https://api.example.com" in output_text # From rev-1 - assert "https://web.example.com" in output_text # From rev-2 - assert "api-lb" in output_text # From rev-1 - assert "web-lb" in output_text # From rev-2 - assert "sg-api123" in output_text # From rev-1 - assert "/aws/ecs/web-logs" in output_text # From rev-2 - - # Status values from both revisions - assert "ACTIVE" in output_text # From both revisions - assert "CREATING" in output_text # From rev-2 - - @patch('time.sleep') - def test_exec_keyboard_interrupt_handling(self, mock_sleep, capsys): - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [], - } - } - - watcher.exec() - captured = capsys.readouterr() - - # Verify completion message is printed - assert "Monitoring Complete!" in captured.out - - @patch('time.sleep') - def test_exec_with_service_not_found_error(self, mock_sleep): - """Test exec() with service not found error bubbles up""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - error = ClientError( - error_response={ - 'Error': { - 'Code': 'ServiceNotFoundException', - 'Message': 'Service not found', - } - }, - operation_name='DescribeExpressGatewayService', - ) - self.mock_client.describe_express_gateway_service.side_effect = error - - with pytest.raises(ClientError) as exc_info: - watcher.exec() - - # Verify the specific error is raised - assert ( - exc_info.value.response['Error']['Code'] - == 'ServiceNotFoundException' - ) - assert ( - exc_info.value.response['Error']['Message'] == 'Service not found' - ) - - @patch('time.sleep') - def test_exec_with_inactive_service_handled_gracefully( - self, mock_sleep, capsys - ): - """Test exec() handles inactive service gracefully""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.side_effect = ClientError( - error_response={ - 'Error': { - 'Code': 'InvalidParameterException', - 'Message': 'Cannot call DescribeServiceRevisions for a service that is INACTIVE', - } - }, - operation_name='DescribeExpressGatewayService', - ) - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Service is inactive"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - - # Verify inactive service is handled and appropriate message shown - assert "inactive" in captured.out.lower() - - @patch('time.sleep') - def test_exec_with_empty_resources(self, mock_sleep, capsys): - """Test parsing edge case: empty/null resources""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - # Empty ecsManagedResources - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [{"arn": "rev-arn", "ecsManagedResources": {}}] - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "No resources"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - # Should handle empty resources gracefully but still show basic structure - assert "Cluster" in output_text - assert "Service" in output_text - # Should NOT contain resource types since ecsManagedResources is empty - assert "IngressPath" not in output_text - assert "LoadBalancer" not in output_text - - @patch('time.sleep') - def test_exec_with_autoscaling_resources(self, mock_sleep, capsys): - """Test autoscaling resource parsing with scalableTarget and policies""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [ - { - "arn": "rev-arn", - "ecsManagedResources": { - "autoScaling": { - "scalableTarget": { - "arn": "arn:aws:application-autoscaling:us-west-2:123456789012:scalable-target/1234567890abcdef", - "status": "ACTIVE", - }, - "applicationAutoScalingPolicies": [ - { - "arn": "arn:aws:application-autoscaling:us-west-2:123456789012:scaling-policy/cpu-policy", - "status": "ACTIVE", - }, - { - "arn": "arn:aws:application-autoscaling:us-west-2:123456789012:scaling-policy/memory-policy", - "status": "ACTIVE", - }, - ], - } - }, - } - ] - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Autoscaling active"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - assert "AutoScaling" in output_text - assert "ScalableTarget" in output_text - assert "AutoScalingPolicy" in output_text - # ScalableTarget identifier - assert "1234567890abcdef" in output_text - # Policy identifiers - assert "cpu-policy" in output_text - assert "memory-policy" in output_text - - @patch('time.sleep') - def test_exec_with_malformed_resource_data(self, mock_sleep, capsys): - """Test parsing edge case: malformed resource data""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - # Malformed resources - missing required fields - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [ - { - "arn": "rev-arn", - "ecsManagedResources": { - "ingressPaths": [ - {"endpoint": "https://example.com"} - ], # Missing loadBalancer - "serviceSecurityGroups": [ - {"status": "ACTIVE"} - ], # Missing arn - }, - } - ] - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Malformed data"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - # Should handle malformed data gracefully and show what it can parse - assert "IngressPath" in output_text - assert "https://example.com" in output_text - # Should show SecurityGroup type even with missing arn - assert "SecurityGroup" in output_text - # Should NOT show LoadBalancer since it's missing from IngressPath - assert "LoadBalancer" not in output_text - - @patch('time.sleep') - def test_exec_eventually_consistent_missing_deployment( - self, mock_sleep, capsys - ): - """Test eventually consistent behavior: deployment missing after list""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [], - } - } - # List shows deployment exists - self.mock_client.list_service_deployments.return_value = { - "serviceDeployments": [{"serviceDeploymentArn": "deploy-arn"}] - } - # But describe fails (eventually consistent) - self.mock_client.describe_service_deployments.return_value = { - "serviceDeployments": [], - "failures": [{"arn": "deploy-arn", "reason": "MISSING"}], - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Eventually consistent"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - # Should handle eventually consistent missing deployment gracefully - # Should show waiting state when deployment is missing - assert "Trying to describe gateway service" in output_text - assert "Monitoring Complete" in output_text - - @patch('time.sleep') - def test_exec_eventually_consistent_missing_revision( - self, mock_sleep, capsys - ): - """Test eventually consistent behavior: service revision missing after deployment describe""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [], - } - } - self.mock_client.list_service_deployments.return_value = { - "serviceDeployments": [{"serviceDeploymentArn": "deploy-arn"}] - } - self.mock_client.describe_service_deployments.return_value = { - "serviceDeployments": [ - { - "serviceDeploymentArn": "deploy-arn", - "status": "IN_PROGRESS", - "targetServiceRevision": {"arn": "target-rev"}, - } - ] - } - # Service revision missing (eventually consistent) - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [], - "failures": [{"arn": "target-rev", "reason": "MISSING"}], - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Revision missing"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - # Should handle eventually consistent missing revision gracefully - # Should show waiting state when revision is missing - assert "Trying to describe gateway service" in output_text - assert "Monitoring Complete" in output_text - - @patch('time.sleep') - def test_exec_with_api_failures(self, mock_sleep): - """Test failure parsing: API returns failures""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - # API returns failures - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [], - "failures": [{"arn": "rev-arn", "reason": "ServiceNotFound"}], - } - - with pytest.raises(MonitoringError) as exc_info: - watcher.exec() - - # Should raise MonitoringError with failure details - error_message = str(exc_info.value) - assert "DescribeServiceRevisions" in error_message - assert "rev-arn" in error_message - assert "ServiceNotFound" in error_message - - @patch('time.sleep') - def test_exec_with_malformed_api_failures(self, mock_sleep): - """Test failure parsing: malformed failure responses""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - # Malformed failures - missing arn or reason - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [], - "failures": [{"reason": "ServiceNotFound"}], # Missing arn - } - - with pytest.raises(MonitoringError) as exc_info: - watcher.exec() - - # Should raise MonitoringError about invalid failure response - error_message = str(exc_info.value) - assert "Invalid failure response" in error_message - assert "missing arn or reason" in error_message - - @patch('time.sleep') - def test_exec_with_missing_response_fields(self, mock_sleep): - """Test response validation: missing required fields""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - # Missing serviceRevisions field - self.mock_client.describe_service_revisions.return_value = {} - - with pytest.raises(MonitoringError) as exc_info: - watcher.exec() - - # Should raise MonitoringError about empty response - error_message = str(exc_info.value) - assert "DescribeServiceRevisions" in error_message - assert "empty" in error_message - class TestMonitoringError: """Test MonitoringError exception class""" diff --git a/tests/unit/customizations/ecs/test_serviceviewcollector.py b/tests/unit/customizations/ecs/test_serviceviewcollector.py new file mode 100644 index 000000000000..997405e9b97f --- /dev/null +++ b/tests/unit/customizations/ecs/test_serviceviewcollector.py @@ -0,0 +1,830 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ + +from unittest.mock import Mock + +import pytest +from botocore.exceptions import ClientError + +from awscli.customizations.ecs.exceptions import MonitoringError +from awscli.customizations.ecs.serviceviewcollector import ( + ServiceViewCollector, +) + + +class TestServiceViewCollector: + """Test ServiceViewCollector business logic""" + + def setup_method(self): + self.mock_client = Mock() + self.service_arn = ( + "arn:aws:ecs:us-west-2:123456789012:service/my-cluster/my-service" + ) + + def test_get_current_view_resource_mode(self): + """Test get_current_view in RESOURCE mode parses resources""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [ + { + "arn": "rev-arn", + "ecsManagedResources": { + "ingressPaths": [ + { + "endpoint": "https://api.example.com", + "loadBalancer": { + "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/my-lb/1234567890abcdef", + "status": "ACTIVE", + }, + } + ], + }, + } + ] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Running"}]}] + } + + output = collector.get_current_view("⠋") + + assert "Cluster" in output + assert "Service" in output + assert "IngressPath" in output + assert "LoadBalancer" in output + assert "https://api.example.com" in output + assert "ACTIVE" in output + + def test_get_current_view_handles_inactive_service(self): + """Test get_current_view handles inactive service gracefully""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.side_effect = ClientError( + error_response={ + 'Error': { + 'Code': 'InvalidParameterException', + 'Message': 'Cannot call DescribeServiceRevisions for a service that is INACTIVE', + } + }, + operation_name='DescribeExpressGatewayService', + ) + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Service is inactive"}]}] + } + + output = collector.get_current_view("⠋") + + assert "inactive" in output.lower() + + def test_get_current_view_with_api_failures(self): + """Test get_current_view raises MonitoringError on API failures""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [], + "failures": [{"arn": "rev-arn", "reason": "ServiceNotFound"}], + } + + with pytest.raises(MonitoringError) as exc_info: + collector.get_current_view("⠋") + + error_message = str(exc_info.value) + assert "DescribeServiceRevisions" in error_message + assert "rev-arn" in error_message + assert "ServiceNotFound" in error_message + + def test_get_current_view_caches_results(self): + """Test get_current_view caches results within refresh interval""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [{"arn": "rev-arn", "ecsManagedResources": {}}] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Running"}]}] + } + + # First call + collector.get_current_view("⠋") + call_count_first = ( + self.mock_client.describe_express_gateway_service.call_count + ) + + # Second call within refresh interval (default 5000ms) + collector.get_current_view("⠙") + # Should use cached result, not call API again + call_count_second = ( + self.mock_client.describe_express_gateway_service.call_count + ) + assert call_count_first == call_count_second # Cached, no new API call + + def test_combined_view_multiple_revisions(self): + """Test RESOURCE mode combines multiple service revisions correctly""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + # Multiple active configurations (combined view) + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [ + {"serviceRevisionArn": "rev-1"}, + {"serviceRevisionArn": "rev-2"}, + ], + } + } + + # Mock multiple revisions with different resources + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [ + { + "arn": "rev-1", + "ecsManagedResources": { + "ingressPaths": [ + { + "endpoint": "https://api.example.com", + "loadBalancer": { + "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/api-lb/1234", + "status": "ACTIVE", + }, + } + ], + "serviceSecurityGroups": [ + { + "arn": "arn:aws:ec2:us-west-2:123456789012:security-group/sg-api123", + "status": "ACTIVE", + } + ], + }, + }, + { + "arn": "rev-2", + "ecsManagedResources": { + "ingressPaths": [ + { + "endpoint": "https://web.example.com", + "loadBalancer": { + "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/web-lb/5678", + "status": "CREATING", + }, + } + ], + "logGroups": [ + { + "arn": "arn:aws:logs:us-west-2:123456789012:log-group:/aws/ecs/web-logs", + "status": "ACTIVE", + } + ], + }, + }, + ] + } + + self.mock_client.describe_services.return_value = { + "services": [ + {"events": [{"message": "Multiple revisions active"}]} + ] + } + + output = collector.get_current_view("⠋") + + # Verify combined view shows resources from both revisions + assert "IngressPath" in output + assert "LoadBalancer" in output + assert "SecurityGroup" in output # From rev-1 + assert "LogGroup" in output # From rev-2 + assert "https://api.example.com" in output # From rev-1 + assert "https://web.example.com" in output # From rev-2 + assert "api-lb" in output # From rev-1 + assert "web-lb" in output # From rev-2 + assert "sg-api123" in output # From rev-1 + assert "/aws/ecs/web-logs" in output # From rev-2 + assert "ACTIVE" in output # From both revisions + assert "CREATING" in output # From rev-2 + + def test_get_current_view_with_empty_resources(self): + """Test parsing edge case: empty/null resources""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + # Empty ecsManagedResources + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [{"arn": "rev-arn", "ecsManagedResources": {}}] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "No resources"}]}] + } + + output = collector.get_current_view("⠋") + + # Should handle empty resources gracefully but still show basic structure + assert "Cluster" in output + assert "Service" in output + # Should NOT contain resource types since ecsManagedResources is empty + assert "IngressPath" not in output + assert "LoadBalancer" not in output + + def test_get_current_view_with_autoscaling_resources(self): + """Test autoscaling resource parsing with scalableTarget and policies""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [ + { + "arn": "rev-arn", + "ecsManagedResources": { + "autoScaling": { + "scalableTarget": { + "arn": "arn:aws:application-autoscaling:us-west-2:123456789012:scalable-target/1234567890abcdef", + "status": "ACTIVE", + }, + "applicationAutoScalingPolicies": [ + { + "arn": "arn:aws:application-autoscaling:us-west-2:123456789012:scaling-policy/cpu-policy", + "status": "ACTIVE", + }, + { + "arn": "arn:aws:application-autoscaling:us-west-2:123456789012:scaling-policy/memory-policy", + "status": "ACTIVE", + }, + ], + } + }, + } + ] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Autoscaling active"}]}] + } + + output = collector.get_current_view("⠋") + + assert "AutoScaling" in output + assert "ScalableTarget" in output + assert "AutoScalingPolicy" in output + assert "1234567890abcdef" in output + assert "cpu-policy" in output + assert "memory-policy" in output + + def test_get_current_view_with_malformed_resource_data(self): + """Test parsing edge case: malformed resource data""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + # Malformed resources - missing required fields + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [ + { + "arn": "rev-arn", + "ecsManagedResources": { + "ingressPaths": [ + {"endpoint": "https://example.com"} + ], # Missing loadBalancer + "serviceSecurityGroups": [ + {"status": "ACTIVE"} + ], # Missing arn + }, + } + ] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Malformed data"}]}] + } + + output = collector.get_current_view("⠋") + + # Should handle malformed data gracefully and show what it can parse + assert "IngressPath" in output + assert "https://example.com" in output + # Should show SecurityGroup type even with missing arn + assert "SecurityGroup" in output + # Should NOT show LoadBalancer since it's missing from IngressPath + assert "LoadBalancer" not in output + + def test_eventually_consistent_missing_deployment(self): + """Test eventually consistent behavior: deployment missing after list""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "DEPLOYMENT" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + # List shows deployment exists + self.mock_client.list_service_deployments.return_value = { + "serviceDeployments": [{"serviceDeploymentArn": "deploy-arn"}] + } + # But describe fails (eventually consistent) + self.mock_client.describe_service_deployments.return_value = { + "serviceDeployments": [], + "failures": [{"arn": "deploy-arn", "reason": "MISSING"}], + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Eventually consistent"}]}] + } + + output = collector.get_current_view("⠋") + + # Should handle eventually consistent missing deployment gracefully + assert "Waiting for a deployment to start" in output + + def test_eventually_consistent_missing_revision(self): + """Test eventually consistent behavior: service revision missing""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "DEPLOYMENT" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + self.mock_client.list_service_deployments.return_value = { + "serviceDeployments": [{"serviceDeploymentArn": "deploy-arn"}] + } + self.mock_client.describe_service_deployments.return_value = { + "serviceDeployments": [ + { + "serviceDeploymentArn": "deploy-arn", + "status": "IN_PROGRESS", + "targetServiceRevision": {"arn": "target-rev"}, + } + ] + } + # Service revision missing (eventually consistent) + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [], + "failures": [{"arn": "target-rev", "reason": "MISSING"}], + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Revision missing"}]}] + } + + output = collector.get_current_view("⠋") + + # Should handle eventually consistent missing revision gracefully + assert "Trying to describe service revisions" in output + + def test_with_malformed_api_failures(self): + """Test failure parsing: malformed failure responses""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + # Malformed failures - missing arn or reason + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [], + "failures": [{"reason": "ServiceNotFound"}], # Missing arn + } + + with pytest.raises(MonitoringError) as exc_info: + collector.get_current_view("⠋") + + # Should raise MonitoringError about invalid failure response + error_message = str(exc_info.value) + assert "Invalid failure response" in error_message + assert "missing arn or reason" in error_message + + def test_with_missing_response_fields(self): + """Test response validation: missing required fields""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + # Missing serviceRevisions field + self.mock_client.describe_service_revisions.return_value = {} + + with pytest.raises(MonitoringError) as exc_info: + collector.get_current_view("⠋") + + # Should raise MonitoringError about missing field + error_message = str(exc_info.value) + assert "DescribeServiceRevisions" in error_message + assert ( + "response is" in error_message + ) # "response is missing" or "response is empty" + + def test_deployment_mode_diff_view(self): + """Test DEPLOYMENT mode shows diff of target vs source revisions""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "DEPLOYMENT" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + self.mock_client.list_service_deployments.return_value = { + "serviceDeployments": [{"serviceDeploymentArn": "deploy-arn"}] + } + self.mock_client.describe_service_deployments.return_value = { + "serviceDeployments": [ + { + "serviceDeploymentArn": "deploy-arn", + "status": "IN_PROGRESS", + "targetServiceRevision": {"arn": "target-rev"}, + "sourceServiceRevisions": [{"arn": "source-rev"}], + } + ] + } + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [ + { + "arn": "target-rev", + "taskDefinition": "task-def-arn", + "ecsManagedResources": { + "ingressPaths": [ + { + "endpoint": "https://new-api.example.com", + "loadBalancer": { + "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/new-lb/1234", + "status": "CREATING", + }, + } + ], + }, + }, + { + "arn": "source-rev", + "ecsManagedResources": { + "ingressPaths": [ + { + "endpoint": "https://old-api.example.com", + "loadBalancer": { + "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/old-lb/5678", + "status": "ACTIVE", + }, + } + ], + }, + }, + ] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Deployment in progress"}]}] + } + + output = collector.get_current_view("⠋") + + # Should show deployment diff + # Initially will show "Trying to describe service revisions" due to mismatch + # But implementation still shows Cluster/Service + assert "Trying to describe service revisions" in output + + def test_waiting_for_deployment_to_start(self): + """Test DEPLOYMENT mode when no deployment exists yet""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "DEPLOYMENT" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + # No deployments + self.mock_client.list_service_deployments.return_value = { + "serviceDeployments": [] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "No deployment"}]}] + } + + output = collector.get_current_view("⠋") + + assert "Waiting for a deployment to start" in output + + def test_deployment_missing_target_revision(self): + """Test DEPLOYMENT mode when deployment is missing target revision""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "DEPLOYMENT" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + self.mock_client.list_service_deployments.return_value = { + "serviceDeployments": [{"serviceDeploymentArn": "deploy-arn"}] + } + self.mock_client.describe_service_deployments.return_value = { + "serviceDeployments": [ + { + "serviceDeploymentArn": "deploy-arn", + "status": "IN_PROGRESS", + # Missing targetServiceRevision + } + ] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Deployment starting"}]}] + } + + output = collector.get_current_view("⠋") + + assert "Waiting for a deployment to start" in output + + def test_empty_describe_gateway_service_response(self): + """Test handling of empty describe_express_gateway_service response""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = None + + output = collector.get_current_view("⠋") + + assert "Trying to describe gateway service" in output + + def test_missing_service_in_response(self): + """Test handling when service field is missing""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = {} + + output = collector.get_current_view("⠋") + + assert "Trying to describe gateway service" in output + + def test_service_missing_required_fields(self): + """Test handling when service is missing required fields""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + # Missing activeConfigurations + self.mock_client.describe_express_gateway_service.return_value = { + "service": {"serviceArn": self.service_arn} + } + + output = collector.get_current_view("⠋") + + assert "Trying to describe gateway service" in output + + def test_empty_response_error(self): + """Test _validate_and_parse_response with empty response""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + with pytest.raises(MonitoringError) as exc_info: + collector._validate_and_parse_response( + None, "TestOperation", "expectedField" + ) + + assert "TestOperation response is empty" in str(exc_info.value) + + def test_missing_expected_field_error(self): + """Test _validate_and_parse_response with missing expected field""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + with pytest.raises(MonitoringError) as exc_info: + collector._validate_and_parse_response( + {"otherField": "value"}, "TestOperation", "expectedField" + ) + + error_message = str(exc_info.value) + assert "TestOperation" in error_message + assert "expectedField" in error_message + + def test_parse_failures_filters_missing_reason(self): + """Test _parse_failures filters MISSING failures for eventually consistent""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + response = { + "failures": [ + {"arn": "arn1", "reason": "MISSING"}, + {"arn": "arn2", "reason": "ServiceNotFound"}, + ] + } + + # Eventually consistent - should only raise for non-MISSING + with pytest.raises(MonitoringError) as exc_info: + collector._parse_failures( + response, "TestOp", eventually_consistent=True + ) + + error_message = str(exc_info.value) + assert "arn2" in error_message + assert "ServiceNotFound" in error_message + # Should NOT include arn1 (MISSING reason) + assert "arn1" not in error_message + + def test_parse_failures_all_missing_no_error(self): + """Test _parse_failures doesn't raise when all failures are MISSING""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + response = { + "failures": [ + {"arn": "arn1", "reason": "MISSING"}, + {"arn": "arn2", "reason": "MISSING"}, + ] + } + + # Should not raise when all failures are MISSING + collector._parse_failures( + response, "TestOp", eventually_consistent=True + ) + + def test_parse_failures_malformed(self): + """Test _parse_failures with malformed failure data""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + response = { + "failures": [ + {"reason": "ServiceNotFound"} # Missing arn + ] + } + + with pytest.raises(MonitoringError) as exc_info: + collector._parse_failures( + response, "TestOp", eventually_consistent=False + ) + + assert "Invalid failure response" in str(exc_info.value) + + def test_parse_all_resource_types(self): + """Test parsing all supported resource types""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "RESOURCE" + ) + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [ + { + "arn": "rev-arn", + "ecsManagedResources": { + "ingressPaths": [ + { + "endpoint": "https://api.example.com", + "loadBalancer": { + "arn": "lb-arn", + "status": "ACTIVE", + }, + "loadBalancerSecurityGroups": [ + {"arn": "lb-sg-arn", "status": "ACTIVE"} + ], + "certificate": { + "arn": "cert-arn", + "status": "ACTIVE", + }, + "listener": { + "arn": "listener-arn", + "status": "ACTIVE", + }, + "rule": { + "arn": "rule-arn", + "status": "ACTIVE", + }, + "targetGroups": [ + {"arn": "tg-arn", "status": "ACTIVE"} + ], + } + ], + "autoScaling": { + "scalableTarget": { + "arn": "st-arn", + "status": "ACTIVE", + }, + "applicationAutoScalingPolicies": [ + {"arn": "policy-arn", "status": "ACTIVE"} + ], + }, + "metricAlarms": [ + {"arn": "alarm-arn", "status": "ACTIVE"} + ], + "serviceSecurityGroups": [ + {"arn": "sg-arn", "status": "ACTIVE"} + ], + "logGroups": [{"arn": "log-arn", "status": "ACTIVE"}], + }, + } + ] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "All resources"}]}] + } + + output = collector.get_current_view("⠋") + + # Verify all resource types are parsed + assert "IngressPath" in output + assert "LoadBalancer" in output + assert "LoadBalancerSecurityGroup" in output + assert "Certificate" in output + assert "Listener" in output + assert "Rule" in output + assert "TargetGroup" in output + assert "AutoScalingConfiguration" in output + assert "ScalableTarget" in output + assert "ApplicationAutoScalingPolicy" in output + assert "MetricAlarms" in output + assert "ServiceSecurityGroups" in output + assert "LogGroups" in output From edd812473ca3057781ecc6e047ff8c2e3d41948c Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Thu, 11 Dec 2025 16:22:43 -0500 Subject: [PATCH 02/17] Address CR feedback --- .../ecs/serviceviewcollector.py | 4 +- .../ecs/test_serviceviewcollector.py | 148 ++++++------------ 2 files changed, 48 insertions(+), 104 deletions(-) diff --git a/awscli/customizations/ecs/serviceviewcollector.py b/awscli/customizations/ecs/serviceviewcollector.py index a9c28732d86f..21a28e19a01f 100644 --- a/awscli/customizations/ecs/serviceviewcollector.py +++ b/awscli/customizations/ecs/serviceviewcollector.py @@ -240,7 +240,7 @@ def _diff_service_view(self, describe_gateway_service_response): if len(source_sr_resources) != len( described_service_deployment.get("sourceServiceRevisions") ): - return (None, "Trying to describe service revisions)") + return (None, "Trying to describe service revisions") source_sr_resources_combined = reduce( lambda x, y: x.combine(y), source_sr_resources ) @@ -248,7 +248,7 @@ def _diff_service_view(self, describe_gateway_service_response): source_sr_resources_combined = ManagedResourceGroup() updating_resources, disassociating_resources = ( - target_sr_resources.compare_resource_sets( + target_sr_resources.diff( source_sr_resources_combined ) ) diff --git a/tests/unit/customizations/ecs/test_serviceviewcollector.py b/tests/unit/customizations/ecs/test_serviceviewcollector.py index 997405e9b97f..36ba679c39f3 100644 --- a/tests/unit/customizations/ecs/test_serviceviewcollector.py +++ b/tests/unit/customizations/ecs/test_serviceviewcollector.py @@ -433,6 +433,52 @@ def test_eventually_consistent_missing_revision(self): # Should handle eventually consistent missing revision gracefully assert "Trying to describe service revisions" in output + def test_eventually_consistent_mixed_failures(self): + """Test eventually consistent behavior: filters MISSING but raises for other failures""" + collector = ServiceViewCollector( + self.mock_client, self.service_arn, "DEPLOYMENT" + ) + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + self.mock_client.list_service_deployments.return_value = { + "serviceDeployments": [ + {"serviceDeploymentArn": "deploy-arn"} + ] + } + self.mock_client.describe_service_deployments.return_value = { + "serviceDeployments": [ + { + "serviceDeploymentArn": "deploy-arn", + "status": "IN_PROGRESS", + "targetServiceRevision": {"arn": "target-rev"}, + } + ] + } + # Mixed failures: MISSING (should be filtered) and ServiceNotFound (should raise) + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [], + "failures": [ + {"arn": "target-rev", "reason": "MISSING"}, + {"arn": "other-rev", "reason": "ServiceNotFound"}, + ], + } + + # Should raise error for non-MISSING failure + with pytest.raises(MonitoringError) as exc_info: + collector.get_current_view("⠋") + + error_message = str(exc_info.value) + # Should include non-MISSING failure + assert "other-rev" in error_message + assert "ServiceNotFound" in error_message + # Should NOT include MISSING failure + assert "target-rev" not in error_message + def test_with_malformed_api_failures(self): """Test failure parsing: malformed failure responses""" collector = ServiceViewCollector( @@ -614,18 +660,6 @@ def test_deployment_missing_target_revision(self): assert "Waiting for a deployment to start" in output - def test_empty_describe_gateway_service_response(self): - """Test handling of empty describe_express_gateway_service response""" - collector = ServiceViewCollector( - self.mock_client, self.service_arn, "RESOURCE" - ) - - self.mock_client.describe_express_gateway_service.return_value = None - - output = collector.get_current_view("⠋") - - assert "Trying to describe gateway service" in output - def test_missing_service_in_response(self): """Test handling when service field is missing""" collector = ServiceViewCollector( @@ -653,96 +687,6 @@ def test_service_missing_required_fields(self): assert "Trying to describe gateway service" in output - def test_empty_response_error(self): - """Test _validate_and_parse_response with empty response""" - collector = ServiceViewCollector( - self.mock_client, self.service_arn, "RESOURCE" - ) - - with pytest.raises(MonitoringError) as exc_info: - collector._validate_and_parse_response( - None, "TestOperation", "expectedField" - ) - - assert "TestOperation response is empty" in str(exc_info.value) - - def test_missing_expected_field_error(self): - """Test _validate_and_parse_response with missing expected field""" - collector = ServiceViewCollector( - self.mock_client, self.service_arn, "RESOURCE" - ) - - with pytest.raises(MonitoringError) as exc_info: - collector._validate_and_parse_response( - {"otherField": "value"}, "TestOperation", "expectedField" - ) - - error_message = str(exc_info.value) - assert "TestOperation" in error_message - assert "expectedField" in error_message - - def test_parse_failures_filters_missing_reason(self): - """Test _parse_failures filters MISSING failures for eventually consistent""" - collector = ServiceViewCollector( - self.mock_client, self.service_arn, "RESOURCE" - ) - - response = { - "failures": [ - {"arn": "arn1", "reason": "MISSING"}, - {"arn": "arn2", "reason": "ServiceNotFound"}, - ] - } - - # Eventually consistent - should only raise for non-MISSING - with pytest.raises(MonitoringError) as exc_info: - collector._parse_failures( - response, "TestOp", eventually_consistent=True - ) - - error_message = str(exc_info.value) - assert "arn2" in error_message - assert "ServiceNotFound" in error_message - # Should NOT include arn1 (MISSING reason) - assert "arn1" not in error_message - - def test_parse_failures_all_missing_no_error(self): - """Test _parse_failures doesn't raise when all failures are MISSING""" - collector = ServiceViewCollector( - self.mock_client, self.service_arn, "RESOURCE" - ) - - response = { - "failures": [ - {"arn": "arn1", "reason": "MISSING"}, - {"arn": "arn2", "reason": "MISSING"}, - ] - } - - # Should not raise when all failures are MISSING - collector._parse_failures( - response, "TestOp", eventually_consistent=True - ) - - def test_parse_failures_malformed(self): - """Test _parse_failures with malformed failure data""" - collector = ServiceViewCollector( - self.mock_client, self.service_arn, "RESOURCE" - ) - - response = { - "failures": [ - {"reason": "ServiceNotFound"} # Missing arn - ] - } - - with pytest.raises(MonitoringError) as exc_info: - collector._parse_failures( - response, "TestOp", eventually_consistent=False - ) - - assert "Invalid failure response" in str(exc_info.value) - def test_parse_all_resource_types(self): """Test parsing all supported resource types""" collector = ServiceViewCollector( From 006beab286d002d1a7dd9b85bfcab13b3c065d1a Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Fri, 12 Dec 2025 09:10:26 -0500 Subject: [PATCH 03/17] Creating stream display to be used for text only monitoring --- .../ecs/expressgateway/managedresource.py | 72 +++- .../expressgateway/managedresourcegroup.py | 92 +++++- .../ecs/expressgateway/stream_display.py | 110 +++++++ .../test_managedresourcegroup.py | 288 ---------------- .../ecs/expressgateway/test_stream_display.py | 309 ++++++++++++++++++ 5 files changed, 569 insertions(+), 302 deletions(-) create mode 100644 awscli/customizations/ecs/expressgateway/stream_display.py create mode 100644 tests/unit/customizations/ecs/expressgateway/test_stream_display.py diff --git a/awscli/customizations/ecs/expressgateway/managedresource.py b/awscli/customizations/ecs/expressgateway/managedresource.py index f4b3303bf678..23581c92a5c6 100644 --- a/awscli/customizations/ecs/expressgateway/managedresource.py +++ b/awscli/customizations/ecs/expressgateway/managedresource.py @@ -113,6 +113,52 @@ def get_status_string(self, spinner_char, depth=0, use_color=True): lines.append("") return '\n'.join(lines) + def get_stream_string(self, timestamp, use_color=True): + """Returns the resource information formatted for stream/text-only display. + + Args: + timestamp (str): Timestamp string to prefix the output + use_color (bool): Whether to use ANSI color codes (default: True) + + Returns: + str: Formatted string with timestamp prefix and bracket-enclosed status + """ + lines = [] + parts = [f"[{timestamp}]"] + + if self.resource_type: + parts.append( + self.color_utils.make_cyan(self.resource_type, use_color) + ) + + if self.identifier: + colored_id = self.color_utils.color_by_status( + self.identifier, self.status, use_color + ) + parts.append(colored_id) + + if self.status: + status_text = self.color_utils.color_by_status( + self.status, self.status, use_color + ) + parts.append(f"[{status_text}]") + + lines.append(" ".join(parts)) + + if self.reason: + lines.append(f" Reason: {self.reason}") + + if self.updated_at: + updated_time = datetime.fromtimestamp(self.updated_at).strftime( + "%Y-%m-%d %H:%M:%S" + ) + lines.append(f" Last Updated At: {updated_time}") + + if self.additional_info: + lines.append(f" Info: {self.additional_info}") + + return "\n".join(lines) + def combine(self, other_resource): """Returns the version of the resource which has the most up to date timestamp. @@ -130,22 +176,28 @@ def combine(self, other_resource): else other_resource ) - def diff(self, other_resource): - """Returns a tuple of (self_diff, other_diff) for resources that are different. + def compare_properties(self, other_resource): + """Compares individual resource properties to detect changes. + + This compares properties like status, reason, updated_at, additional_info + to detect if a resource has changed between polls. Args: other_resource (ManagedResource): Resource to compare against Returns: - tuple: (self_diff, other_diff) where: - - self_diff (ManagedResource): This resource if different, None if same - - other_diff (ManagedResource): Other resource if different, None if same + bool: True if properties differ, False if same """ if not other_resource: - return (self, None) - if ( + # No previous resource means it's new/different + return True + + # Resources are different if any field differs + return ( self.resource_type != other_resource.resource_type or self.identifier != other_resource.identifier - ): - return (self, other_resource) - return (None, None) + or self.status != other_resource.status + or self.reason != other_resource.reason + or self.updated_at != other_resource.updated_at + or self.additional_info != other_resource.additional_info + ) diff --git a/awscli/customizations/ecs/expressgateway/managedresourcegroup.py b/awscli/customizations/ecs/expressgateway/managedresourcegroup.py index b5643bc3b355..1bf3080cd6b2 100644 --- a/awscli/customizations/ecs/expressgateway/managedresourcegroup.py +++ b/awscli/customizations/ecs/expressgateway/managedresourcegroup.py @@ -39,7 +39,7 @@ def __init__( ): self.resource_type = resource_type self.identifier = identifier - # maintain input ordering + # Maintain input ordering self.sorted_resource_keys = [ self._create_key(resource) for resource in resources ] @@ -57,6 +57,86 @@ def _create_key(self, resource): identifier = resource.identifier if resource.identifier else "" return resource_type + "/" + identifier + def get_stream_string(self, timestamp, use_color=True): + """Returns flattened stream strings for all resources in the group. + + Args: + timestamp (str): Timestamp string to prefix each resource + use_color (bool): Whether to use ANSI color codes (default: True) + + Returns: + str: All flattened resources formatted for stream display, separated by newlines + """ + flat_resources = [] + + for resource in self.resource_mapping.values(): + if isinstance(resource, ManagedResourceGroup): + # Recursively flatten nested groups + nested = resource.get_stream_string(timestamp, use_color) + if nested: + flat_resources.append(nested) + elif isinstance(resource, ManagedResource): + # Get stream string for individual resource + flat_resources.append( + resource.get_stream_string(timestamp, use_color) + ) + + return "\n".join(flat_resources) + + def get_changed_resources(self, previous_resources_dict): + """Get flattened list of resources that have changed properties. + + Compares individual resource properties (status, reason, updated_at, etc.) + against previous state to detect changes. This is used for change detection + in TEXT-ONLY mode, NOT for DEPLOYMENT diff (use compare_resource_sets for that). + + Args: + previous_resources_dict: Dict of {(resource_type, identifier): ManagedResource} + from previous poll. Can be empty dict for first poll. + + Returns: + tuple: (changed_resources, updated_dict, removed_keys) + - changed_resources: List of ManagedResource that changed or None if no changes + - updated_dict: Updated dict with current resources for next comparison + - removed_keys: Set of keys that were removed since last poll + """ + current_resources = self._flatten_to_list() + changed_resources = [] + updated_dict = {} + + for resource in current_resources: + resource_key = (resource.resource_type, resource.identifier) + previous_resource = previous_resources_dict.get(resource_key) + + if not previous_resource: + changed_resources.append(resource) + else: + if resource.compare_properties(previous_resource): + changed_resources.append(resource) + + updated_dict[resource_key] = resource + + current_keys = { + (r.resource_type, r.identifier) for r in current_resources + } + removed_keys = set(previous_resources_dict.keys()) - current_keys + + return ( + changed_resources if changed_resources else None, + updated_dict, + removed_keys, + ) + + def _flatten_to_list(self): + """Flatten this resource group into a list of individual resources.""" + flat_list = [] + for resource in self.resource_mapping.values(): + if isinstance(resource, ManagedResourceGroup): + flat_list.extend(resource._flatten_to_list()) + elif isinstance(resource, ManagedResource): + flat_list.append(resource) + return flat_list + def is_terminal(self): return not self.resource_mapping or all( [ @@ -188,8 +268,12 @@ def _combine_child_resources(self, resource_a, resource_b): else: return resource_b - def diff(self, other_resource_group): - """Returns two ManagedResourceGroups representing unique resources in each group. + def compare_resource_sets(self, other_resource_group): + """Compares resource SETS between two groups to find additions/removals. + + This is used for DEPLOYMENT view to show which resources were added or removed + between service configurations, NOT for detecting property changes within + individual resources (that's compare_properties() in ManagedResource). Args: other_resource_group (ManagedResourceGroup): Resource group to compare against @@ -218,7 +302,7 @@ def diff(self, other_resource_group): common_keys = self_keys & other_keys common_diff = { - key: self.resource_mapping[key].diff( + key: self.resource_mapping[key].compare_resource_sets( other_resource_group.resource_mapping.get(key) ) for key in common_keys diff --git a/awscli/customizations/ecs/expressgateway/stream_display.py b/awscli/customizations/ecs/expressgateway/stream_display.py new file mode 100644 index 000000000000..626ee17f099c --- /dev/null +++ b/awscli/customizations/ecs/expressgateway/stream_display.py @@ -0,0 +1,110 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +"""Stream display implementation for ECS Express Gateway Service monitoring.""" + +import time + +from awscli.customizations.ecs.expressgateway.managedresourcegroup import ( + ManagedResourceGroup, +) +from awscli.customizations.utils import uni_print + + +class StreamDisplay: + """Stream display for monitoring that outputs changes to stdout. + + Provides text-based monitoring output suitable for non-interactive + environments, logging, or piping to other commands. + """ + + def __init__(self, use_color=True): + self.previous_resources_by_key = {} + self.use_color = use_color + + def show_startup_message(self): + """Show startup message.""" + timestamp = self._get_timestamp() + uni_print(f"[{timestamp}] Starting monitoring...\n") + + def show_polling_message(self): + """Show polling message.""" + timestamp = self._get_timestamp() + uni_print(f"[{timestamp}] Polling for updates...\n") + + def show_monitoring_data(self, resource_group, info): + """Show monitoring data for resources with diff detection. + + Args: + resource_group: ManagedResourceGroup or None + info: Additional info text to display + """ + timestamp = self._get_timestamp() + + if resource_group: + ( + changed_resources, + updated_dict, + removed_keys, + ) = resource_group.get_changed_resources( + self.previous_resources_by_key + ) + self.previous_resources_by_key = updated_dict + + if changed_resources: + self._print_flattened_resources_list( + changed_resources, timestamp + ) + + if info: + uni_print(f"[{timestamp}] {info}\n") + + def _print_flattened_resources_list(self, resources_list, timestamp): + """Print individual resources from a flat list as timestamped lines. + + Args: + resources_list: List of ManagedResource objects to print + timestamp: Timestamp string to prefix each line + """ + for resource in resources_list: + output = resource.get_stream_string(timestamp, self.use_color) + uni_print(output + "\n") + + def show_timeout_message(self): + """Show timeout message.""" + timestamp = self._get_timestamp() + uni_print(f"[{timestamp}] Monitoring timeout reached!\n") + + def show_service_inactive_message(self): + """Show service inactive message.""" + timestamp = self._get_timestamp() + uni_print(f"[{timestamp}] Service is inactive\n") + + def show_completion_message(self): + """Show completion message.""" + timestamp = self._get_timestamp() + uni_print(f"[{timestamp}] Monitoring complete!\n") + + def show_user_stop_message(self): + """Show user stop message.""" + timestamp = self._get_timestamp() + uni_print(f"[{timestamp}] Monitoring stopped by user\n") + + def show_error_message(self, error): + """Show error message.""" + timestamp = self._get_timestamp() + uni_print(f"[{timestamp}] Error: {error}\n") + + def _get_timestamp(self): + """Get formatted timestamp.""" + return time.strftime("%Y-%m-%d %H:%M:%S") diff --git a/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py b/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py index f6d99e6bf6f8..e69de29bb2d1 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py +++ b/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py @@ -1,288 +0,0 @@ -import unittest - -from awscli.customizations.ecs.expressgateway.managedresource import ( - ManagedResource, -) -from awscli.customizations.ecs.expressgateway.managedresourcegroup import ( - ManagedResourceGroup, -) - - -class TestManagedResourceGroup(unittest.TestCase): - def setUp(self): - self.resource1 = ManagedResource( - "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 - ) - self.resource2 = ManagedResource( - "Certificate", "cert-456", "PROVISIONING", 1761230543.151 - ) - - def test_is_terminal_all_terminal(self): - terminal_resource = ManagedResource( - "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 - ) - group = ManagedResourceGroup(resources=[terminal_resource]) - self.assertTrue(group.is_terminal()) - - def test_is_terminal_mixed(self): - group = ManagedResourceGroup( - resources=[self.resource1, self.resource2] - ) - self.assertFalse(group.is_terminal()) - - def test_is_terminal_empty(self): - group = ManagedResourceGroup() - self.assertTrue(group.is_terminal()) - - def test_get_status_string_with_header(self): - group = ManagedResourceGroup( - resource_type="IngressPaths", - identifier="endpoint-1", - resources=[self.resource1], - ) - status_string = group.get_status_string("⠋") - self.assertIn("IngressPaths", status_string) - self.assertIn("endpoint-1", status_string) - - def test_create_key(self): - group = ManagedResourceGroup() - key = group._create_key(self.resource1) - self.assertEqual(key, "LoadBalancer/lb-123") - - def test_get_status_string_empty_group(self): - group = ManagedResourceGroup(resource_type="EmptyGroup", resources=[]) - status_string = group.get_status_string("⠋") - self.assertIn("EmptyGroup", status_string) - self.assertIn("", status_string) - - def test_combine_resource_groups(self): - group1 = ManagedResourceGroup(resources=[self.resource1]) - group2 = ManagedResourceGroup(resources=[self.resource2]) - combined = group1.combine(group2) - self.assertEqual(len(combined.resource_mapping), 2) - - def test_combine_child_resources_both_none(self): - group = ManagedResourceGroup() - result = group._combine_child_resources(None, None) - self.assertIsNone(result) - - def test_combine_child_resources_first_none(self): - group = ManagedResourceGroup() - resource = ManagedResource("LoadBalancer", "lb-123", "ACTIVE") - result = group._combine_child_resources(None, resource) - self.assertEqual(result, resource) - - def test_combine_overlapping_resources(self): - older_resource = ManagedResource( - "LoadBalancer", "lb-123", "PROVISIONING", 1761230543.151 - ) - newer_resource = ManagedResource( - "LoadBalancer", "lb-123", "ACTIVE", 1761230600.151 - ) - - group1 = ManagedResourceGroup(resources=[older_resource]) - group2 = ManagedResourceGroup(resources=[newer_resource]) - - combined = group1.combine(group2) - - key = "LoadBalancer/lb-123" - self.assertIn(key, combined.resource_mapping) - self.assertEqual(combined.resource_mapping[key].status, "ACTIVE") - - def test_create_key_with_none_values(self): - group = ManagedResourceGroup() - resource = ManagedResource(None, None) - key = group._create_key(resource) - self.assertEqual(key, "/") - - def test_create_key_partial_none(self): - group = ManagedResourceGroup() - - resource1 = ManagedResource(None, "identifier") - key1 = group._create_key(resource1) - self.assertEqual(key1, "/identifier") - - resource2 = ManagedResource("ResourceType", None) - key2 = group._create_key(resource2) - self.assertEqual(key2, "ResourceType/") - - def test_diff_unique_resources(self): - # Test diff with completely different resources - group1 = ManagedResourceGroup( - resources=[self.resource1] - ) # LoadBalancer - group2 = ManagedResourceGroup( - resources=[self.resource2] - ) # Certificate - - diff1, diff2 = group1.diff(group2) - - # Each group should contain its unique resource - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertEqual(len(diff2.resource_mapping), 1) - self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) - self.assertIn("Certificate/cert-456", diff2.resource_mapping) - - def test_diff_overlapping_resources(self): - # Test diff with same resource type but different identifiers - resource3 = ManagedResource( - "LoadBalancer", "lb-456", "FAILED", 1761230600.151 - ) - group1 = ManagedResourceGroup( - resources=[self.resource1, self.resource2] - ) # lb-123, cert-456 - group2 = ManagedResourceGroup( - resources=[self.resource2, resource3] - ) # cert-456, lb-456 - - diff1, diff2 = group1.diff(group2) - - # group1 unique: lb-123, group2 unique: lb-456, common: cert-456 (should not appear in diff) - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertEqual(len(diff2.resource_mapping), 1) - self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) - self.assertIn("LoadBalancer/lb-456", diff2.resource_mapping) - # Common resource should not be in either diff - self.assertNotIn("Certificate/cert-456", diff1.resource_mapping) - self.assertNotIn("Certificate/cert-456", diff2.resource_mapping) - - def test_diff_identical_groups(self): - # Test diff with identical resource groups - group1 = ManagedResourceGroup( - resources=[self.resource1, self.resource2] - ) - group2 = ManagedResourceGroup( - resources=[self.resource1, self.resource2] - ) - - diff1, diff2 = group1.diff(group2) - - # No differences should be found - self.assertEqual(len(diff1.resource_mapping), 0) - self.assertEqual(len(diff2.resource_mapping), 0) - - def test_diff_empty_groups(self): - # Test diff with empty groups - group1 = ManagedResourceGroup(resources=[self.resource1]) - group2 = ManagedResourceGroup(resources=[]) - - diff1, diff2 = group1.diff(group2) - - # group1 should contain its resource, group2 should be empty - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertEqual(len(diff2.resource_mapping), 0) - self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) - - def test_diff_excludes_matching_types_without_identifier(self): - # Test that resources in other are excluded if self has same type without identifier - resource_without_id = ManagedResource("LoadBalancer", None) - resource_with_id = ManagedResource("LoadBalancer", "lb-456") - - group1 = ManagedResourceGroup( - resources=[resource_without_id] - ) # LoadBalancer/ - group2 = ManagedResourceGroup( - resources=[resource_with_id] - ) # LoadBalancer/lb-456 - - diff1, diff2 = group1.diff(group2) - - # group1 should contain its resource without identifier - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertIn("LoadBalancer/", diff1.resource_mapping) - - # group2 should be empty because LoadBalancer/lb-456 is excluded by LoadBalancer/ - self.assertEqual(len(diff2.resource_mapping), 0) - - def test_get_status_string_with_status(self): - group = ManagedResourceGroup( - resource_type="IngressPaths", identifier="test-id", status="ACTIVE" - ) - result = group.get_status_string("⠋") - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertIn("✓", result) # Green checkmark for ACTIVE - self.assertIn("ACTIVE", result) - - def test_get_status_string_without_status(self): - group = ManagedResourceGroup( - resource_type="IngressPaths", identifier="test-id" - ) - result = group.get_status_string("⠋") - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertNotIn("✓", result) # No symbol when no status - self.assertNotIn("ACTIVE", result) - - def test_get_status_string_status_without_identifier(self): - group = ManagedResourceGroup( - resource_type="IngressPaths", status="FAILED" - ) - result = group.get_status_string("⠋") - self.assertIn("IngressPaths", result) - self.assertIn("X", result) # Red X for FAILED - self.assertIn("FAILED", result) - - def test_get_status_string_no_color(self): - group = ManagedResourceGroup( - resource_type="IngressPaths", - identifier="test-id", - status="ACTIVE", - resources=[self.resource1], - ) - result = group.get_status_string("⠋", use_color=False) - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertIn("✓", result) # Checkmark should still be there - self.assertIn("ACTIVE", result) - # Should not contain ANSI color codes - self.assertNotIn("\x1b[", result) - - def test_get_status_string_with_color(self): - group = ManagedResourceGroup( - resource_type="IngressPaths", - identifier="test-id", - status="ACTIVE", - resources=[self.resource1], - ) - result = group.get_status_string("⠋", use_color=True) - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertIn("✓", result) # Checkmark should be there - self.assertIn("ACTIVE", result) - # Should contain ANSI color codes - self.assertIn("\x1b[", result) - - def test_combine_prioritizes_resources_with_identifier(self): - from awscli.customizations.ecs.expressgateway.managedresource import ( - ManagedResource, - ) - - resource_with_id = ManagedResource( - "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 - ) - resource_without_id = ManagedResource( - "LoadBalancer", - None, - "PROVISIONING", - 1761230600.151, # newer timestamp - ) - - group1 = ManagedResourceGroup( - resource_type="Service", resources=[resource_with_id] - ) - group2 = ManagedResourceGroup( - resource_type="Service", resources=[resource_without_id] - ) - - # Should prefer the one with identifier despite older timestamp - result = group1.combine(group2) - - # Should only have the resource with identifier - self.assertEqual(len(result.resource_mapping), 1) - combined_resource = list(result.resource_mapping.values())[0] - self.assertEqual(combined_resource.identifier, "lb-123") - - -if __name__ == '__main__': - unittest.main() diff --git a/tests/unit/customizations/ecs/expressgateway/test_stream_display.py b/tests/unit/customizations/ecs/expressgateway/test_stream_display.py new file mode 100644 index 000000000000..9c8217513f0c --- /dev/null +++ b/tests/unit/customizations/ecs/expressgateway/test_stream_display.py @@ -0,0 +1,309 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ + +import time + +from awscli.customizations.ecs.expressgateway.managedresource import ( + ManagedResource, +) +from awscli.customizations.ecs.expressgateway.managedresourcegroup import ( + ManagedResourceGroup, +) +from awscli.customizations.ecs.expressgateway.stream_display import ( + StreamDisplay, +) + + +class TestStreamDisplay: + """Test StreamDisplay for text-based monitoring output.""" + + def setup_method(self): + self.display = StreamDisplay(use_color=True) + + def test_show_startup_message(self, capsys): + """Test startup message includes timestamp""" + self.display.show_startup_message() + + output = capsys.readouterr().out + assert "Starting monitoring..." in output + assert "[" in output + + def test_show_polling_message(self, capsys): + """Test polling message includes timestamp""" + self.display.show_polling_message() + + output = capsys.readouterr().out + assert "Polling for updates..." in output + + def test_show_monitoring_data_with_info(self, capsys): + """Test showing info message""" + self.display.show_monitoring_data(None, "Info message") + + output = capsys.readouterr().out + assert "Info message" in output and output.endswith("\n") + + def test_show_monitoring_data_first_poll_shows_all(self, capsys): + """Test first poll shows all resources""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", None, None + ) + resource_group = ManagedResourceGroup(resources=[resource]) + + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert "LoadBalancer" in output + assert "lb-123" in output + + def test_show_monitoring_data_no_changes(self, capsys): + """Test no output when resources haven't changed""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", None, None + ) + resource_group = ManagedResourceGroup(resources=[resource]) + + # First poll - show all + self.display.show_monitoring_data(resource_group, None) + capsys.readouterr() + + # Second poll - same resources, no changes + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert output == "" + + def test_show_monitoring_data_with_new_resource(self, capsys): + """Test output when new resources are added""" + resource1 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", None, None + ) + resource_group1 = ManagedResourceGroup(resources=[resource1]) + + self.display.show_monitoring_data(resource_group1, None) + capsys.readouterr() + + # Second resource group with additional resource + resource2 = ManagedResource( + "TargetGroup", "tg-456", "ACTIVE", None, None + ) + resource_group2 = ManagedResourceGroup( + resources=[resource1, resource2] + ) + + self.display.show_monitoring_data(resource_group2, None) + + output = capsys.readouterr().out + assert "TargetGroup" in output + + def test_show_timeout_message(self, capsys): + """Test timeout message""" + self.display.show_timeout_message() + + output = capsys.readouterr().out + assert "timeout reached" in output.lower() + + def test_show_service_inactive_message(self, capsys): + """Test service inactive message""" + self.display.show_service_inactive_message() + + output = capsys.readouterr().out + assert "inactive" in output.lower() + + def test_show_completion_message(self, capsys): + """Test completion message""" + self.display.show_completion_message() + + output = capsys.readouterr().out + assert "complete" in output.lower() + + def test_show_user_stop_message(self, capsys): + """Test user stop message""" + self.display.show_user_stop_message() + + output = capsys.readouterr().out + assert "stopped by user" in output.lower() + + def test_show_error_message(self, capsys): + """Test error message""" + self.display.show_error_message("Test error") + + output = capsys.readouterr().out + assert "Error" in output + assert "Test error" in output + + def test_use_color_parameter(self): + """Test use_color parameter is stored""" + display_with_color = StreamDisplay(use_color=True) + assert display_with_color.use_color is True + + display_no_color = StreamDisplay(use_color=False) + assert display_no_color.use_color is False + + def test_print_flattened_resources_with_reason(self, capsys): + """Test resource with reason prints on separate line""" + resource = ManagedResource( + "LoadBalancer", + "lb-123", + "CREATING", + None, + "Waiting for DNS propagation", + ) + resource_group = ManagedResourceGroup(resources=[resource]) + + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert "LoadBalancer" in output + assert "Reason: Waiting for DNS propagation" in output + + def test_print_flattened_resources_with_updated_at(self, capsys): + """Test resource with updated_at timestamp prints on separate line""" + current_time = time.time() + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", current_time, None + ) + resource_group = ManagedResourceGroup(resources=[resource]) + + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert "LoadBalancer" in output + assert "Last Updated At:" in output + + def test_print_flattened_resources_with_additional_info(self, capsys): + """Test resource with additional_info prints on separate line""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", None, None + ) + resource.additional_info = "DNS: example.elb.amazonaws.com" + resource_group = ManagedResourceGroup(resources=[resource]) + + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert "LoadBalancer" in output + assert "Info: DNS: example.elb.amazonaws.com" in output + + def test_print_flattened_resources_complete_multi_line(self, capsys): + """Test resource with all fields prints on multiple lines""" + resource = ManagedResource( + "LoadBalancer", + "lb-123", + "CREATING", + time.time(), + "Provisioning load balancer", + ) + resource.additional_info = "Type: application" + resource_group = ManagedResourceGroup(resources=[resource]) + + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert "LoadBalancer" in output and "lb-123" in output + assert "Reason: Provisioning load balancer" in output + assert "Last Updated At:" in output + assert "Info: Type: application" in output + + def test_diff_detects_status_change(self, capsys): + """Test diff detects when status changes""" + resource1 = ManagedResource( + "LoadBalancer", "lb-123", "CREATING", None, None + ) + resource_group1 = ManagedResourceGroup(resources=[resource1]) + + # First poll + self.display.show_monitoring_data(resource_group1, None) + capsys.readouterr() + + # Second poll - same resource but status changed + resource2 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", None, None + ) + resource_group2 = ManagedResourceGroup(resources=[resource2]) + + self.display.show_monitoring_data(resource_group2, None) + + output = capsys.readouterr().out + assert "LoadBalancer" in output + assert "ACTIVE" in output + + def test_diff_detects_reason_change(self, capsys): + """Test diff detects when reason changes""" + resource1 = ManagedResource( + "LoadBalancer", "lb-123", "CREATING", None, "Creating resources" + ) + resource_group1 = ManagedResourceGroup(resources=[resource1]) + + # First poll + self.display.show_monitoring_data(resource_group1, None) + capsys.readouterr() + + # Second poll - same resource but reason changed + resource2 = ManagedResource( + "LoadBalancer", "lb-123", "CREATING", None, "Waiting for DNS" + ) + resource_group2 = ManagedResourceGroup(resources=[resource2]) + + self.display.show_monitoring_data(resource_group2, None) + + output = capsys.readouterr().out + assert "Waiting for DNS" in output + + def test_diff_detects_additional_info_change(self, capsys): + """Test diff detects when additional_info changes""" + resource1 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", None, None + ) + resource1.additional_info = "DNS: old.example.com" + resource_group1 = ManagedResourceGroup(resources=[resource1]) + + # First poll + self.display.show_monitoring_data(resource_group1, None) + capsys.readouterr() + + # Second poll - same resource but additional_info changed + resource2 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", None, None + ) + resource2.additional_info = "DNS: new.example.com" + resource_group2 = ManagedResourceGroup(resources=[resource2]) + + self.display.show_monitoring_data(resource_group2, None) + + output = capsys.readouterr().out + assert "new.example.com" in output + + def test_resource_with_none_type_shows_identifier(self, capsys): + """Test resources with resource_type=None show identifier without type""" + resource = ManagedResource( + None, + "mystery-resource-123", + "FAILED", + reason="Something went wrong", + ) + resource_group = ManagedResourceGroup( + resource_type="TestGroup", resources=[resource] + ) + + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert "mystery-resource-123" in output + assert "FAILED" in output + + def test_resource_with_none_type_and_none_identifier(self, capsys): + """Test resources with both resource_type=None and identifier=None show only status""" + resource = ManagedResource(None, None, "ACTIVE") + resource_group = ManagedResourceGroup( + resource_type="TestGroup", resources=[resource] + ) + + self.display.show_monitoring_data(resource_group, None) + + output = capsys.readouterr().out + assert "ACTIVE" in output From b281b0b682e8384978c50c6a5056dde1e80f18b8 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Mon, 15 Dec 2025 10:57:34 -0500 Subject: [PATCH 04/17] Addressing PR comments --- .../ecs/expressgateway/managedresource.py | 20 +- .../ecs/expressgateway/stream_display.py | 1 + .../expressgateway/test_managedresource.py | 119 ++++ .../test_managedresourcegroup.py | 512 ++++++++++++++++++ .../ecs/expressgateway/test_stream_display.py | 207 ++++--- 5 files changed, 788 insertions(+), 71 deletions(-) diff --git a/awscli/customizations/ecs/expressgateway/managedresource.py b/awscli/customizations/ecs/expressgateway/managedresource.py index 23581c92a5c6..292642066491 100644 --- a/awscli/customizations/ecs/expressgateway/managedresource.py +++ b/awscli/customizations/ecs/expressgateway/managedresource.py @@ -126,16 +126,22 @@ def get_stream_string(self, timestamp, use_color=True): lines = [] parts = [f"[{timestamp}]"] - if self.resource_type: + # If both resource_type and identifier are None, show a placeholder + if not self.resource_type and not self.identifier: parts.append( - self.color_utils.make_cyan(self.resource_type, use_color) + self.color_utils.make_cyan("Unknown Resource", use_color) ) + else: + if self.resource_type: + parts.append( + self.color_utils.make_cyan(self.resource_type, use_color) + ) - if self.identifier: - colored_id = self.color_utils.color_by_status( - self.identifier, self.status, use_color - ) - parts.append(colored_id) + if self.identifier: + colored_id = self.color_utils.color_by_status( + self.identifier, self.status, use_color + ) + parts.append(colored_id) if self.status: status_text = self.color_utils.color_by_status( diff --git a/awscli/customizations/ecs/expressgateway/stream_display.py b/awscli/customizations/ecs/expressgateway/stream_display.py index 626ee17f099c..7beb26b0ade2 100644 --- a/awscli/customizations/ecs/expressgateway/stream_display.py +++ b/awscli/customizations/ecs/expressgateway/stream_display.py @@ -59,6 +59,7 @@ def show_monitoring_data(self, resource_group, info): ) = resource_group.get_changed_resources( self.previous_resources_by_key ) + self.previous_resources_by_key = updated_dict if changed_resources: diff --git a/tests/unit/customizations/ecs/expressgateway/test_managedresource.py b/tests/unit/customizations/ecs/expressgateway/test_managedresource.py index db8157e066a2..dc0d6372b1a8 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_managedresource.py +++ b/tests/unit/customizations/ecs/expressgateway/test_managedresource.py @@ -175,6 +175,125 @@ def test_get_status_string_with_color(self): # Should contain ANSI color codes self.assertIn("\x1b[", status_string) + def test_get_stream_string_basic(self): + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("[2025-12-15 10:00:00]", stream_string) + self.assertIn("LoadBalancer", stream_string) + self.assertIn("lb-123", stream_string) + self.assertIn("ACTIVE", stream_string) + + def test_get_stream_string_with_reason(self): + resource = ManagedResource( + "LoadBalancer", + "lb-123", + "PROVISIONING", + 1761230543.151, + "Waiting for DNS propagation", + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("Reason: Waiting for DNS propagation", stream_string) + + def test_get_stream_string_with_additional_info(self): + resource = ManagedResource( + "LoadBalancer", + "lb-123", + "ACTIVE", + 1761230543.151, + additional_info="DNS: example.elb.amazonaws.com", + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("Info: DNS: example.elb.amazonaws.com", stream_string) + + def test_get_stream_string_with_updated_at(self): + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("Last Updated At:", stream_string) + # Check timestamp format YYYY-MM-DD HH:MM:SS + self.assertRegex(stream_string, r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}") + + def test_get_stream_string_all_fields(self): + resource = ManagedResource( + "TargetGroup", + "tg-456", + "PROVISIONING", + 1761230543.151, + "Registering targets", + "Health check interval: 30s", + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("[2025-12-15 10:00:00]", stream_string) + self.assertIn("TargetGroup", stream_string) + self.assertIn("tg-456", stream_string) + self.assertIn("PROVISIONING", stream_string) + self.assertIn("Reason: Registering targets", stream_string) + self.assertIn("Last Updated At:", stream_string) + self.assertIn("Info: Health check interval: 30s", stream_string) + + def test_get_stream_string_no_identifier(self): + resource = ManagedResource( + "LoadBalancer", None, "ACTIVE", 1761230543.151 + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("[2025-12-15 10:00:00]", stream_string) + self.assertIn("LoadBalancer", stream_string) + self.assertIn("ACTIVE", stream_string) + self.assertNotIn("None", stream_string) + + def test_get_stream_string_no_status(self): + resource = ManagedResource( + "LoadBalancer", "lb-123", None, 1761230543.151 + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("LoadBalancer", stream_string) + self.assertIn("lb-123", stream_string) + # Should not have status brackets when status is None + self.assertNotIn("[None]", stream_string) + + def test_get_stream_string_no_color(self): + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + stream_string = resource.get_stream_string( + "2025-12-15 10:00:00", use_color=False + ) + self.assertIn("LoadBalancer", stream_string) + self.assertIn("lb-123", stream_string) + self.assertIn("ACTIVE", stream_string) + # Should not contain ANSI color codes + self.assertNotIn("\x1b[", stream_string) + + def test_get_stream_string_with_color(self): + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + stream_string = resource.get_stream_string( + "2025-12-15 10:00:00", use_color=True + ) + self.assertIn("LoadBalancer", stream_string) + self.assertIn("lb-123", stream_string) + self.assertIn("ACTIVE", stream_string) + # Should contain ANSI color codes + self.assertIn("\x1b[", stream_string) + + def test_get_stream_string_failed_status(self): + resource = ManagedResource( + "LoadBalancer", + "lb-123", + "FAILED", + 1761230543.151, + "Connection timeout", + ) + stream_string = resource.get_stream_string("2025-12-15 10:00:00") + self.assertIn("FAILED", stream_string) + self.assertIn("Reason: Connection timeout", stream_string) + # Failed status should use color coding + self.assertIn("\x1b[", stream_string) + class TestConstants(unittest.TestCase): def test_terminal_resource_statuses(self): diff --git a/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py b/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py index e69de29bb2d1..b9c9e6c56b30 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py +++ b/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py @@ -0,0 +1,512 @@ +import unittest + +from awscli.customizations.ecs.expressgateway.managedresource import ( + ManagedResource, +) +from awscli.customizations.ecs.expressgateway.managedresourcegroup import ( + ManagedResourceGroup, +) + + +class TestManagedResourceGroup(unittest.TestCase): + def setUp(self): + self.resource1 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + self.resource2 = ManagedResource( + "Certificate", "cert-456", "PROVISIONING", 1761230543.151 + ) + + def test_is_terminal_all_terminal(self): + terminal_resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + group = ManagedResourceGroup(resources=[terminal_resource]) + self.assertTrue(group.is_terminal()) + + def test_is_terminal_mixed(self): + group = ManagedResourceGroup( + resources=[self.resource1, self.resource2] + ) + self.assertFalse(group.is_terminal()) + + def test_is_terminal_empty(self): + group = ManagedResourceGroup() + self.assertTrue(group.is_terminal()) + + def test_get_status_string_with_header(self): + group = ManagedResourceGroup( + resource_type="IngressPaths", + identifier="endpoint-1", + resources=[self.resource1], + ) + status_string = group.get_status_string("⠋") + self.assertIn("IngressPaths", status_string) + self.assertIn("endpoint-1", status_string) + + def test_create_key(self): + group = ManagedResourceGroup() + key = group._create_key(self.resource1) + self.assertEqual(key, "LoadBalancer/lb-123") + + def test_get_status_string_empty_group(self): + group = ManagedResourceGroup(resource_type="EmptyGroup", resources=[]) + status_string = group.get_status_string("⠋") + self.assertIn("EmptyGroup", status_string) + self.assertIn("", status_string) + + def test_combine_resource_groups(self): + group1 = ManagedResourceGroup(resources=[self.resource1]) + group2 = ManagedResourceGroup(resources=[self.resource2]) + combined = group1.combine(group2) + self.assertEqual(len(combined.resource_mapping), 2) + + def test_combine_child_resources_both_none(self): + group = ManagedResourceGroup() + result = group._combine_child_resources(None, None) + self.assertIsNone(result) + + def test_combine_child_resources_first_none(self): + group = ManagedResourceGroup() + resource = ManagedResource("LoadBalancer", "lb-123", "ACTIVE") + result = group._combine_child_resources(None, resource) + self.assertEqual(result, resource) + + def test_combine_overlapping_resources(self): + older_resource = ManagedResource( + "LoadBalancer", "lb-123", "PROVISIONING", 1761230543.151 + ) + newer_resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230600.151 + ) + + group1 = ManagedResourceGroup(resources=[older_resource]) + group2 = ManagedResourceGroup(resources=[newer_resource]) + + combined = group1.combine(group2) + + key = "LoadBalancer/lb-123" + self.assertIn(key, combined.resource_mapping) + self.assertEqual(combined.resource_mapping[key].status, "ACTIVE") + + def test_create_key_with_none_values(self): + group = ManagedResourceGroup() + resource = ManagedResource(None, None) + key = group._create_key(resource) + self.assertEqual(key, "/") + + def test_create_key_partial_none(self): + group = ManagedResourceGroup() + + resource1 = ManagedResource(None, "identifier") + key1 = group._create_key(resource1) + self.assertEqual(key1, "/identifier") + + resource2 = ManagedResource("ResourceType", None) + key2 = group._create_key(resource2) + self.assertEqual(key2, "ResourceType/") + + def test_compare_resource_sets_unique_resources(self): + # Test compare_resource_sets with completely different resources + group1 = ManagedResourceGroup( + resources=[self.resource1] + ) # LoadBalancer + group2 = ManagedResourceGroup( + resources=[self.resource2] + ) # Certificate + + diff1, diff2 = group1.compare_resource_sets(group2) + + # Each group should contain its unique resource + self.assertEqual(len(diff1.resource_mapping), 1) + self.assertEqual(len(diff2.resource_mapping), 1) + self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) + self.assertIn("Certificate/cert-456", diff2.resource_mapping) + + def test_compare_resource_sets_overlapping_resources(self): + # Test compare_resource_sets with same resource type but different identifiers + resource3 = ManagedResource( + "LoadBalancer", "lb-456", "FAILED", 1761230600.151 + ) + group1 = ManagedResourceGroup( + resources=[self.resource1, self.resource2] + ) # lb-123, cert-456 + group2 = ManagedResourceGroup( + resources=[self.resource2, resource3] + ) # cert-456, lb-456 + + diff1, diff2 = group1.compare_resource_sets(group2) + + # group1 unique: lb-123, group2 unique: lb-456, common: cert-456 (should not appear in diff) + self.assertEqual(len(diff1.resource_mapping), 1) + self.assertEqual(len(diff2.resource_mapping), 1) + self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) + self.assertIn("LoadBalancer/lb-456", diff2.resource_mapping) + # Common resource should not be in either diff + self.assertNotIn("Certificate/cert-456", diff1.resource_mapping) + self.assertNotIn("Certificate/cert-456", diff2.resource_mapping) + + def test_compare_resource_sets_identical_groups(self): + # Test compare_resource_sets with identical resource groups + group1 = ManagedResourceGroup( + resources=[self.resource1, self.resource2] + ) + group2 = ManagedResourceGroup( + resources=[self.resource1, self.resource2] + ) + + diff1, diff2 = group1.compare_resource_sets(group2) + + # No differences should be found + self.assertEqual(len(diff1.resource_mapping), 0) + self.assertEqual(len(diff2.resource_mapping), 0) + + def test_compare_resource_sets_empty_groups(self): + # Test compare_resource_sets with empty groups + group1 = ManagedResourceGroup(resources=[self.resource1]) + group2 = ManagedResourceGroup(resources=[]) + + diff1, diff2 = group1.compare_resource_sets(group2) + + # group1 should contain its resource, group2 should be empty + self.assertEqual(len(diff1.resource_mapping), 1) + self.assertEqual(len(diff2.resource_mapping), 0) + self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) + + def test_compare_resource_sets_excludes_matching_types_without_identifier( + self, + ): + # Test that resources in other are excluded if self has same type without identifier + resource_without_id = ManagedResource("LoadBalancer", None) + resource_with_id = ManagedResource("LoadBalancer", "lb-456") + + group1 = ManagedResourceGroup( + resources=[resource_without_id] + ) # LoadBalancer/ + group2 = ManagedResourceGroup( + resources=[resource_with_id] + ) # LoadBalancer/lb-456 + + diff1, diff2 = group1.compare_resource_sets(group2) + + # group1 should contain its resource without identifier + self.assertEqual(len(diff1.resource_mapping), 1) + self.assertIn("LoadBalancer/", diff1.resource_mapping) + + # group2 should be empty because LoadBalancer/lb-456 is excluded by LoadBalancer/ + self.assertEqual(len(diff2.resource_mapping), 0) + + def test_get_status_string_with_status(self): + group = ManagedResourceGroup( + resource_type="IngressPaths", identifier="test-id", status="ACTIVE" + ) + result = group.get_status_string("⠋") + self.assertIn("IngressPaths", result) + self.assertIn("test-id", result) + self.assertIn("✓", result) # Green checkmark for ACTIVE + self.assertIn("ACTIVE", result) + + def test_get_status_string_without_status(self): + group = ManagedResourceGroup( + resource_type="IngressPaths", identifier="test-id" + ) + result = group.get_status_string("⠋") + self.assertIn("IngressPaths", result) + self.assertIn("test-id", result) + self.assertNotIn("✓", result) # No symbol when no status + self.assertNotIn("ACTIVE", result) + + def test_get_status_string_status_without_identifier(self): + group = ManagedResourceGroup( + resource_type="IngressPaths", status="FAILED" + ) + result = group.get_status_string("⠋") + self.assertIn("IngressPaths", result) + self.assertIn("X", result) # Red X for FAILED + self.assertIn("FAILED", result) + + def test_get_status_string_no_color(self): + group = ManagedResourceGroup( + resource_type="IngressPaths", + identifier="test-id", + status="ACTIVE", + resources=[self.resource1], + ) + result = group.get_status_string("⠋", use_color=False) + self.assertIn("IngressPaths", result) + self.assertIn("test-id", result) + self.assertIn("✓", result) # Checkmark should still be there + self.assertIn("ACTIVE", result) + # Should not contain ANSI color codes + self.assertNotIn("\x1b[", result) + + def test_get_status_string_with_color(self): + group = ManagedResourceGroup( + resource_type="IngressPaths", + identifier="test-id", + status="ACTIVE", + resources=[self.resource1], + ) + result = group.get_status_string("⠋", use_color=True) + self.assertIn("IngressPaths", result) + self.assertIn("test-id", result) + self.assertIn("✓", result) # Checkmark should be there + self.assertIn("ACTIVE", result) + # Should contain ANSI color codes + self.assertIn("\x1b[", result) + + def test_combine_prioritizes_resources_with_identifier(self): + resource_with_id = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + resource_without_id = ManagedResource( + "LoadBalancer", + None, + "PROVISIONING", + 1761230600.151, # newer timestamp + ) + + group1 = ManagedResourceGroup( + resource_type="Service", resources=[resource_with_id] + ) + group2 = ManagedResourceGroup( + resource_type="Service", resources=[resource_without_id] + ) + + # Should prefer the one with identifier despite older timestamp + result = group1.combine(group2) + + # Should only have the resource with identifier + self.assertEqual(len(result.resource_mapping), 1) + combined_resource = list(result.resource_mapping.values())[0] + self.assertEqual(combined_resource.identifier, "lb-123") + + def test_get_stream_string_empty_group(self): + """Test empty resource group returns empty string""" + group = ManagedResourceGroup() + result = group.get_stream_string("2025-12-15 10:00:00") + self.assertEqual(result, "") + + def test_get_stream_string_single_resource(self): + """Test resource group with single resource""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + group = ManagedResourceGroup(resources=[resource]) + result = group.get_stream_string("2025-12-15 10:00:00") + self.assertIn("[2025-12-15 10:00:00]", result) + self.assertIn("LoadBalancer", result) + self.assertIn("lb-123", result) + self.assertIn("ACTIVE", result) + + def test_get_stream_string_multiple_resources(self): + """Test resource group with multiple resources""" + resource1 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + resource2 = ManagedResource( + "TargetGroup", "tg-456", "PROVISIONING", 1761230543.151 + ) + group = ManagedResourceGroup(resources=[resource1, resource2]) + result = group.get_stream_string("2025-12-15 10:00:00") + + # Should have both resources + self.assertIn("LoadBalancer", result) + self.assertIn("lb-123", result) + self.assertIn("TargetGroup", result) + self.assertIn("tg-456", result) + + # Should have newline between resources + lines = result.split("\n") + self.assertGreater(len(lines), 1) + + def test_get_stream_string_nested_groups(self): + """Test resource group with nested groups""" + resource1 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + resource2 = ManagedResource( + "TargetGroup", "tg-456", "ACTIVE", 1761230543.151 + ) + nested_group = ManagedResourceGroup(resources=[resource2]) + + group = ManagedResourceGroup(resources=[resource1, nested_group]) + result = group.get_stream_string("2025-12-15 10:00:00") + + # Should have flattened both resources + self.assertIn("LoadBalancer", result) + self.assertIn("lb-123", result) + self.assertIn("TargetGroup", result) + self.assertIn("tg-456", result) + + def test_get_stream_string_deeply_nested_groups(self): + """Test resource group with multiple levels of nesting""" + resource1 = ManagedResource( + "Cluster", "cluster-1", "ACTIVE", 1761230543.151 + ) + resource2 = ManagedResource( + "Service", "service-1", "ACTIVE", 1761230543.151 + ) + resource3 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + + nested_level2 = ManagedResourceGroup(resources=[resource3]) + nested_level1 = ManagedResourceGroup( + resources=[resource2, nested_level2] + ) + group = ManagedResourceGroup(resources=[resource1, nested_level1]) + + result = group.get_stream_string("2025-12-15 10:00:00") + + # Should have all resources flattened + self.assertIn("Cluster", result) + self.assertIn("Service", result) + self.assertIn("LoadBalancer", result) + + def test_get_stream_string_with_resource_details(self): + """Test that resource details are preserved in stream output""" + resource = ManagedResource( + "LoadBalancer", + "lb-123", + "PROVISIONING", + 1761230543.151, + "Waiting for DNS propagation", + "DNS: example.elb.amazonaws.com", + ) + group = ManagedResourceGroup(resources=[resource]) + result = group.get_stream_string("2025-12-15 10:00:00") + + self.assertIn("Reason: Waiting for DNS propagation", result) + self.assertIn("Info: DNS: example.elb.amazonaws.com", result) + self.assertIn("Last Updated At:", result) + + def test_get_stream_string_preserves_order(self): + """Test that resource order is preserved in output""" + resource1 = ManagedResource( + "Cluster", "cluster-1", "ACTIVE", 1761230543.151 + ) + resource2 = ManagedResource( + "Service", "service-1", "ACTIVE", 1761230543.151 + ) + resource3 = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + + group = ManagedResourceGroup( + resources=[resource1, resource2, resource3] + ) + result = group.get_stream_string("2025-12-15 10:00:00") + + # Find positions of each resource type in output + cluster_pos = result.find("Cluster") + service_pos = result.find("Service") + lb_pos = result.find("LoadBalancer") + + # Order should be preserved + self.assertLess(cluster_pos, service_pos) + self.assertLess(service_pos, lb_pos) + + def test_get_stream_string_no_color(self): + """Test stream string without color codes""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + group = ManagedResourceGroup(resources=[resource]) + result = group.get_stream_string( + "2025-12-15 10:00:00", use_color=False + ) + + self.assertIn("LoadBalancer", result) + self.assertIn("lb-123", result) + # Should not contain ANSI color codes + self.assertNotIn("\x1b[", result) + + def test_get_stream_string_with_color(self): + """Test stream string with color codes""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + group = ManagedResourceGroup(resources=[resource]) + result = group.get_stream_string("2025-12-15 10:00:00", use_color=True) + + self.assertIn("LoadBalancer", result) + self.assertIn("lb-123", result) + # Should contain ANSI color codes + self.assertIn("\x1b[", result) + + def test_get_stream_string_mixed_resource_types(self): + """Test group with various resource types and statuses""" + resources = [ + ManagedResource("Cluster", "cluster-1", "ACTIVE", 1761230543.151), + ManagedResource( + "Service", "service-1", "UPDATING", 1761230543.151 + ), + ManagedResource( + "LoadBalancer", + "lb-123", + "PROVISIONING", + 1761230543.151, + "Creating", + ), + ManagedResource( + "TargetGroup", "tg-456", "FAILED", 1761230543.151, "Error" + ), + ] + group = ManagedResourceGroup(resources=resources) + result = group.get_stream_string("2025-12-15 10:00:00") + + # All resources should be present + self.assertIn("Cluster", result) + self.assertIn("Service", result) + self.assertIn("LoadBalancer", result) + self.assertIn("TargetGroup", result) + + # All statuses should be present + self.assertIn("ACTIVE", result) + self.assertIn("UPDATING", result) + self.assertIn("PROVISIONING", result) + self.assertIn("FAILED", result) + + def test_get_stream_string_with_group_metadata(self): + """Test that group-level metadata doesn't affect stream output""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + group = ManagedResourceGroup( + resource_type="ManagedResources", + identifier="group-1", + resources=[resource], + status="ACTIVE", + reason="All resources healthy", + ) + result = group.get_stream_string("2025-12-15 10:00:00") + + # Should still show the actual resource, not group metadata + self.assertIn("LoadBalancer", result) + self.assertIn("lb-123", result) + + # Group metadata should not appear in stream output + # (stream output is for individual resources only) + self.assertNotIn("group-1", result) + + def test_get_stream_string_empty_nested_group(self): + """Test nested group that is empty is handled correctly""" + resource = ManagedResource( + "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 + ) + empty_nested = ManagedResourceGroup() + + group = ManagedResourceGroup(resources=[resource, empty_nested]) + result = group.get_stream_string("2025-12-15 10:00:00") + + # Should show the resource from non-empty group + self.assertIn("LoadBalancer", result) + # Empty nested group shouldn't add extra content + lines = [line for line in result.split("\n") if line.strip()] + # LoadBalancer resource produces multiple lines (timestamp line + optional detail lines) + self.assertGreater(len(lines), 0) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/unit/customizations/ecs/expressgateway/test_stream_display.py b/tests/unit/customizations/ecs/expressgateway/test_stream_display.py index 9c8217513f0c..4a86d3661ef7 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_stream_display.py +++ b/tests/unit/customizations/ecs/expressgateway/test_stream_display.py @@ -8,6 +8,8 @@ import time +import pytest + from awscli.customizations.ecs.expressgateway.managedresource import ( ManagedResource, ) @@ -19,48 +21,51 @@ ) +@pytest.fixture +def display(): + """Fixture that returns a StreamDisplay with color enabled.""" + return StreamDisplay(use_color=True) + + class TestStreamDisplay: """Test StreamDisplay for text-based monitoring output.""" - def setup_method(self): - self.display = StreamDisplay(use_color=True) - - def test_show_startup_message(self, capsys): + def test_show_startup_message(self, display, capsys): """Test startup message includes timestamp""" - self.display.show_startup_message() + display.show_startup_message() output = capsys.readouterr().out assert "Starting monitoring..." in output assert "[" in output - def test_show_polling_message(self, capsys): + def test_show_polling_message(self, display, capsys): """Test polling message includes timestamp""" - self.display.show_polling_message() + display.show_polling_message() output = capsys.readouterr().out assert "Polling for updates..." in output - def test_show_monitoring_data_with_info(self, capsys): + def test_show_monitoring_data_with_info(self, display, capsys): """Test showing info message""" - self.display.show_monitoring_data(None, "Info message") + display.show_monitoring_data(None, "Info message") output = capsys.readouterr().out assert "Info message" in output and output.endswith("\n") - def test_show_monitoring_data_first_poll_shows_all(self, capsys): + def test_show_monitoring_data_first_poll_shows_all(self, display, capsys): """Test first poll shows all resources""" resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", None, None ) resource_group = ManagedResourceGroup(resources=[resource]) - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out assert "LoadBalancer" in output assert "lb-123" in output - def test_show_monitoring_data_no_changes(self, capsys): + def test_show_monitoring_data_no_changes(self, display, capsys): """Test no output when resources haven't changed""" resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", None, None @@ -68,24 +73,24 @@ def test_show_monitoring_data_no_changes(self, capsys): resource_group = ManagedResourceGroup(resources=[resource]) # First poll - show all - self.display.show_monitoring_data(resource_group, None) - capsys.readouterr() + display.show_monitoring_data(resource_group, None) + capsys.readouterr() # Clear output to test second call # Second poll - same resources, no changes - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out assert output == "" - def test_show_monitoring_data_with_new_resource(self, capsys): + def test_show_monitoring_data_with_new_resource(self, display, capsys): """Test output when new resources are added""" resource1 = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", None, None ) resource_group1 = ManagedResourceGroup(resources=[resource1]) - self.display.show_monitoring_data(resource_group1, None) - capsys.readouterr() + display.show_monitoring_data(resource_group1, None) + capsys.readouterr() # Clear initial output to verify new resource shows # Second resource group with additional resource resource2 = ManagedResource( @@ -95,42 +100,42 @@ def test_show_monitoring_data_with_new_resource(self, capsys): resources=[resource1, resource2] ) - self.display.show_monitoring_data(resource_group2, None) + display.show_monitoring_data(resource_group2, None) output = capsys.readouterr().out assert "TargetGroup" in output - def test_show_timeout_message(self, capsys): + def test_show_timeout_message(self, display, capsys): """Test timeout message""" - self.display.show_timeout_message() + display.show_timeout_message() output = capsys.readouterr().out assert "timeout reached" in output.lower() - def test_show_service_inactive_message(self, capsys): + def test_show_service_inactive_message(self, display, capsys): """Test service inactive message""" - self.display.show_service_inactive_message() + display.show_service_inactive_message() output = capsys.readouterr().out assert "inactive" in output.lower() - def test_show_completion_message(self, capsys): + def test_show_completion_message(self, display, capsys): """Test completion message""" - self.display.show_completion_message() + display.show_completion_message() output = capsys.readouterr().out assert "complete" in output.lower() - def test_show_user_stop_message(self, capsys): + def test_show_user_stop_message(self, display, capsys): """Test user stop message""" - self.display.show_user_stop_message() + display.show_user_stop_message() output = capsys.readouterr().out assert "stopped by user" in output.lower() - def test_show_error_message(self, capsys): + def test_show_error_message(self, display, capsys): """Test error message""" - self.display.show_error_message("Test error") + display.show_error_message("Test error") output = capsys.readouterr().out assert "Error" in output @@ -144,7 +149,7 @@ def test_use_color_parameter(self): display_no_color = StreamDisplay(use_color=False) assert display_no_color.use_color is False - def test_print_flattened_resources_with_reason(self, capsys): + def test_print_flattened_resources_with_reason(self, display, capsys): """Test resource with reason prints on separate line""" resource = ManagedResource( "LoadBalancer", @@ -155,13 +160,27 @@ def test_print_flattened_resources_with_reason(self, capsys): ) resource_group = ManagedResourceGroup(resources=[resource]) - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out - assert "LoadBalancer" in output - assert "Reason: Waiting for DNS propagation" in output + lines = output.splitlines() - def test_print_flattened_resources_with_updated_at(self, capsys): + # Find the line with LoadBalancer + lb_line_idx = next( + i for i, line in enumerate(lines) if "LoadBalancer" in line + ) + reason_line_idx = next( + i + for i, line in enumerate(lines) + if "Reason: Waiting for DNS propagation" in line + ) + + # Reason should be on a different line than LoadBalancer + assert lb_line_idx != reason_line_idx + # Reason should come after the LoadBalancer line + assert reason_line_idx > lb_line_idx + + def test_print_flattened_resources_with_updated_at(self, display, capsys): """Test resource with updated_at timestamp prints on separate line""" current_time = time.time() resource = ManagedResource( @@ -169,13 +188,27 @@ def test_print_flattened_resources_with_updated_at(self, capsys): ) resource_group = ManagedResourceGroup(resources=[resource]) - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out - assert "LoadBalancer" in output - assert "Last Updated At:" in output + lines = output.splitlines() + + # Find the line with LoadBalancer + lb_line_idx = next( + i for i, line in enumerate(lines) if "LoadBalancer" in line + ) + updated_line_idx = next( + i for i, line in enumerate(lines) if "Last Updated At:" in line + ) - def test_print_flattened_resources_with_additional_info(self, capsys): + # Updated timestamp should be on a different line than LoadBalancer + assert lb_line_idx != updated_line_idx + # Updated timestamp should come after the LoadBalancer line + assert updated_line_idx > lb_line_idx + + def test_print_flattened_resources_with_additional_info( + self, display, capsys + ): """Test resource with additional_info prints on separate line""" resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", None, None @@ -183,13 +216,29 @@ def test_print_flattened_resources_with_additional_info(self, capsys): resource.additional_info = "DNS: example.elb.amazonaws.com" resource_group = ManagedResourceGroup(resources=[resource]) - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out - assert "LoadBalancer" in output - assert "Info: DNS: example.elb.amazonaws.com" in output + lines = output.splitlines() - def test_print_flattened_resources_complete_multi_line(self, capsys): + # Find the line with LoadBalancer + lb_line_idx = next( + i for i, line in enumerate(lines) if "LoadBalancer" in line + ) + info_line_idx = next( + i + for i, line in enumerate(lines) + if "Info: DNS: example.elb.amazonaws.com" in line + ) + + # Info should be on a different line than LoadBalancer + assert lb_line_idx != info_line_idx + # Info should come after the LoadBalancer line + assert info_line_idx > lb_line_idx + + def test_print_flattened_resources_complete_multi_line( + self, display, capsys + ): """Test resource with all fields prints on multiple lines""" resource = ManagedResource( "LoadBalancer", @@ -201,15 +250,42 @@ def test_print_flattened_resources_complete_multi_line(self, capsys): resource.additional_info = "Type: application" resource_group = ManagedResourceGroup(resources=[resource]) - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out - assert "LoadBalancer" in output and "lb-123" in output - assert "Reason: Provisioning load balancer" in output - assert "Last Updated At:" in output - assert "Info: Type: application" in output + lines = output.splitlines() + + # Find all the relevant lines + lb_line_idx = next( + i + for i, line in enumerate(lines) + if "LoadBalancer" in line and "lb-123" in line + ) + reason_line_idx = next( + i + for i, line in enumerate(lines) + if "Reason: Provisioning load balancer" in line + ) + updated_line_idx = next( + i for i, line in enumerate(lines) if "Last Updated At:" in line + ) + info_line_idx = next( + i + for i, line in enumerate(lines) + if "Info: Type: application" in line + ) + + # All detail lines should be different from the main line + assert reason_line_idx != lb_line_idx + assert updated_line_idx != lb_line_idx + assert info_line_idx != lb_line_idx + + # All detail lines should come after the main line + assert reason_line_idx > lb_line_idx + assert updated_line_idx > lb_line_idx + assert info_line_idx > lb_line_idx - def test_diff_detects_status_change(self, capsys): + def test_diff_detects_status_change(self, display, capsys): """Test diff detects when status changes""" resource1 = ManagedResource( "LoadBalancer", "lb-123", "CREATING", None, None @@ -217,8 +293,8 @@ def test_diff_detects_status_change(self, capsys): resource_group1 = ManagedResourceGroup(resources=[resource1]) # First poll - self.display.show_monitoring_data(resource_group1, None) - capsys.readouterr() + display.show_monitoring_data(resource_group1, None) + capsys.readouterr() # Clear initial output to test status change detection # Second poll - same resource but status changed resource2 = ManagedResource( @@ -226,13 +302,13 @@ def test_diff_detects_status_change(self, capsys): ) resource_group2 = ManagedResourceGroup(resources=[resource2]) - self.display.show_monitoring_data(resource_group2, None) + display.show_monitoring_data(resource_group2, None) output = capsys.readouterr().out assert "LoadBalancer" in output assert "ACTIVE" in output - def test_diff_detects_reason_change(self, capsys): + def test_diff_detects_reason_change(self, display, capsys): """Test diff detects when reason changes""" resource1 = ManagedResource( "LoadBalancer", "lb-123", "CREATING", None, "Creating resources" @@ -240,8 +316,8 @@ def test_diff_detects_reason_change(self, capsys): resource_group1 = ManagedResourceGroup(resources=[resource1]) # First poll - self.display.show_monitoring_data(resource_group1, None) - capsys.readouterr() + display.show_monitoring_data(resource_group1, None) + capsys.readouterr() # Clear initial output to test reason change detection # Second poll - same resource but reason changed resource2 = ManagedResource( @@ -249,12 +325,12 @@ def test_diff_detects_reason_change(self, capsys): ) resource_group2 = ManagedResourceGroup(resources=[resource2]) - self.display.show_monitoring_data(resource_group2, None) + display.show_monitoring_data(resource_group2, None) output = capsys.readouterr().out assert "Waiting for DNS" in output - def test_diff_detects_additional_info_change(self, capsys): + def test_diff_detects_additional_info_change(self, display, capsys): """Test diff detects when additional_info changes""" resource1 = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", None, None @@ -263,8 +339,8 @@ def test_diff_detects_additional_info_change(self, capsys): resource_group1 = ManagedResourceGroup(resources=[resource1]) # First poll - self.display.show_monitoring_data(resource_group1, None) - capsys.readouterr() + display.show_monitoring_data(resource_group1, None) + capsys.readouterr() # Clear initial output to test additional_info change detection # Second poll - same resource but additional_info changed resource2 = ManagedResource( @@ -273,12 +349,12 @@ def test_diff_detects_additional_info_change(self, capsys): resource2.additional_info = "DNS: new.example.com" resource_group2 = ManagedResourceGroup(resources=[resource2]) - self.display.show_monitoring_data(resource_group2, None) + display.show_monitoring_data(resource_group2, None) output = capsys.readouterr().out assert "new.example.com" in output - def test_resource_with_none_type_shows_identifier(self, capsys): + def test_resource_with_none_type_shows_identifier(self, display, capsys): """Test resources with resource_type=None show identifier without type""" resource = ManagedResource( None, @@ -290,20 +366,23 @@ def test_resource_with_none_type_shows_identifier(self, capsys): resource_type="TestGroup", resources=[resource] ) - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out assert "mystery-resource-123" in output assert "FAILED" in output - def test_resource_with_none_type_and_none_identifier(self, capsys): - """Test resources with both resource_type=None and identifier=None show only status""" + def test_resource_with_none_type_and_none_identifier( + self, display, capsys + ): + """Test resources with both resource_type=None and identifier=None show 'Unknown Resource' placeholder""" resource = ManagedResource(None, None, "ACTIVE") resource_group = ManagedResourceGroup( resource_type="TestGroup", resources=[resource] ) - self.display.show_monitoring_data(resource_group, None) + display.show_monitoring_data(resource_group, None) output = capsys.readouterr().out + assert "Unknown Resource" in output assert "ACTIVE" in output From da6f3ea28de1679b14634d9c62a5bca668d409c1 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Mon, 15 Dec 2025 15:47:55 -0500 Subject: [PATCH 05/17] Migrate ManagedResource(Group) tests to pytest --- .../expressgateway/test_managedresource.py | 144 ++++---- .../test_managedresourcegroup.py | 316 +++++++----------- 2 files changed, 196 insertions(+), 264 deletions(-) diff --git a/tests/unit/customizations/ecs/expressgateway/test_managedresource.py b/tests/unit/customizations/ecs/expressgateway/test_managedresource.py index dc0d6372b1a8..2b253d4e7317 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_managedresource.py +++ b/tests/unit/customizations/ecs/expressgateway/test_managedresource.py @@ -1,4 +1,6 @@ -import unittest +import re + +import pytest from awscli.customizations.ecs.expressgateway.managedresource import ( TERMINAL_RESOURCE_STATUSES, @@ -6,33 +8,33 @@ ) -class TestManagedResource(unittest.TestCase): +class TestManagedResource: def test_is_terminal_active(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) - self.assertTrue(resource.is_terminal()) + assert resource.is_terminal() def test_is_terminal_failed(self): resource = ManagedResource( "LoadBalancer", "lb-123", "FAILED", 1761230543.151 ) - self.assertTrue(resource.is_terminal()) + assert resource.is_terminal() def test_is_terminal_provisioning(self): resource = ManagedResource( "LoadBalancer", "lb-123", "PROVISIONING", 1761230543.151 ) - self.assertFalse(resource.is_terminal()) + assert not resource.is_terminal() def test_get_status_string_active(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) status_string = resource.get_status_string("⠋") - self.assertIn("LoadBalancer", status_string) - self.assertIn("lb-123", status_string) - self.assertIn("ACTIVE", status_string) + assert "LoadBalancer" in status_string + assert "lb-123" in status_string + assert "ACTIVE" in status_string def test_get_status_string_failed_with_reason(self): resource = ManagedResource( @@ -43,8 +45,8 @@ def test_get_status_string_failed_with_reason(self): "Connection timeout", ) status_string = resource.get_status_string("⠋") - self.assertIn("FAILED", status_string) - self.assertIn("Connection timeout", status_string) + assert "FAILED" in status_string + assert "Connection timeout" in status_string def test_get_status_string_active_with_reason(self): resource = ManagedResource( @@ -55,8 +57,8 @@ def test_get_status_string_active_with_reason(self): "Load balancer ready", ) status_string = resource.get_status_string("⠋") - self.assertIn("ACTIVE", status_string) - self.assertIn("Load balancer ready", status_string) + assert "ACTIVE" in status_string + assert "Load balancer ready" in status_string def test_combine_newer_resource(self): older = ManagedResource( @@ -66,7 +68,7 @@ def test_combine_newer_resource(self): "LoadBalancer", "lb-123", "ACTIVE", 1761230600.151 ) result = older.combine(newer) - self.assertEqual(result, newer) + assert result == newer def test_combine_older_resource(self): older = ManagedResource( @@ -76,33 +78,33 @@ def test_combine_older_resource(self): "LoadBalancer", "lb-123", "ACTIVE", 1761230600.151 ) result = newer.combine(older) - self.assertEqual(result, newer) + assert result == newer def test_combine_with_none(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) result = resource.combine(None) - self.assertEqual(result, resource) + assert result == resource def test_is_terminal_deleted_status(self): resource = ManagedResource("LoadBalancer", "lb-123", "DELETED") - self.assertTrue(resource.is_terminal()) + assert resource.is_terminal() def test_is_terminal_no_status(self): resource = ManagedResource("LoadBalancer", "lb-123", None) - self.assertFalse(resource.is_terminal()) + assert not resource.is_terminal() def test_init_with_string_timestamp(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", "2025-11-05T18:00:00Z" ) - self.assertIsInstance(resource.updated_at, float) - self.assertGreater(resource.updated_at, 0) + assert isinstance(resource.updated_at, float) + assert resource.updated_at > 0 def test_init_with_none_timestamp(self): resource = ManagedResource("LoadBalancer", "lb-123", "ACTIVE", None) - self.assertIsNone(resource.updated_at) + assert resource.updated_at is None def test_combine_with_no_timestamp(self): resource1 = ManagedResource( @@ -112,7 +114,7 @@ def test_combine_with_no_timestamp(self): "LoadBalancer", "lb-123", "PROVISIONING", None ) result = resource1.combine(resource2) - self.assertEqual(result, resource1) + assert result == resource1 def test_combine_equal_timestamps(self): timestamp = 1761230543.151 @@ -123,7 +125,7 @@ def test_combine_equal_timestamps(self): "LoadBalancer", "lb-123", "PROVISIONING", timestamp ) result = resource1.combine(resource2) - self.assertEqual(result, resource1) + assert result == resource1 def test_get_status_string_with_depth(self): resource = ManagedResource( @@ -132,7 +134,7 @@ def test_get_status_string_with_depth(self): status_string = resource.get_status_string("⠋", depth=2) # Should have proper indentation lines = status_string.split('\n') - self.assertTrue(lines[0].startswith(" ")) # 2 spaces for depth=2 + assert lines[0].startswith(" ") # 2 spaces for depth=2 def test_get_status_string_with_additional_info(self): resource = ManagedResource( @@ -143,47 +145,47 @@ def test_get_status_string_with_additional_info(self): additional_info="Load balancer is healthy", ) status_string = resource.get_status_string("⠋") - self.assertIn("Load balancer is healthy", status_string) + assert "Load balancer is healthy" in status_string def test_get_status_string_no_identifier(self): resource = ManagedResource( "LoadBalancer", None, "ACTIVE", 1761230543.151 ) status_string = resource.get_status_string("⠋") - self.assertIn("LoadBalancer", status_string) - self.assertIn("ACTIVE", status_string) + assert "LoadBalancer" in status_string + assert "ACTIVE" in status_string def test_get_status_string_no_color(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) status_string = resource.get_status_string("⠋", use_color=False) - self.assertIn("LoadBalancer", status_string) - self.assertIn("lb-123", status_string) - self.assertIn("ACTIVE", status_string) + assert "LoadBalancer" in status_string + assert "lb-123" in status_string + assert "ACTIVE" in status_string # Should not contain ANSI color codes - self.assertNotIn("\x1b[", status_string) + assert "\x1b[" not in status_string def test_get_status_string_with_color(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) status_string = resource.get_status_string("⠋", use_color=True) - self.assertIn("LoadBalancer", status_string) - self.assertIn("lb-123", status_string) - self.assertIn("ACTIVE", status_string) + assert "LoadBalancer" in status_string + assert "lb-123" in status_string + assert "ACTIVE" in status_string # Should contain ANSI color codes - self.assertIn("\x1b[", status_string) + assert "\x1b[" in status_string def test_get_stream_string_basic(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("[2025-12-15 10:00:00]", stream_string) - self.assertIn("LoadBalancer", stream_string) - self.assertIn("lb-123", stream_string) - self.assertIn("ACTIVE", stream_string) + assert "[2025-12-15 10:00:00]" in stream_string + assert "LoadBalancer" in stream_string + assert "lb-123" in stream_string + assert "ACTIVE" in stream_string def test_get_stream_string_with_reason(self): resource = ManagedResource( @@ -194,7 +196,7 @@ def test_get_stream_string_with_reason(self): "Waiting for DNS propagation", ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("Reason: Waiting for DNS propagation", stream_string) + assert "Reason: Waiting for DNS propagation" in stream_string def test_get_stream_string_with_additional_info(self): resource = ManagedResource( @@ -205,16 +207,16 @@ def test_get_stream_string_with_additional_info(self): additional_info="DNS: example.elb.amazonaws.com", ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("Info: DNS: example.elb.amazonaws.com", stream_string) + assert "Info: DNS: example.elb.amazonaws.com" in stream_string def test_get_stream_string_with_updated_at(self): resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("Last Updated At:", stream_string) + assert "Last Updated At:" in stream_string # Check timestamp format YYYY-MM-DD HH:MM:SS - self.assertRegex(stream_string, r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}") + assert re.search(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}", stream_string) def test_get_stream_string_all_fields(self): resource = ManagedResource( @@ -226,33 +228,33 @@ def test_get_stream_string_all_fields(self): "Health check interval: 30s", ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("[2025-12-15 10:00:00]", stream_string) - self.assertIn("TargetGroup", stream_string) - self.assertIn("tg-456", stream_string) - self.assertIn("PROVISIONING", stream_string) - self.assertIn("Reason: Registering targets", stream_string) - self.assertIn("Last Updated At:", stream_string) - self.assertIn("Info: Health check interval: 30s", stream_string) + assert "[2025-12-15 10:00:00]" in stream_string + assert "TargetGroup" in stream_string + assert "tg-456" in stream_string + assert "PROVISIONING" in stream_string + assert "Reason: Registering targets" in stream_string + assert "Last Updated At:" in stream_string + assert "Info: Health check interval: 30s" in stream_string def test_get_stream_string_no_identifier(self): resource = ManagedResource( "LoadBalancer", None, "ACTIVE", 1761230543.151 ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("[2025-12-15 10:00:00]", stream_string) - self.assertIn("LoadBalancer", stream_string) - self.assertIn("ACTIVE", stream_string) - self.assertNotIn("None", stream_string) + assert "[2025-12-15 10:00:00]" in stream_string + assert "LoadBalancer" in stream_string + assert "ACTIVE" in stream_string + assert "None" not in stream_string def test_get_stream_string_no_status(self): resource = ManagedResource( "LoadBalancer", "lb-123", None, 1761230543.151 ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("LoadBalancer", stream_string) - self.assertIn("lb-123", stream_string) + assert "LoadBalancer" in stream_string + assert "lb-123" in stream_string # Should not have status brackets when status is None - self.assertNotIn("[None]", stream_string) + assert "[None]" not in stream_string def test_get_stream_string_no_color(self): resource = ManagedResource( @@ -261,11 +263,11 @@ def test_get_stream_string_no_color(self): stream_string = resource.get_stream_string( "2025-12-15 10:00:00", use_color=False ) - self.assertIn("LoadBalancer", stream_string) - self.assertIn("lb-123", stream_string) - self.assertIn("ACTIVE", stream_string) + assert "LoadBalancer" in stream_string + assert "lb-123" in stream_string + assert "ACTIVE" in stream_string # Should not contain ANSI color codes - self.assertNotIn("\x1b[", stream_string) + assert "\x1b[" not in stream_string def test_get_stream_string_with_color(self): resource = ManagedResource( @@ -274,11 +276,11 @@ def test_get_stream_string_with_color(self): stream_string = resource.get_stream_string( "2025-12-15 10:00:00", use_color=True ) - self.assertIn("LoadBalancer", stream_string) - self.assertIn("lb-123", stream_string) - self.assertIn("ACTIVE", stream_string) + assert "LoadBalancer" in stream_string + assert "lb-123" in stream_string + assert "ACTIVE" in stream_string # Should contain ANSI color codes - self.assertIn("\x1b[", stream_string) + assert "\x1b[" in stream_string def test_get_stream_string_failed_status(self): resource = ManagedResource( @@ -289,17 +291,13 @@ def test_get_stream_string_failed_status(self): "Connection timeout", ) stream_string = resource.get_stream_string("2025-12-15 10:00:00") - self.assertIn("FAILED", stream_string) - self.assertIn("Reason: Connection timeout", stream_string) + assert "FAILED" in stream_string + assert "Reason: Connection timeout" in stream_string # Failed status should use color coding - self.assertIn("\x1b[", stream_string) + assert "\x1b[" in stream_string -class TestConstants(unittest.TestCase): +class TestConstants: def test_terminal_resource_statuses(self): expected_statuses = ["ACTIVE", "DELETED", "FAILED"] - self.assertEqual(TERMINAL_RESOURCE_STATUSES, expected_statuses) - - -if __name__ == '__main__': - unittest.main() + assert TERMINAL_RESOURCE_STATUSES == expected_statuses diff --git a/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py b/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py index b9c9e6c56b30..d32300e4c5a3 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py +++ b/tests/unit/customizations/ecs/expressgateway/test_managedresourcegroup.py @@ -1,4 +1,4 @@ -import unittest +import pytest from awscli.customizations.ecs.expressgateway.managedresource import ( ManagedResource, @@ -8,170 +8,108 @@ ) -class TestManagedResourceGroup(unittest.TestCase): - def setUp(self): - self.resource1 = ManagedResource( - "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 - ) - self.resource2 = ManagedResource( - "Certificate", "cert-456", "PROVISIONING", 1761230543.151 - ) +@pytest.fixture +def resource1(): + return ManagedResource("LoadBalancer", "lb-123", "ACTIVE", 1761230543.151) + + +@pytest.fixture +def resource2(): + return ManagedResource( + "Certificate", "cert-456", "PROVISIONING", 1761230543.151 + ) + +class TestManagedResourceGroup: def test_is_terminal_all_terminal(self): terminal_resource = ManagedResource( "LoadBalancer", "lb-123", "ACTIVE", 1761230543.151 ) group = ManagedResourceGroup(resources=[terminal_resource]) - self.assertTrue(group.is_terminal()) + assert group.is_terminal() - def test_is_terminal_mixed(self): - group = ManagedResourceGroup( - resources=[self.resource1, self.resource2] - ) - self.assertFalse(group.is_terminal()) + def test_is_terminal_mixed(self, resource1, resource2): + group = ManagedResourceGroup(resources=[resource1, resource2]) + assert not group.is_terminal() def test_is_terminal_empty(self): group = ManagedResourceGroup() - self.assertTrue(group.is_terminal()) + assert group.is_terminal() - def test_get_status_string_with_header(self): + def test_get_status_string_with_header(self, resource1): group = ManagedResourceGroup( resource_type="IngressPaths", identifier="endpoint-1", - resources=[self.resource1], + resources=[resource1], ) status_string = group.get_status_string("⠋") - self.assertIn("IngressPaths", status_string) - self.assertIn("endpoint-1", status_string) - - def test_create_key(self): - group = ManagedResourceGroup() - key = group._create_key(self.resource1) - self.assertEqual(key, "LoadBalancer/lb-123") - - def test_get_status_string_empty_group(self): - group = ManagedResourceGroup(resource_type="EmptyGroup", resources=[]) - status_string = group.get_status_string("⠋") - self.assertIn("EmptyGroup", status_string) - self.assertIn("", status_string) - - def test_combine_resource_groups(self): - group1 = ManagedResourceGroup(resources=[self.resource1]) - group2 = ManagedResourceGroup(resources=[self.resource2]) - combined = group1.combine(group2) - self.assertEqual(len(combined.resource_mapping), 2) - - def test_combine_child_resources_both_none(self): - group = ManagedResourceGroup() - result = group._combine_child_resources(None, None) - self.assertIsNone(result) - - def test_combine_child_resources_first_none(self): - group = ManagedResourceGroup() - resource = ManagedResource("LoadBalancer", "lb-123", "ACTIVE") - result = group._combine_child_resources(None, resource) - self.assertEqual(result, resource) - - def test_combine_overlapping_resources(self): - older_resource = ManagedResource( - "LoadBalancer", "lb-123", "PROVISIONING", 1761230543.151 - ) - newer_resource = ManagedResource( - "LoadBalancer", "lb-123", "ACTIVE", 1761230600.151 - ) + assert "IngressPaths" in status_string + assert "endpoint-1" in status_string - group1 = ManagedResourceGroup(resources=[older_resource]) - group2 = ManagedResourceGroup(resources=[newer_resource]) - - combined = group1.combine(group2) - - key = "LoadBalancer/lb-123" - self.assertIn(key, combined.resource_mapping) - self.assertEqual(combined.resource_mapping[key].status, "ACTIVE") - - def test_create_key_with_none_values(self): - group = ManagedResourceGroup() - resource = ManagedResource(None, None) - key = group._create_key(resource) - self.assertEqual(key, "/") - - def test_create_key_partial_none(self): - group = ManagedResourceGroup() - - resource1 = ManagedResource(None, "identifier") - key1 = group._create_key(resource1) - self.assertEqual(key1, "/identifier") - - resource2 = ManagedResource("ResourceType", None) - key2 = group._create_key(resource2) - self.assertEqual(key2, "ResourceType/") - - def test_compare_resource_sets_unique_resources(self): + def test_compare_resource_sets_unique_resources( + self, resource1, resource2 + ): # Test compare_resource_sets with completely different resources - group1 = ManagedResourceGroup( - resources=[self.resource1] - ) # LoadBalancer - group2 = ManagedResourceGroup( - resources=[self.resource2] - ) # Certificate + group1 = ManagedResourceGroup(resources=[resource1]) # LoadBalancer + group2 = ManagedResourceGroup(resources=[resource2]) # Certificate diff1, diff2 = group1.compare_resource_sets(group2) # Each group should contain its unique resource - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertEqual(len(diff2.resource_mapping), 1) - self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) - self.assertIn("Certificate/cert-456", diff2.resource_mapping) + assert len(diff1.resource_mapping) == 1 + assert len(diff2.resource_mapping) == 1 + assert "LoadBalancer/lb-123" in diff1.resource_mapping + assert "Certificate/cert-456" in diff2.resource_mapping - def test_compare_resource_sets_overlapping_resources(self): + def test_compare_resource_sets_overlapping_resources( + self, resource1, resource2 + ): # Test compare_resource_sets with same resource type but different identifiers resource3 = ManagedResource( "LoadBalancer", "lb-456", "FAILED", 1761230600.151 ) group1 = ManagedResourceGroup( - resources=[self.resource1, self.resource2] + resources=[resource1, resource2] ) # lb-123, cert-456 group2 = ManagedResourceGroup( - resources=[self.resource2, resource3] + resources=[resource2, resource3] ) # cert-456, lb-456 diff1, diff2 = group1.compare_resource_sets(group2) # group1 unique: lb-123, group2 unique: lb-456, common: cert-456 (should not appear in diff) - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertEqual(len(diff2.resource_mapping), 1) - self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) - self.assertIn("LoadBalancer/lb-456", diff2.resource_mapping) + assert len(diff1.resource_mapping) == 1 + assert len(diff2.resource_mapping) == 1 + assert "LoadBalancer/lb-123" in diff1.resource_mapping + assert "LoadBalancer/lb-456" in diff2.resource_mapping # Common resource should not be in either diff - self.assertNotIn("Certificate/cert-456", diff1.resource_mapping) - self.assertNotIn("Certificate/cert-456", diff2.resource_mapping) + assert "Certificate/cert-456" not in diff1.resource_mapping + assert "Certificate/cert-456" not in diff2.resource_mapping - def test_compare_resource_sets_identical_groups(self): + def test_compare_resource_sets_identical_groups( + self, resource1, resource2 + ): # Test compare_resource_sets with identical resource groups - group1 = ManagedResourceGroup( - resources=[self.resource1, self.resource2] - ) - group2 = ManagedResourceGroup( - resources=[self.resource1, self.resource2] - ) + group1 = ManagedResourceGroup(resources=[resource1, resource2]) + group2 = ManagedResourceGroup(resources=[resource1, resource2]) diff1, diff2 = group1.compare_resource_sets(group2) # No differences should be found - self.assertEqual(len(diff1.resource_mapping), 0) - self.assertEqual(len(diff2.resource_mapping), 0) + assert len(diff1.resource_mapping) == 0 + assert len(diff2.resource_mapping) == 0 - def test_compare_resource_sets_empty_groups(self): + def test_compare_resource_sets_empty_groups(self, resource1): # Test compare_resource_sets with empty groups - group1 = ManagedResourceGroup(resources=[self.resource1]) + group1 = ManagedResourceGroup(resources=[resource1]) group2 = ManagedResourceGroup(resources=[]) diff1, diff2 = group1.compare_resource_sets(group2) # group1 should contain its resource, group2 should be empty - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertEqual(len(diff2.resource_mapping), 0) - self.assertIn("LoadBalancer/lb-123", diff1.resource_mapping) + assert len(diff1.resource_mapping) == 1 + assert len(diff2.resource_mapping) == 0 + assert "LoadBalancer/lb-123" in diff1.resource_mapping def test_compare_resource_sets_excludes_matching_types_without_identifier( self, @@ -190,70 +128,70 @@ def test_compare_resource_sets_excludes_matching_types_without_identifier( diff1, diff2 = group1.compare_resource_sets(group2) # group1 should contain its resource without identifier - self.assertEqual(len(diff1.resource_mapping), 1) - self.assertIn("LoadBalancer/", diff1.resource_mapping) + assert len(diff1.resource_mapping) == 1 + assert "LoadBalancer/" in diff1.resource_mapping # group2 should be empty because LoadBalancer/lb-456 is excluded by LoadBalancer/ - self.assertEqual(len(diff2.resource_mapping), 0) + assert len(diff2.resource_mapping) == 0 def test_get_status_string_with_status(self): group = ManagedResourceGroup( resource_type="IngressPaths", identifier="test-id", status="ACTIVE" ) result = group.get_status_string("⠋") - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertIn("✓", result) # Green checkmark for ACTIVE - self.assertIn("ACTIVE", result) + assert "IngressPaths" in result + assert "test-id" in result + assert "✓" in result # Green checkmark for ACTIVE + assert "ACTIVE" in result def test_get_status_string_without_status(self): group = ManagedResourceGroup( resource_type="IngressPaths", identifier="test-id" ) result = group.get_status_string("⠋") - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertNotIn("✓", result) # No symbol when no status - self.assertNotIn("ACTIVE", result) + assert "IngressPaths" in result + assert "test-id" in result + assert "✓" not in result # No symbol when no status + assert "ACTIVE" not in result def test_get_status_string_status_without_identifier(self): group = ManagedResourceGroup( resource_type="IngressPaths", status="FAILED" ) result = group.get_status_string("⠋") - self.assertIn("IngressPaths", result) - self.assertIn("X", result) # Red X for FAILED - self.assertIn("FAILED", result) + assert "IngressPaths" in result + assert "X" in result # Red X for FAILED + assert "FAILED" in result - def test_get_status_string_no_color(self): + def test_get_status_string_no_color(self, resource1): group = ManagedResourceGroup( resource_type="IngressPaths", identifier="test-id", status="ACTIVE", - resources=[self.resource1], + resources=[resource1], ) result = group.get_status_string("⠋", use_color=False) - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertIn("✓", result) # Checkmark should still be there - self.assertIn("ACTIVE", result) + assert "IngressPaths" in result + assert "test-id" in result + assert "✓" in result # Checkmark should still be there + assert "ACTIVE" in result # Should not contain ANSI color codes - self.assertNotIn("\x1b[", result) + assert "\x1b[" not in result - def test_get_status_string_with_color(self): + def test_get_status_string_with_color(self, resource1): group = ManagedResourceGroup( resource_type="IngressPaths", identifier="test-id", status="ACTIVE", - resources=[self.resource1], + resources=[resource1], ) result = group.get_status_string("⠋", use_color=True) - self.assertIn("IngressPaths", result) - self.assertIn("test-id", result) - self.assertIn("✓", result) # Checkmark should be there - self.assertIn("ACTIVE", result) + assert "IngressPaths" in result + assert "test-id" in result + assert "✓" in result # Checkmark should be there + assert "ACTIVE" in result # Should contain ANSI color codes - self.assertIn("\x1b[", result) + assert "\x1b[" in result def test_combine_prioritizes_resources_with_identifier(self): resource_with_id = ManagedResource( @@ -277,15 +215,15 @@ def test_combine_prioritizes_resources_with_identifier(self): result = group1.combine(group2) # Should only have the resource with identifier - self.assertEqual(len(result.resource_mapping), 1) + assert len(result.resource_mapping) == 1 combined_resource = list(result.resource_mapping.values())[0] - self.assertEqual(combined_resource.identifier, "lb-123") + assert combined_resource.identifier == "lb-123" def test_get_stream_string_empty_group(self): """Test empty resource group returns empty string""" group = ManagedResourceGroup() result = group.get_stream_string("2025-12-15 10:00:00") - self.assertEqual(result, "") + assert result == "" def test_get_stream_string_single_resource(self): """Test resource group with single resource""" @@ -294,10 +232,10 @@ def test_get_stream_string_single_resource(self): ) group = ManagedResourceGroup(resources=[resource]) result = group.get_stream_string("2025-12-15 10:00:00") - self.assertIn("[2025-12-15 10:00:00]", result) - self.assertIn("LoadBalancer", result) - self.assertIn("lb-123", result) - self.assertIn("ACTIVE", result) + assert "[2025-12-15 10:00:00]" in result + assert "LoadBalancer" in result + assert "lb-123" in result + assert "ACTIVE" in result def test_get_stream_string_multiple_resources(self): """Test resource group with multiple resources""" @@ -311,14 +249,14 @@ def test_get_stream_string_multiple_resources(self): result = group.get_stream_string("2025-12-15 10:00:00") # Should have both resources - self.assertIn("LoadBalancer", result) - self.assertIn("lb-123", result) - self.assertIn("TargetGroup", result) - self.assertIn("tg-456", result) + assert "LoadBalancer" in result + assert "lb-123" in result + assert "TargetGroup" in result + assert "tg-456" in result # Should have newline between resources lines = result.split("\n") - self.assertGreater(len(lines), 1) + assert len(lines) > 1 def test_get_stream_string_nested_groups(self): """Test resource group with nested groups""" @@ -334,10 +272,10 @@ def test_get_stream_string_nested_groups(self): result = group.get_stream_string("2025-12-15 10:00:00") # Should have flattened both resources - self.assertIn("LoadBalancer", result) - self.assertIn("lb-123", result) - self.assertIn("TargetGroup", result) - self.assertIn("tg-456", result) + assert "LoadBalancer" in result + assert "lb-123" in result + assert "TargetGroup" in result + assert "tg-456" in result def test_get_stream_string_deeply_nested_groups(self): """Test resource group with multiple levels of nesting""" @@ -360,9 +298,9 @@ def test_get_stream_string_deeply_nested_groups(self): result = group.get_stream_string("2025-12-15 10:00:00") # Should have all resources flattened - self.assertIn("Cluster", result) - self.assertIn("Service", result) - self.assertIn("LoadBalancer", result) + assert "Cluster" in result + assert "Service" in result + assert "LoadBalancer" in result def test_get_stream_string_with_resource_details(self): """Test that resource details are preserved in stream output""" @@ -377,9 +315,9 @@ def test_get_stream_string_with_resource_details(self): group = ManagedResourceGroup(resources=[resource]) result = group.get_stream_string("2025-12-15 10:00:00") - self.assertIn("Reason: Waiting for DNS propagation", result) - self.assertIn("Info: DNS: example.elb.amazonaws.com", result) - self.assertIn("Last Updated At:", result) + assert "Reason: Waiting for DNS propagation" in result + assert "Info: DNS: example.elb.amazonaws.com" in result + assert "Last Updated At:" in result def test_get_stream_string_preserves_order(self): """Test that resource order is preserved in output""" @@ -404,8 +342,8 @@ def test_get_stream_string_preserves_order(self): lb_pos = result.find("LoadBalancer") # Order should be preserved - self.assertLess(cluster_pos, service_pos) - self.assertLess(service_pos, lb_pos) + assert cluster_pos < service_pos + assert service_pos < lb_pos def test_get_stream_string_no_color(self): """Test stream string without color codes""" @@ -417,10 +355,10 @@ def test_get_stream_string_no_color(self): "2025-12-15 10:00:00", use_color=False ) - self.assertIn("LoadBalancer", result) - self.assertIn("lb-123", result) + assert "LoadBalancer" in result + assert "lb-123" in result # Should not contain ANSI color codes - self.assertNotIn("\x1b[", result) + assert "\x1b[" not in result def test_get_stream_string_with_color(self): """Test stream string with color codes""" @@ -430,10 +368,10 @@ def test_get_stream_string_with_color(self): group = ManagedResourceGroup(resources=[resource]) result = group.get_stream_string("2025-12-15 10:00:00", use_color=True) - self.assertIn("LoadBalancer", result) - self.assertIn("lb-123", result) + assert "LoadBalancer" in result + assert "lb-123" in result # Should contain ANSI color codes - self.assertIn("\x1b[", result) + assert "\x1b[" in result def test_get_stream_string_mixed_resource_types(self): """Test group with various resource types and statuses""" @@ -457,16 +395,16 @@ def test_get_stream_string_mixed_resource_types(self): result = group.get_stream_string("2025-12-15 10:00:00") # All resources should be present - self.assertIn("Cluster", result) - self.assertIn("Service", result) - self.assertIn("LoadBalancer", result) - self.assertIn("TargetGroup", result) + assert "Cluster" in result + assert "Service" in result + assert "LoadBalancer" in result + assert "TargetGroup" in result # All statuses should be present - self.assertIn("ACTIVE", result) - self.assertIn("UPDATING", result) - self.assertIn("PROVISIONING", result) - self.assertIn("FAILED", result) + assert "ACTIVE" in result + assert "UPDATING" in result + assert "PROVISIONING" in result + assert "FAILED" in result def test_get_stream_string_with_group_metadata(self): """Test that group-level metadata doesn't affect stream output""" @@ -483,12 +421,12 @@ def test_get_stream_string_with_group_metadata(self): result = group.get_stream_string("2025-12-15 10:00:00") # Should still show the actual resource, not group metadata - self.assertIn("LoadBalancer", result) - self.assertIn("lb-123", result) + assert "LoadBalancer" in result + assert "lb-123" in result # Group metadata should not appear in stream output # (stream output is for individual resources only) - self.assertNotIn("group-1", result) + assert "group-1" not in result def test_get_stream_string_empty_nested_group(self): """Test nested group that is empty is handled correctly""" @@ -501,12 +439,8 @@ def test_get_stream_string_empty_nested_group(self): result = group.get_stream_string("2025-12-15 10:00:00") # Should show the resource from non-empty group - self.assertIn("LoadBalancer", result) + assert "LoadBalancer" in result # Empty nested group shouldn't add extra content lines = [line for line in result.split("\n") if line.strip()] # LoadBalancer resource produces multiple lines (timestamp line + optional detail lines) - self.assertGreater(len(lines), 0) - - -if __name__ == '__main__': - unittest.main() + assert len(lines) > 0 From 4a75702c20c38191bdaa6b7889e4d44ad6fad29c Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Mon, 15 Dec 2025 13:25:35 -0500 Subject: [PATCH 06/17] Extract mode specific display into DisplayStrategy --- .../ecs/expressgateway/display_strategy.py | 140 +++++++++++ .../ecs/monitorexpressgatewayservice.py | 83 +------ .../expressgateway/test_display_strategy.py | 224 ++++++++++++++++++ .../ecs/test_monitorexpressgatewayservice.py | 6 - 4 files changed, 374 insertions(+), 79 deletions(-) create mode 100644 awscli/customizations/ecs/expressgateway/display_strategy.py create mode 100644 tests/unit/customizations/ecs/expressgateway/test_display_strategy.py diff --git a/awscli/customizations/ecs/expressgateway/display_strategy.py b/awscli/customizations/ecs/expressgateway/display_strategy.py new file mode 100644 index 000000000000..1ef084f9ce8b --- /dev/null +++ b/awscli/customizations/ecs/expressgateway/display_strategy.py @@ -0,0 +1,140 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +"""Display strategy implementations for ECS Express Gateway Service monitoring.""" + +import asyncio +import time + +from botocore.exceptions import ClientError + +from awscli.customizations.utils import uni_print + + +class DisplayStrategy: + """Base class for display strategies. + + Each strategy controls its own execution model, timing, and output format. + """ + + def execute(self, collector, start_time, timeout_minutes): + """Execute the monitoring loop. + + Args: + collector: ServiceViewCollector instance for data fetching + start_time: Start timestamp for timeout calculation + timeout_minutes: Maximum monitoring duration in minutes + """ + raise NotImplementedError + + +class InteractiveDisplayStrategy(DisplayStrategy): + """Interactive display strategy with async spinner and keyboard navigation. + + Uses dual async tasks: + - Data task: Polls ECS APIs every 5 seconds + - Spinner task: Updates display every 100ms with rotating spinner + """ + + def __init__(self, display, use_color): + self.display = display + self.use_color = use_color + + def execute(self, collector, start_time, timeout_minutes): + """Execute async monitoring with spinner and keyboard controls.""" + final_output, timed_out = asyncio.run( + self._execute_async(collector, start_time, timeout_minutes) + ) + if timed_out: + uni_print(final_output + "\nMonitoring timed out!\n") + else: + uni_print(final_output + "\nMonitoring Complete!\n") + + async def _execute_async(self, collector, start_time, timeout_minutes): + """Async execution with dual tasks for data and spinner.""" + spinner_chars = "⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏" + spinner_index = 0 + current_output = "Waiting for initial data" + timed_out = False + + async def update_data(): + nonlocal current_output, timed_out + while True: + current_time = time.time() + if current_time - start_time > timeout_minutes * 60: + timed_out = True + self.display.app.exit() + break + + try: + loop = asyncio.get_event_loop() + new_output = await loop.run_in_executor( + None, collector.get_current_view, "{SPINNER}" + ) + current_output = new_output + except ClientError as e: + if ( + e.response.get('Error', {}).get('Code') + == 'InvalidParameterException' + ): + error_message = e.response.get('Error', {}).get( + 'Message', '' + ) + if ( + "Cannot call DescribeServiceRevisions for a service that is INACTIVE" + in error_message + ): + current_output = "Service is inactive" + else: + raise + else: + raise + + await asyncio.sleep(5.0) + + async def update_spinner(): + nonlocal spinner_index + while True: + spinner_char = spinner_chars[spinner_index] + display_output = current_output.replace( + "{SPINNER}", spinner_char + ) + status_text = f"Getting updates... {spinner_char} | up/down to scroll, q to quit" + self.display.display(display_output, status_text) + spinner_index = (spinner_index + 1) % len(spinner_chars) + await asyncio.sleep(0.1) + + data_task = asyncio.create_task(update_data()) + spinner_task = asyncio.create_task(update_spinner()) + display_task = None + + try: + display_task = asyncio.create_task(self.display.run()) + + done, pending = await asyncio.wait( + [display_task, data_task], return_when=asyncio.FIRST_COMPLETED + ) + + if data_task in done: + await data_task + + finally: + spinner_task.cancel() + if display_task is not None and not display_task.done(): + display_task.cancel() + try: + await display_task + except asyncio.CancelledError: + pass + + return current_output.replace("{SPINNER}", ""), timed_out diff --git a/awscli/customizations/ecs/monitorexpressgatewayservice.py b/awscli/customizations/ecs/monitorexpressgatewayservice.py index e946c010dce3..a5ed8769cf1c 100644 --- a/awscli/customizations/ecs/monitorexpressgatewayservice.py +++ b/awscli/customizations/ecs/monitorexpressgatewayservice.py @@ -39,15 +39,16 @@ aws ecs monitor-express-gateway-service --service-arn [--resource-view RESOURCE|DEPLOYMENT] """ -import asyncio import sys -import threading import time from botocore.exceptions import ClientError from awscli.customizations.commands import BasicCommand from awscli.customizations.ecs.exceptions import MonitoringError +from awscli.customizations.ecs.expressgateway.display_strategy import ( + InteractiveDisplayStrategy, +) from awscli.customizations.ecs.prompt_toolkit_display import Display from awscli.customizations.ecs.serviceviewcollector import ServiceViewCollector from awscli.customizations.utils import uni_print @@ -185,7 +186,7 @@ def __init__( service_arn, mode, timeout_minutes=30, - display=None, + display_strategy=None, use_color=True, collector=None, ): @@ -195,7 +196,9 @@ def __init__( self.timeout_minutes = timeout_minutes self.start_time = time.time() self.use_color = use_color - self.display = display or Display() + self.display_strategy = display_strategy or InteractiveDisplayStrategy( + display=Display(), use_color=use_color + ) self.collector = collector or ServiceViewCollector( client, service_arn, mode, use_color ) @@ -207,72 +210,6 @@ def is_monitoring_available(): def exec(self): """Start monitoring the express gateway service with progress display.""" - - def monitor_service(spinner_char): - return self.collector.get_current_view(spinner_char) - - asyncio.run(self._execute_with_progress_async(monitor_service, 100)) - - async def _execute_with_progress_async( - self, execution, progress_refresh_millis, execution_refresh_millis=5000 - ): - """Execute monitoring loop with animated progress display.""" - spinner_chars = "⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏" - spinner_index = 0 - - current_output = "Waiting for initial data" - - async def update_data(): - nonlocal current_output - while True: - current_time = time.time() - if current_time - self.start_time > self.timeout_minutes * 60: - break - try: - loop = asyncio.get_event_loop() - new_output = await loop.run_in_executor( - None, execution, "{SPINNER}" - ) - current_output = new_output - except ClientError as e: - if ( - e.response.get('Error', {}).get('Code') - == 'InvalidParameterException' - ): - error_message = e.response.get('Error', {}).get( - 'Message', '' - ) - if ( - "Cannot call DescribeServiceRevisions for a service that is INACTIVE" - in error_message - ): - current_output = "Service is inactive" - else: - raise - else: - raise - await asyncio.sleep(execution_refresh_millis / 1000.0) - - async def update_spinner(): - nonlocal spinner_index - while True: - spinner_char = spinner_chars[spinner_index] - display_output = current_output.replace( - "{SPINNER}", spinner_char - ) - status_text = f"Getting updates... {spinner_char} | up/down to scroll, q to quit" - self.display.display(display_output, status_text) - spinner_index = (spinner_index + 1) % len(spinner_chars) - await asyncio.sleep(progress_refresh_millis / 1000.0) - - # Start both tasks - data_task = asyncio.create_task(update_data()) - spinner_task = asyncio.create_task(update_spinner()) - - try: - await self.display.run() - finally: - data_task.cancel() - spinner_task.cancel() - final_output = current_output.replace("{SPINNER}", "") - uni_print(final_output + "\nMonitoring Complete!\n") + self.display_strategy.execute_monitoring( + self.collector, self.timeout_minutes, self.start_time + ) diff --git a/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py new file mode 100644 index 000000000000..c3c6baa40d60 --- /dev/null +++ b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py @@ -0,0 +1,224 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ + +import asyncio +import time +from unittest.mock import Mock, patch + +import pytest +from botocore.exceptions import ClientError +from prompt_toolkit.application import create_app_session +from prompt_toolkit.output import DummyOutput + +from awscli.customizations.ecs.expressgateway.display_strategy import ( + DisplayStrategy, + InteractiveDisplayStrategy, +) + + +class TestDisplayStrategy: + """Test base DisplayStrategy class.""" + + def test_base_strategy_not_implemented(self): + """Test base class raises NotImplementedError.""" + strategy = DisplayStrategy() + with pytest.raises(NotImplementedError): + strategy.execute(None, None, None) + + +@pytest.fixture +def app_session(): + """Fixture that creates and manages an app session for prompt_toolkit.""" + session = create_app_session(output=DummyOutput()) + session.__enter__() + yield session + session.__exit__(None, None, None) + + +class TestInteractiveDisplayStrategy: + """Test InteractiveDisplayStrategy.""" + + @patch('time.sleep') + def test_execute_with_mock_display(self, mock_sleep, app_session): + """Test strategy executes with mocked display.""" + + async def mock_run_async(): + await asyncio.sleep(0.01) + + mock_display = Mock() + mock_display.display = Mock() + mock_display.run = Mock(return_value=mock_run_async()) + + mock_collector = Mock() + mock_collector.get_current_view = Mock( + return_value="Test output {SPINNER}" + ) + + strategy = InteractiveDisplayStrategy( + display=mock_display, use_color=True + ) + + mock_sleep.side_effect = KeyboardInterrupt() + + start_time = time.time() + strategy.execute(mock_collector, start_time, timeout_minutes=1) + + # Verify display was called + assert mock_display.display.called + assert mock_display.run.called + + def test_strategy_uses_provided_color_setting(self): + """Test strategy respects use_color parameter.""" + mock_display = Mock() + + strategy_with_color = InteractiveDisplayStrategy( + display=mock_display, use_color=True + ) + assert strategy_with_color.use_color is True + + strategy_no_color = InteractiveDisplayStrategy( + display=mock_display, use_color=False + ) + assert strategy_no_color.use_color is False + + @patch('time.sleep') + def test_completion_message_on_normal_exit( + self, mock_sleep, app_session, capsys + ): + """Test displays completion message when monitoring completes normally.""" + + async def mock_run_async(): + await asyncio.sleep(0.01) + + mock_display = Mock() + mock_display.display = Mock() + mock_display.run = Mock(return_value=mock_run_async()) + + mock_collector = Mock() + mock_collector.get_current_view = Mock(return_value="Resources ready") + + strategy = InteractiveDisplayStrategy( + display=mock_display, use_color=True + ) + + mock_sleep.side_effect = KeyboardInterrupt() + + start_time = time.time() + strategy.execute(mock_collector, start_time, timeout_minutes=1) + + captured = capsys.readouterr() + assert "Monitoring Complete!" in captured.out + assert "Monitoring timed out!" not in captured.out + + @patch('time.sleep') + def test_collector_output_is_displayed( + self, mock_sleep, app_session, capsys + ): + """Test that collector output appears in final output.""" + + async def mock_run_async(): + await asyncio.sleep(0.01) + + mock_display = Mock() + mock_display.display = Mock() + mock_display.run = Mock(return_value=mock_run_async()) + + mock_collector = Mock() + unique_output = "LoadBalancer lb-12345 ACTIVE" + mock_collector.get_current_view = Mock(return_value=unique_output) + + strategy = InteractiveDisplayStrategy( + display=mock_display, use_color=True + ) + + mock_sleep.side_effect = KeyboardInterrupt() + + start_time = time.time() + strategy.execute(mock_collector, start_time, timeout_minutes=1) + + captured = capsys.readouterr() + assert unique_output in captured.out + + @patch('time.sleep') + def test_execute_handles_service_inactive( + self, mock_sleep, app_session, capsys + ): + """Test strategy handles service inactive error.""" + + async def mock_run_async(): + await asyncio.sleep(0.01) + + mock_display = Mock() + mock_display.display = Mock() + mock_display.run = Mock(return_value=mock_run_async()) + + mock_collector = Mock() + error = ClientError( + error_response={ + 'Error': { + 'Code': 'InvalidParameterException', + 'Message': 'Cannot call DescribeServiceRevisions for a service that is INACTIVE', + } + }, + operation_name='DescribeServiceRevisions', + ) + mock_collector.get_current_view = Mock(side_effect=error) + + strategy = InteractiveDisplayStrategy( + display=mock_display, use_color=True + ) + + mock_sleep.side_effect = KeyboardInterrupt() + + start_time = time.time() + strategy.execute(mock_collector, start_time, timeout_minutes=1) + + # Strategy should handle the error and set output to "Service is inactive" + captured = capsys.readouterr() + assert "Service is inactive" in captured.out + + @patch('time.sleep') + def test_execute_other_client_errors_propagate( + self, mock_sleep, app_session + ): + """Test strategy propagates non-service-inactive ClientErrors.""" + + async def mock_run_async(): + await asyncio.sleep(0.01) + + mock_display = Mock() + mock_display.display = Mock() + mock_display.run = Mock(return_value=mock_run_async()) + + mock_collector = Mock() + error = ClientError( + error_response={ + 'Error': { + 'Code': 'AccessDeniedException', + 'Message': 'Access denied', + } + }, + operation_name='DescribeServiceRevisions', + ) + mock_collector.get_current_view = Mock(side_effect=error) + + strategy = InteractiveDisplayStrategy( + display=mock_display, use_color=True + ) + + mock_sleep.side_effect = KeyboardInterrupt() + + start_time = time.time() + + # Other client errors should propagate + with pytest.raises(ClientError) as exc_info: + strategy.execute(mock_collector, start_time, timeout_minutes=1) + + assert ( + exc_info.value.response['Error']['Code'] == 'AccessDeniedException' + ) diff --git a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py index e0677a8e3451..6edecc02b4bf 100644 --- a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py @@ -19,12 +19,6 @@ ECSMonitorExpressGatewayService, ) -# Suppress thread exception warnings - tests use KeyboardInterrupt to exit monitoring loops, -# which causes expected exceptions in background threads -pytestmark = pytest.mark.filterwarnings( - "ignore::pytest.PytestUnhandledThreadExceptionWarning" -) - class TestECSMonitorExpressGatewayServiceCommand: """Test the command class through public interface""" From 0788841fe4ce4d55c1187ad0735cbba07892e826 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Tue, 16 Dec 2025 11:32:00 -0500 Subject: [PATCH 07/17] Address PR comments --- .../ecs/expressgateway/display_strategy.py | 14 ++++ .../expressgateway/test_display_strategy.py | 71 ++++++------------- 2 files changed, 36 insertions(+), 49 deletions(-) diff --git a/awscli/customizations/ecs/expressgateway/display_strategy.py b/awscli/customizations/ecs/expressgateway/display_strategy.py index 1ef084f9ce8b..74ff4b77b105 100644 --- a/awscli/customizations/ecs/expressgateway/display_strategy.py +++ b/awscli/customizations/ecs/expressgateway/display_strategy.py @@ -126,12 +126,26 @@ async def update_spinner(): ) if data_task in done: + # Await to retrieve result or re-raise any exception from the + # task. asyncio.wait() doesn't retrieve exceptions itself. await data_task + # Cancel pending tasks + for task in pending: + task.cancel() + # Await cancelled task to ensure proper cleanup and prevent + # warnings about unawaited tasks + try: + await task + except asyncio.CancelledError: + pass + finally: spinner_task.cancel() if display_task is not None and not display_task.done(): display_task.cancel() + # Await cancelled task to ensure proper cleanup and prevent + # warnings about unawaited tasks try: await display_task except asyncio.CancelledError: diff --git a/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py index c3c6baa40d60..7de5b4e79595 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py +++ b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py @@ -34,26 +34,31 @@ def test_base_strategy_not_implemented(self): @pytest.fixture def app_session(): """Fixture that creates and manages an app session for prompt_toolkit.""" - session = create_app_session(output=DummyOutput()) - session.__enter__() - yield session - session.__exit__(None, None, None) + with create_app_session(output=DummyOutput()) as session: + yield session + + +@pytest.fixture +def mock_display(): + """Fixture that creates a mock display for testing.""" + + async def mock_run_async(): + await asyncio.sleep(0.01) + + display = Mock() + display.display = Mock() + display.run = Mock(return_value=mock_run_async()) + return display class TestInteractiveDisplayStrategy: """Test InteractiveDisplayStrategy.""" @patch('time.sleep') - def test_execute_with_mock_display(self, mock_sleep, app_session): + def test_execute_with_mock_display( + self, mock_sleep, app_session, mock_display + ): """Test strategy executes with mocked display.""" - - async def mock_run_async(): - await asyncio.sleep(0.01) - - mock_display = Mock() - mock_display.display = Mock() - mock_display.run = Mock(return_value=mock_run_async()) - mock_collector = Mock() mock_collector.get_current_view = Mock( return_value="Test output {SPINNER}" @@ -88,17 +93,9 @@ def test_strategy_uses_provided_color_setting(self): @patch('time.sleep') def test_completion_message_on_normal_exit( - self, mock_sleep, app_session, capsys + self, mock_sleep, app_session, mock_display, capsys ): """Test displays completion message when monitoring completes normally.""" - - async def mock_run_async(): - await asyncio.sleep(0.01) - - mock_display = Mock() - mock_display.display = Mock() - mock_display.run = Mock(return_value=mock_run_async()) - mock_collector = Mock() mock_collector.get_current_view = Mock(return_value="Resources ready") @@ -117,17 +114,9 @@ async def mock_run_async(): @patch('time.sleep') def test_collector_output_is_displayed( - self, mock_sleep, app_session, capsys + self, mock_sleep, app_session, mock_display, capsys ): """Test that collector output appears in final output.""" - - async def mock_run_async(): - await asyncio.sleep(0.01) - - mock_display = Mock() - mock_display.display = Mock() - mock_display.run = Mock(return_value=mock_run_async()) - mock_collector = Mock() unique_output = "LoadBalancer lb-12345 ACTIVE" mock_collector.get_current_view = Mock(return_value=unique_output) @@ -146,17 +135,9 @@ async def mock_run_async(): @patch('time.sleep') def test_execute_handles_service_inactive( - self, mock_sleep, app_session, capsys + self, mock_sleep, app_session, mock_display, capsys ): """Test strategy handles service inactive error.""" - - async def mock_run_async(): - await asyncio.sleep(0.01) - - mock_display = Mock() - mock_display.display = Mock() - mock_display.run = Mock(return_value=mock_run_async()) - mock_collector = Mock() error = ClientError( error_response={ @@ -184,17 +165,9 @@ async def mock_run_async(): @patch('time.sleep') def test_execute_other_client_errors_propagate( - self, mock_sleep, app_session + self, mock_sleep, app_session, mock_display ): """Test strategy propagates non-service-inactive ClientErrors.""" - - async def mock_run_async(): - await asyncio.sleep(0.01) - - mock_display = Mock() - mock_display.display = Mock() - mock_display.run = Mock(return_value=mock_run_async()) - mock_collector = Mock() error = ClientError( error_response={ From 99e401d01148d19749848cbbf40ee1e049666a86 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Tue, 16 Dec 2025 12:26:59 -0500 Subject: [PATCH 08/17] Address PR comments --- .../customizations/ecs/expressgateway/display_strategy.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/awscli/customizations/ecs/expressgateway/display_strategy.py b/awscli/customizations/ecs/expressgateway/display_strategy.py index 74ff4b77b105..7cf287fb0b71 100644 --- a/awscli/customizations/ecs/expressgateway/display_strategy.py +++ b/awscli/customizations/ecs/expressgateway/display_strategy.py @@ -126,9 +126,11 @@ async def update_spinner(): ) if data_task in done: - # Await to retrieve result or re-raise any exception from the - # task. asyncio.wait() doesn't retrieve exceptions itself. - await data_task + # Retrieve and re-raise any exception from the task. + # asyncio.wait() doesn't retrieve exceptions itself. + exc = data_task.exception() + if exc: + raise exc # Cancel pending tasks for task in pending: From 298b0716843e3a9ba26ace247dce655be9001e2b Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Tue, 16 Dec 2025 13:01:51 -0500 Subject: [PATCH 09/17] Address PR comments --- .../ecs/expressgateway/display_strategy.py | 13 ++++- .../expressgateway/test_display_strategy.py | 47 ++++++++++++++++--- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/awscli/customizations/ecs/expressgateway/display_strategy.py b/awscli/customizations/ecs/expressgateway/display_strategy.py index 7cf287fb0b71..1065232054ee 100644 --- a/awscli/customizations/ecs/expressgateway/display_strategy.py +++ b/awscli/customizations/ecs/expressgateway/display_strategy.py @@ -27,7 +27,7 @@ class DisplayStrategy: Each strategy controls its own execution model, timing, and output format. """ - def execute(self, collector, start_time, timeout_minutes): + def execute_monitoring(self, collector, start_time, timeout_minutes): """Execute the monitoring loop. Args: @@ -47,10 +47,17 @@ class InteractiveDisplayStrategy(DisplayStrategy): """ def __init__(self, display, use_color): + """Initialize the interactive display strategy. + + Args: + display: Display instance from prompt_toolkit_display module + providing the interactive terminal interface + use_color: Whether to use colored output + """ self.display = display self.use_color = use_color - def execute(self, collector, start_time, timeout_minutes): + def execute_monitoring(self, collector, start_time, timeout_minutes): """Execute async monitoring with spinner and keyboard controls.""" final_output, timed_out = asyncio.run( self._execute_async(collector, start_time, timeout_minutes) @@ -143,6 +150,8 @@ async def update_spinner(): pass finally: + # Ensure display app is properly shut down + self.display.app.exit() spinner_task.cancel() if display_task is not None and not display_task.done(): display_task.cancel() diff --git a/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py index 7de5b4e79595..119b3d252a43 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py +++ b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py @@ -28,7 +28,7 @@ def test_base_strategy_not_implemented(self): """Test base class raises NotImplementedError.""" strategy = DisplayStrategy() with pytest.raises(NotImplementedError): - strategy.execute(None, None, None) + strategy.execute_monitoring(None, None, None) @pytest.fixture @@ -71,7 +71,9 @@ def test_execute_with_mock_display( mock_sleep.side_effect = KeyboardInterrupt() start_time = time.time() - strategy.execute(mock_collector, start_time, timeout_minutes=1) + strategy.execute_monitoring( + mock_collector, start_time, timeout_minutes=1 + ) # Verify display was called assert mock_display.display.called @@ -106,7 +108,9 @@ def test_completion_message_on_normal_exit( mock_sleep.side_effect = KeyboardInterrupt() start_time = time.time() - strategy.execute(mock_collector, start_time, timeout_minutes=1) + strategy.execute_monitoring( + mock_collector, start_time, timeout_minutes=1 + ) captured = capsys.readouterr() assert "Monitoring Complete!" in captured.out @@ -128,7 +132,9 @@ def test_collector_output_is_displayed( mock_sleep.side_effect = KeyboardInterrupt() start_time = time.time() - strategy.execute(mock_collector, start_time, timeout_minutes=1) + strategy.execute_monitoring( + mock_collector, start_time, timeout_minutes=1 + ) captured = capsys.readouterr() assert unique_output in captured.out @@ -157,7 +163,9 @@ def test_execute_handles_service_inactive( mock_sleep.side_effect = KeyboardInterrupt() start_time = time.time() - strategy.execute(mock_collector, start_time, timeout_minutes=1) + strategy.execute_monitoring( + mock_collector, start_time, timeout_minutes=1 + ) # Strategy should handle the error and set output to "Service is inactive" captured = capsys.readouterr() @@ -190,8 +198,35 @@ def test_execute_other_client_errors_propagate( # Other client errors should propagate with pytest.raises(ClientError) as exc_info: - strategy.execute(mock_collector, start_time, timeout_minutes=1) + strategy.execute_monitoring( + mock_collector, start_time, timeout_minutes=1 + ) assert ( exc_info.value.response['Error']['Code'] == 'AccessDeniedException' ) + + @patch('time.sleep') + def test_display_cleanup_on_exception( + self, mock_sleep, app_session, mock_display + ): + """Test display app is properly shut down when exception occurs.""" + mock_collector = Mock() + error = ClientError( + error_response={'Error': {'Code': 'ThrottlingException'}}, + operation_name='DescribeServiceRevisions', + ) + mock_collector.get_current_view = Mock(side_effect=error) + + strategy = InteractiveDisplayStrategy( + display=mock_display, use_color=True + ) + mock_sleep.side_effect = KeyboardInterrupt() + + with pytest.raises(ClientError): + strategy.execute_monitoring( + mock_collector, time.time(), timeout_minutes=1 + ) + + # Verify app.exit() was called in finally block despite exception + mock_display.app.exit.assert_called() From 532d26c195753c3fe0f13921acf464df3a65127c Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Mon, 15 Dec 2025 13:07:43 -0500 Subject: [PATCH 10/17] Introduce text only mode --- .../next-release/enhancement-ecs-63253.json | 5 + .../ecs/expressgateway/color_utils.py | 2 +- .../ecs/expressgateway/display_strategy.py | 74 ++++- .../ecs/monitorexpressgatewayservice.py | 115 ++++++-- .../ecs/serviceviewcollector.py | 4 +- .../expressgateway/test_display_strategy.py | 62 +++++ .../ecs/test_monitorexpressgatewayservice.py | 255 ++++++++++++++++-- .../ecs/test_serviceviewcollector.py | 4 +- 8 files changed, 473 insertions(+), 48 deletions(-) create mode 100644 .changes/next-release/enhancement-ecs-63253.json diff --git a/.changes/next-release/enhancement-ecs-63253.json b/.changes/next-release/enhancement-ecs-63253.json new file mode 100644 index 000000000000..3e4985cf1189 --- /dev/null +++ b/.changes/next-release/enhancement-ecs-63253.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "``ecs``", + "description": "Introduce text-only mode to ECS Express Mode service monitoring APIs" +} diff --git a/awscli/customizations/ecs/expressgateway/color_utils.py b/awscli/customizations/ecs/expressgateway/color_utils.py index 038120df60ae..7335d22f0877 100644 --- a/awscli/customizations/ecs/expressgateway/color_utils.py +++ b/awscli/customizations/ecs/expressgateway/color_utils.py @@ -24,7 +24,7 @@ class ColorUtils: def __init__(self): # Initialize colorama - init(autoreset=True, strip=False) + init(autoreset=False, strip=False) def make_green(self, text, use_color=True): if not use_color: diff --git a/awscli/customizations/ecs/expressgateway/display_strategy.py b/awscli/customizations/ecs/expressgateway/display_strategy.py index 1065232054ee..6d3c9ed54b17 100644 --- a/awscli/customizations/ecs/expressgateway/display_strategy.py +++ b/awscli/customizations/ecs/expressgateway/display_strategy.py @@ -18,6 +18,10 @@ from botocore.exceptions import ClientError +from awscli.customizations.ecs.exceptions import MonitoringError +from awscli.customizations.ecs.expressgateway.stream_display import ( + StreamDisplay, +) from awscli.customizations.utils import uni_print @@ -80,7 +84,9 @@ async def update_data(): current_time = time.time() if current_time - start_time > timeout_minutes * 60: timed_out = True - self.display.app.exit() + # Only exit if app is running to avoid "Application is not running" error + if self.display.app.is_running: + self.display.app.exit() break try: @@ -151,7 +157,9 @@ async def update_spinner(): finally: # Ensure display app is properly shut down - self.display.app.exit() + # Only exit if app is running to avoid "Application is not running" error + if self.display.app.is_running: + self.display.app.exit() spinner_task.cancel() if display_task is not None and not display_task.done(): display_task.cancel() @@ -163,3 +171,65 @@ async def update_spinner(): pass return current_output.replace("{SPINNER}", ""), timed_out + + +class TextOnlyDisplayStrategy(DisplayStrategy): + """Text-only display strategy with diff detection and timestamped output. + + Uses synchronous polling loop with change detection to output only + individual resource changes with timestamps. + """ + + def __init__(self, use_color): + self.stream_display = StreamDisplay(use_color) + + def execute_monitoring(self, collector, start_time, timeout_minutes): + """Execute synchronous monitoring with text output.""" + self.stream_display.show_startup_message() + + try: + while True: + current_time = time.time() + if current_time - start_time > timeout_minutes * 60: + self.stream_display.show_timeout_message() + break + + try: + self.stream_display.show_polling_message() + + collector.get_current_view("") + + # Extract cached result for diff detection + managed_resources, info = collector.cached_monitor_result + + self.stream_display.show_monitoring_data( + managed_resources, info + ) + + except ClientError as e: + if ( + e.response.get('Error', {}).get('Code') + == 'InvalidParameterException' + ): + error_message = e.response.get('Error', {}).get( + 'Message', '' + ) + if ( + "Cannot call DescribeServiceRevisions for a service that is INACTIVE" + in error_message + ): + self.stream_display.show_service_inactive_message() + break + else: + raise + else: + raise + + time.sleep(5.0) + + except KeyboardInterrupt: + self.stream_display.show_user_stop_message() + except MonitoringError as e: + self.stream_display.show_error_message(e) + finally: + self.stream_display.show_completion_message() diff --git a/awscli/customizations/ecs/monitorexpressgatewayservice.py b/awscli/customizations/ecs/monitorexpressgatewayservice.py index a5ed8769cf1c..649cffbbc70e 100644 --- a/awscli/customizations/ecs/monitorexpressgatewayservice.py +++ b/awscli/customizations/ecs/monitorexpressgatewayservice.py @@ -25,6 +25,10 @@ - RESOURCE: Displays all resources associated with the service - DEPLOYMENT: Shows resources that have changed in the most recent deployment +And two display modes: +- INTERACTIVE: Real-time display with spinner and keyboard navigation (requires TTY) +- TEXT-ONLY: Text output with timestamps and change detection (works without TTY) + Key Features: - Real-time progress monitoring with spinner animations - Diff-based resource tracking for deployment changes @@ -36,7 +40,7 @@ ECSExpressGatewayServiceWatcher: Core monitoring logic and resource tracking Usage: - aws ecs monitor-express-gateway-service --service-arn [--resource-view RESOURCE|DEPLOYMENT] + aws ecs monitor-express-gateway-service --service-arn [--resource-view RESOURCE|DEPLOYMENT] [--mode INTERACTIVE|TEXT-ONLY] """ import sys @@ -48,6 +52,7 @@ from awscli.customizations.ecs.exceptions import MonitoringError from awscli.customizations.ecs.expressgateway.display_strategy import ( InteractiveDisplayStrategy, + TextOnlyDisplayStrategy, ) from awscli.customizations.ecs.prompt_toolkit_display import Display from awscli.customizations.ecs.serviceviewcollector import ServiceViewCollector @@ -65,14 +70,16 @@ class ECSMonitorExpressGatewayService(BasicCommand): DESCRIPTION = ( "Monitors the progress of resource creation for an ECS Express Gateway Service. " - "This command provides real-time monitoring of service deployments with interactive " - "progress display, showing the status of load balancers, security groups, auto-scaling " + "This command provides real-time monitoring of service deployments showing the status " + "of load balancers, security groups, auto-scaling " "configurations, and other AWS resources as they are created or updated. " "Use ``--resource-view RESOURCE`` to view all service resources, or ``--resource-view DEPLOYMENT`` to track only " "resources that have changed in the most recent deployment. " - "The command requires a terminal (TTY) to run and the monitoring session continues " - "until manually stopped by the user or the specified timeout is reached. " - "Use keyboard shortcuts to navigate: up/down to scroll through resources, 'q' to quit monitoring." + "Choose ``--mode INTERACTIVE`` for real-time display with keyboard navigation (requires TTY), " + "or ``--mode text-only`` for text output with timestamps (works without TTY). " + "The monitoring session continues until manually stopped by the user or the specified timeout is reached. " + "In interactive mode, use keyboard shortcuts: up/down to scroll through resources, 'q' to quit. " + "In TEXT-ONLY mode, press Ctrl+C to stop monitoring." ) ARG_TABLE = [ @@ -97,6 +104,16 @@ class ECSMonitorExpressGatewayService(BasicCommand): 'default': 'RESOURCE', 'choices': ['RESOURCE', 'DEPLOYMENT'], }, + { + 'name': 'mode', + 'help_text': ( + "Display mode for monitoring output. " + "interactive (default if TTY available) - Real-time display with spinner and keyboard navigation. " + "text-only - Text output with timestamps and change detection (works without TTY)." + ), + 'required': False, + 'choices': ['interactive', 'text-only'], + }, { 'name': 'timeout', 'help_text': ( @@ -127,15 +144,12 @@ def _run_main(self, parsed_args, parsed_globals): parsed_globals: Global CLI configuration including region and endpoint """ try: - # Check if running in a TTY for interactive display - if not sys.stdout.isatty(): - uni_print( - "Error: This command requires a TTY. " - "Please run this command in a terminal.", - sys.stderr, - ) - return 1 + display_mode = self._determine_display_mode(parsed_args.mode) + except ValueError as e: + uni_print(str(e), sys.stderr) + return 1 + try: self._client = self._session.create_client( 'ecs', region_name=parsed_globals.region, @@ -150,14 +164,52 @@ def _run_main(self, parsed_args, parsed_globals): self._client, parsed_args.service_arn, parsed_args.resource_view, + display_mode, timeout_minutes=parsed_args.timeout, use_color=use_color, ).exec() except MonitoringError as e: uni_print(f"Error monitoring service: {e}", sys.stderr) + return 1 + + def _determine_display_mode(self, requested_mode): + """Determine and validate the display mode. + + Args: + requested_mode: User-requested mode ('interactive', 'text-only', or None) + + Returns: + str: Validated display mode ('interactive' or 'text-only') + + Raises: + ValueError: If interactive mode is requested without TTY + """ + # Determine display mode with auto-detection + if requested_mode is None: + # Auto-detect: interactive if TTY available, else text-only + return 'interactive' if sys.stdout.isatty() else 'text-only' + + # Validate requested mode + if requested_mode == 'interactive': + if not sys.stdout.isatty(): + raise ValueError( + "Error: Interactive mode requires a TTY (terminal). " + "Use --mode text-only for non-interactive environments." + ) + return 'interactive' + + # text-only mode doesn't require TTY + return requested_mode def _should_use_color(self, parsed_globals): - """Determine if color output should be used based on global settings.""" + """Determine if color output should be used based on global settings. + + Args: + parsed_globals: Global CLI configuration + + Returns: + bool: True if color should be used + """ if parsed_globals.color == 'on': return True elif parsed_globals.color == 'off': @@ -176,7 +228,8 @@ class ECSExpressGatewayServiceWatcher: Args: client: ECS client for API calls service_arn (str): ARN of the service to monitor - mode (str): Monitoring mode - 'RESOURCE' or 'DEPLOYMENT' + resource_view (str): Resource view mode - 'RESOURCE' or 'DEPLOYMENT' + display_mode (str): Display mode - 'INTERACTIVE' or 'TEXT-ONLY' timeout_minutes (int): Maximum monitoring time in minutes (default: 30) """ @@ -184,7 +237,8 @@ def __init__( self, client, service_arn, - mode, + resource_view, + display_mode, timeout_minutes=30, display_strategy=None, use_color=True, @@ -192,15 +246,15 @@ def __init__( ): self._client = client self.service_arn = service_arn - self.mode = mode + self.display_mode = display_mode self.timeout_minutes = timeout_minutes self.start_time = time.time() self.use_color = use_color - self.display_strategy = display_strategy or InteractiveDisplayStrategy( - display=Display(), use_color=use_color + self.display_strategy = ( + display_strategy or self._create_display_strategy() ) self.collector = collector or ServiceViewCollector( - client, service_arn, mode, use_color + client, service_arn, resource_view, use_color ) @staticmethod @@ -209,7 +263,22 @@ def is_monitoring_available(): return sys.stdout.isatty() def exec(self): - """Start monitoring the express gateway service with progress display.""" + """Execute monitoring using the appropriate display strategy.""" self.display_strategy.execute_monitoring( - self.collector, self.timeout_minutes, self.start_time + collector=self.collector, + start_time=self.start_time, + timeout_minutes=self.timeout_minutes, ) + + def _create_display_strategy(self): + """Create display strategy based on display mode. + + Returns: + DisplayStrategy: Appropriate strategy for the selected mode + """ + if self.display_mode == 'text-only': + return TextOnlyDisplayStrategy(use_color=self.use_color) + else: + return InteractiveDisplayStrategy( + display=Display(), use_color=self.use_color + ) diff --git a/awscli/customizations/ecs/serviceviewcollector.py b/awscli/customizations/ecs/serviceviewcollector.py index 21a28e19a01f..4aac5d600fcd 100644 --- a/awscli/customizations/ecs/serviceviewcollector.py +++ b/awscli/customizations/ecs/serviceviewcollector.py @@ -248,9 +248,7 @@ def _diff_service_view(self, describe_gateway_service_response): source_sr_resources_combined = ManagedResourceGroup() updating_resources, disassociating_resources = ( - target_sr_resources.diff( - source_sr_resources_combined - ) + target_sr_resources.diff(source_sr_resources_combined) ) updating_resources.resource_type = "Updating" disassociating_resources.resource_type = "Disassociating" diff --git a/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py index 119b3d252a43..1f52afff5a60 100644 --- a/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py +++ b/tests/unit/customizations/ecs/expressgateway/test_display_strategy.py @@ -18,6 +18,7 @@ from awscli.customizations.ecs.expressgateway.display_strategy import ( DisplayStrategy, InteractiveDisplayStrategy, + TextOnlyDisplayStrategy, ) @@ -230,3 +231,64 @@ def test_display_cleanup_on_exception( # Verify app.exit() was called in finally block despite exception mock_display.app.exit.assert_called() + + +class TestTextOnlyDisplayStrategy: + """Test TextOnlyDisplayStrategy.""" + + @patch('time.sleep') + def test_execute_with_mock_collector(self, mock_sleep, capsys): + """Test strategy executes sync loop with text output.""" + mock_collector = Mock() + mock_collector.get_current_view = Mock(return_value="Test output") + mock_collector.cached_monitor_result = (None, "Test info") + + strategy = TextOnlyDisplayStrategy(use_color=True) + + # Make sleep raise to exit loop after first iteration + mock_sleep.side_effect = KeyboardInterrupt() + + start_time = time.time() + strategy.execute_monitoring( + mock_collector, start_time, timeout_minutes=1 + ) + + output = capsys.readouterr().out + printed_output = output + assert "Starting monitoring" in printed_output + assert "Polling for updates" in printed_output + assert "stopped by user" in printed_output + assert "complete" in printed_output + + @patch('time.sleep') + @patch('time.time') + def test_execute_handles_timeout(self, mock_time, mock_sleep, capsys): + """Test strategy handles timeout correctly.""" + mock_collector = Mock() + mock_collector.get_current_view = Mock(return_value="Test output") + mock_collector.cached_monitor_result = (None, None) + + strategy = TextOnlyDisplayStrategy(use_color=True) + + # Simulate timeout after first poll + start_time = 1000.0 + mock_time.side_effect = [ + 1000.0, # First check - within timeout + 2000.0, # Second check - exceeded timeout + ] + + strategy.execute_monitoring( + mock_collector, start_time, timeout_minutes=1 + ) + + output = capsys.readouterr().out + printed_output = output + assert "timeout reached" in printed_output.lower() + + def test_strategy_uses_provided_color_setting(self): + """Test strategy respects use_color parameter.""" + strategy_with_color = TextOnlyDisplayStrategy(use_color=True) + assert strategy_with_color.stream_display.use_color is True + + strategy_no_color = TextOnlyDisplayStrategy(use_color=False) + assert strategy_no_color.stream_display.use_color is False diff --git a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py index 6edecc02b4bf..8230009cbc4c 100644 --- a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py @@ -77,44 +77,265 @@ def test_non_monitoring_error_bubbles_up(self, mock_isatty): @patch('sys.stdout.isatty') def test_interactive_mode_requires_tty(self, mock_isatty, capsys): - """Test command fails when not in TTY""" + """Test interactive mode fails without TTY""" # Not in TTY mock_isatty.return_value = False mock_session = Mock() + mock_client = Mock() + mock_session.create_client.return_value = mock_client + command = ECSMonitorExpressGatewayService(mock_session) parsed_args = Mock( - service_arn="test-arn", resource_view="RESOURCE", timeout=30 + service_arn="test-arn", + resource_view="RESOURCE", + timeout=30, + mode='interactive', # Explicitly request interactive mode ) parsed_globals = Mock( - region="us-west-2", endpoint_url=None, verify_ssl=True + region="us-west-2", + endpoint_url=None, + verify_ssl=True, + color='auto', ) result = command._run_main(parsed_args, parsed_globals) captured = capsys.readouterr() + assert "Interactive mode requires a TTY" in captured.err assert result == 1 - assert "This command requires a TTY" in captured.err + + @patch('sys.stdout.isatty') + def test_text_only_mode_without_tty(self, mock_isatty, capsys): + """Test command uses text-only mode when not in TTY""" + # Not in TTY + mock_isatty.return_value = False + + mock_session = Mock() + mock_client = Mock() + mock_session.create_client.return_value = mock_client + + mock_watcher_class = Mock() + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + command = ECSMonitorExpressGatewayService( + mock_session, watcher_class=mock_watcher_class + ) + + parsed_args = Mock( + service_arn="test-arn", + resource_view="RESOURCE", + timeout=30, + mode=None, + ) + parsed_globals = Mock( + region="us-west-2", endpoint_url=None, verify_ssl=True + ) + + command._run_main(parsed_args, parsed_globals) class TestECSExpressGatewayServiceWatcher: """Test the watcher class through public interface""" - @patch('sys.stdout.isatty') - def test_is_monitoring_available_with_tty(self, mock_isatty): - """Test is_monitoring_available returns True when TTY is available""" - mock_isatty.return_value = True + def setup_method(self): + self.app_session = create_app_session(output=DummyOutput()) + self.app_session.__enter__() + self.mock_client = Mock() + self.service_arn = ( + "arn:aws:ecs:us-west-2:123456789012:service/my-cluster/my-service" + ) + + def teardown_method(self): + if hasattr(self, 'app_session'): + self.app_session.__exit__(None, None, None) + + def _create_watcher_with_mocks( + self, resource_view="RESOURCE", timeout=1, display_mode="interactive" + ): + """Helper to create watcher with mocked display strategy""" + mock_display_strategy = Mock() + + # Create watcher with mocked strategy + watcher = ECSExpressGatewayServiceWatcher( + self.mock_client, + self.service_arn, + resource_view, + display_mode, + timeout_minutes=timeout, + display_strategy=mock_display_strategy, + ) + + # Mock exec to call the collector once and print output + collector = watcher.collector + + def mock_exec(): + try: + output = collector.get_current_view("⠋") + print(output) + print("Monitoring Complete!") + except Exception as e: + # Re-raise expected exceptions + if isinstance(e, (ClientError, MonitoringError)): + raise + # For other exceptions, just print and complete + print("Monitoring Complete!") + + watcher.exec = mock_exec + return watcher + + @patch('time.sleep') + def test_exec_successful_all_mode_monitoring(self, mock_sleep, capsys): + """Test successful monitoring in RESOURCE mode with resource parsing""" + watcher = self._create_watcher_with_mocks() + mock_sleep.side_effect = KeyboardInterrupt() + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], + } + } + self.mock_client.describe_service_revisions.return_value = { + "serviceRevisions": [ + { + "arn": "rev-arn", + "ecsManagedResources": { + "ingressPaths": [ + { + "endpoint": "https://api.example.com", + "loadBalancer": { + "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/my-lb/1234567890abcdef", + "status": "ACTIVE", + }, + "targetGroups": [ + { + "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/my-tg/1234567890abcdef", + "status": "HEALTHY", + } + ], + } + ], + "serviceSecurityGroups": [ + { + "arn": "arn:aws:ec2:us-west-2:123456789012:security-group/sg-1234567890abcdef0", + "status": "ACTIVE", + } + ], + "logGroups": [ + { + "arn": "arn:aws:logs:us-west-2:123456789012:log-group:/aws/ecs/my-service", + "status": "ACTIVE", + } + ], + }, + } + ] + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Running"}]}] + } + + watcher.exec() + captured = capsys.readouterr() + output_text = captured.out + + # Verify parsed resources appear in output + assert "Cluster" in output_text + assert "Service" in output_text + assert "IngressPath" in output_text + assert "LoadBalancer" in output_text + assert "TargetGroup" in output_text + assert "SecurityGroup" in output_text + assert "LogGroup" in output_text + + # Specific identifiers + assert "https://api.example.com" in output_text # IngressPath endpoint + assert "my-lb" in output_text # LoadBalancer identifier + assert "my-tg" in output_text # TargetGroup identifier assert ( - ECSExpressGatewayServiceWatcher.is_monitoring_available() is True + "sg-1234567890abcdef0" in output_text + ) # SecurityGroup identifier + assert "/aws/ecs/my-service" in output_text # LogGroup identifier + + # Status values + assert "ACTIVE" in output_text # LoadBalancer and SecurityGroup status + assert "HEALTHY" in output_text # TargetGroup status + + @patch('time.sleep') + def test_exec_successful_delta_mode_with_deployment( + self, mock_sleep, capsys + ): + """Test DEPLOYMENT mode executes successfully""" + watcher = self._create_watcher_with_mocks() + mock_sleep.side_effect = KeyboardInterrupt() + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + self.mock_client.describe_services.return_value = { + "services": [{"events": [{"message": "Service running"}]}] + } + + watcher.exec() + captured = capsys.readouterr() + + # Verify DEPLOYMENT mode executes successfully + assert captured.out + + @patch('time.sleep') + def test_exec_keyboard_interrupt_handling(self, mock_sleep, capsys): + watcher = self._create_watcher_with_mocks() + mock_sleep.side_effect = KeyboardInterrupt() + + self.mock_client.describe_express_gateway_service.return_value = { + "service": { + "serviceArn": self.service_arn, + "cluster": "my-cluster", + "activeConfigurations": [], + } + } + + watcher.exec() + captured = capsys.readouterr() + + # Verify completion message is printed + assert "Monitoring Complete!" in captured.out + + @patch('time.sleep') + def test_exec_with_service_not_found_error(self, mock_sleep): + """Test exec() with service not found error bubbles up""" + watcher = self._create_watcher_with_mocks() + mock_sleep.side_effect = KeyboardInterrupt() + + error = ClientError( + error_response={ + 'Error': { + 'Code': 'ServiceNotFoundException', + 'Message': 'Service not found', + } + }, + operation_name='DescribeExpressGatewayService', ) + self.mock_client.describe_express_gateway_service.side_effect = error - @patch('sys.stdout.isatty') - def test_is_monitoring_available_without_tty(self, mock_isatty): - """Test is_monitoring_available returns False when TTY is not available""" - mock_isatty.return_value = False + with pytest.raises(ClientError) as exc_info: + watcher.exec() + + # Verify the specific error is raised + assert ( + exc_info.value.response['Error']['Code'] + == 'ServiceNotFoundException' + ) assert ( - ECSExpressGatewayServiceWatcher.is_monitoring_available() is False + exc_info.value.response['Error']['Message'] == 'Service not found' ) @@ -184,15 +405,17 @@ def test_watcher_accepts_use_color_parameter(self): mock_client, "arn:aws:ecs:us-east-1:123456789012:service/test-service", "ALL", + "interactive", use_color=True, ) - assert watcher.use_color is True + assert watcher.collector.use_color is True # Test with use_color=False watcher = ECSExpressGatewayServiceWatcher( mock_client, "arn:aws:ecs:us-east-1:123456789012:service/test-service", "ALL", + "interactive", use_color=False, ) - assert watcher.use_color is False + assert watcher.collector.use_color is False diff --git a/tests/unit/customizations/ecs/test_serviceviewcollector.py b/tests/unit/customizations/ecs/test_serviceviewcollector.py index 36ba679c39f3..ebab9fefab83 100644 --- a/tests/unit/customizations/ecs/test_serviceviewcollector.py +++ b/tests/unit/customizations/ecs/test_serviceviewcollector.py @@ -446,9 +446,7 @@ def test_eventually_consistent_mixed_failures(self): } } self.mock_client.list_service_deployments.return_value = { - "serviceDeployments": [ - {"serviceDeploymentArn": "deploy-arn"} - ] + "serviceDeployments": [{"serviceDeploymentArn": "deploy-arn"}] } self.mock_client.describe_service_deployments.return_value = { "serviceDeployments": [ From cce5450d1f7b1401262514730ad53246f9d3d70c Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Tue, 16 Dec 2025 23:18:04 -0500 Subject: [PATCH 11/17] Address PR comments --- .../next-release/enhancement-ecs-63253.json | 5 - .../ecs/monitorexpressgatewayservice.py | 29 +- .../ecs/serviceviewcollector.py | 5 +- .../ecs/test_monitorexpressgatewayservice.py | 289 ++++++++++++++++++ .../ecs/test_monitorexpressgatewayservice.py | 267 ++++++---------- 5 files changed, 402 insertions(+), 193 deletions(-) delete mode 100644 .changes/next-release/enhancement-ecs-63253.json create mode 100644 tests/functional/ecs/test_monitorexpressgatewayservice.py diff --git a/.changes/next-release/enhancement-ecs-63253.json b/.changes/next-release/enhancement-ecs-63253.json deleted file mode 100644 index 3e4985cf1189..000000000000 --- a/.changes/next-release/enhancement-ecs-63253.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "type": "enhancement", - "category": "``ecs``", - "description": "Introduce text-only mode to ECS Express Mode service monitoring APIs" -} diff --git a/awscli/customizations/ecs/monitorexpressgatewayservice.py b/awscli/customizations/ecs/monitorexpressgatewayservice.py index 649cffbbc70e..addd9502d7a1 100644 --- a/awscli/customizations/ecs/monitorexpressgatewayservice.py +++ b/awscli/customizations/ecs/monitorexpressgatewayservice.py @@ -57,6 +57,7 @@ from awscli.customizations.ecs.prompt_toolkit_display import Display from awscli.customizations.ecs.serviceviewcollector import ServiceViewCollector from awscli.customizations.utils import uni_print +from awscli.utils import write_exception class ECSMonitorExpressGatewayService(BasicCommand): @@ -76,7 +77,7 @@ class ECSMonitorExpressGatewayService(BasicCommand): "Use ``--resource-view RESOURCE`` to view all service resources, or ``--resource-view DEPLOYMENT`` to track only " "resources that have changed in the most recent deployment. " "Choose ``--mode INTERACTIVE`` for real-time display with keyboard navigation (requires TTY), " - "or ``--mode text-only`` for text output with timestamps (works without TTY). " + "or ``--mode TEXT-ONLY`` for text output with timestamps (works without TTY). " "The monitoring session continues until manually stopped by the user or the specified timeout is reached. " "In interactive mode, use keyboard shortcuts: up/down to scroll through resources, 'q' to quit. " "In TEXT-ONLY mode, press Ctrl+C to stop monitoring." @@ -108,11 +109,11 @@ class ECSMonitorExpressGatewayService(BasicCommand): 'name': 'mode', 'help_text': ( "Display mode for monitoring output. " - "interactive (default if TTY available) - Real-time display with spinner and keyboard navigation. " - "text-only - Text output with timestamps and change detection (works without TTY)." + "INTERACTIVE (default if TTY available) - Real-time display with spinner and keyboard navigation. " + "TEXT-ONLY - Text output with timestamps and change detection (works without TTY)." ), 'required': False, - 'choices': ['interactive', 'text-only'], + 'choices': ['INTERACTIVE', 'TEXT-ONLY'], }, { 'name': 'timeout', @@ -146,7 +147,7 @@ def _run_main(self, parsed_args, parsed_globals): try: display_mode = self._determine_display_mode(parsed_args.mode) except ValueError as e: - uni_print(str(e), sys.stderr) + write_exception(e, sys.stderr) return 1 try: @@ -187,16 +188,16 @@ def _determine_display_mode(self, requested_mode): # Determine display mode with auto-detection if requested_mode is None: # Auto-detect: interactive if TTY available, else text-only - return 'interactive' if sys.stdout.isatty() else 'text-only' + return 'INTERACTIVE' if sys.stdout.isatty() else 'TEXT-ONLY' # Validate requested mode - if requested_mode == 'interactive': + if requested_mode == 'INTERACTIVE': if not sys.stdout.isatty(): raise ValueError( "Error: Interactive mode requires a TTY (terminal). " - "Use --mode text-only for non-interactive environments." + "Use --mode TEXT-ONLY for non-interactive environments." ) - return 'interactive' + return 'INTERACTIVE' # text-only mode doesn't require TTY return requested_mode @@ -244,7 +245,6 @@ def __init__( use_color=True, collector=None, ): - self._client = client self.service_arn = service_arn self.display_mode = display_mode self.timeout_minutes = timeout_minutes @@ -275,10 +275,15 @@ def _create_display_strategy(self): Returns: DisplayStrategy: Appropriate strategy for the selected mode + + Raises: + ValueError: If display mode is not 'INTERACTIVE' or 'TEXT-ONLY' """ - if self.display_mode == 'text-only': + if self.display_mode == 'TEXT-ONLY': return TextOnlyDisplayStrategy(use_color=self.use_color) - else: + elif self.display_mode == 'INTERACTIVE': return InteractiveDisplayStrategy( display=Display(), use_color=self.use_color ) + else: + raise ValueError(f"Invalid display mode: {self.display_mode}") diff --git a/awscli/customizations/ecs/serviceviewcollector.py b/awscli/customizations/ecs/serviceviewcollector.py index 4aac5d600fcd..1dc308f7e2ab 100644 --- a/awscli/customizations/ecs/serviceviewcollector.py +++ b/awscli/customizations/ecs/serviceviewcollector.py @@ -282,7 +282,10 @@ def _combined_service_view(self, describe_gateway_service_response): self._describe_and_parse_service_revisions(service_revision_arns) ) - if len(service_revision_resources) != len(service_revision_arns): + # If empty, we're still waiting for active configurations + if not service_revision_resources or len( + service_revision_resources + ) != len(service_revision_arns): return (None, "Trying to describe service revisions") service_resource = reduce( diff --git a/tests/functional/ecs/test_monitorexpressgatewayservice.py b/tests/functional/ecs/test_monitorexpressgatewayservice.py new file mode 100644 index 000000000000..2f3f3e71435d --- /dev/null +++ b/tests/functional/ecs/test_monitorexpressgatewayservice.py @@ -0,0 +1,289 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ + +from unittest.mock import Mock, patch + +import pytest + +from awscli.customizations.ecs import inject_commands +from awscli.customizations.ecs.monitorexpressgatewayservice import ( + ECSMonitorExpressGatewayService, +) + + +@pytest.fixture +def mock_watcher_class(): + """Fixture that provides a mock watcher class.""" + return Mock() + + +@pytest.fixture +def mock_session(): + """Fixture that provides a mock session.""" + return Mock() + + +@pytest.fixture +def command(mock_session, mock_watcher_class): + """Fixture that provides an ECSMonitorExpressGatewayService command.""" + return ECSMonitorExpressGatewayService( + mock_session, watcher_class=mock_watcher_class + ) + + +@pytest.fixture +def mock_client(mock_session): + """Fixture that provides a mock ECS client.""" + client = Mock() + mock_session.create_client.return_value = client + return client + + +class TestECSMonitorExpressGatewayService: + def test_init(self, command, mock_session, mock_watcher_class): + assert command.name == 'monitor-express-gateway-service' + assert command.DESCRIPTION.startswith('Monitors the progress') + + def test_add_arguments(self, command): + command._build_arg_table() + arg_table = command.arg_table + + assert 'service-arn' in arg_table + assert 'resource-view' in arg_table + assert 'timeout' in arg_table + assert 'mode' in arg_table + + # Verify resource-view argument has correct choices + resource_view_arg = arg_table['resource-view'] + assert resource_view_arg.choices == ['RESOURCE', 'DEPLOYMENT'] + + # Verify mode argument has correct choices + mode_arg = arg_table['mode'] + assert mode_arg.choices == ['INTERACTIVE', 'TEXT-ONLY'] + + @patch('sys.stdout.isatty', return_value=False) + def test_run_main_with_text_only_mode( + self, mock_isatty, command, mock_watcher_class, mock_client + ): + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + parsed_args = Mock( + service_arn='arn:aws:ecs:us-west-2:123456789012:service/cluster/service', + resource_view='RESOURCE', + timeout=30, + mode='TEXT-ONLY', + ) + parsed_globals = Mock( + region='us-west-2', + endpoint_url=None, + verify_ssl=True, + color='off', + ) + + command._run_main(parsed_args, parsed_globals) + + # Verify watcher was created with correct parameters (positional) + mock_watcher_class.assert_called_once_with( + mock_client, + parsed_args.service_arn, + 'RESOURCE', + 'TEXT-ONLY', + timeout_minutes=30, + use_color=False, + ) + + # Verify watcher was executed + mock_watcher.exec.assert_called_once() + + @patch('sys.stdout.isatty', return_value=True) + def test_run_main_with_interactive_mode( + self, mock_isatty, command, mock_watcher_class, mock_client + ): + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + parsed_args = Mock( + service_arn='arn:aws:ecs:us-west-2:123456789012:service/cluster/service', + resource_view='DEPLOYMENT', + timeout=60, + mode='INTERACTIVE', + ) + parsed_globals = Mock( + region='us-west-2', + endpoint_url=None, + verify_ssl=True, + color='auto', + ) + + command._run_main(parsed_args, parsed_globals) + + # Verify watcher was created with correct mode + mock_watcher_class.assert_called_once_with( + mock_client, + parsed_args.service_arn, + 'DEPLOYMENT', + 'INTERACTIVE', + timeout_minutes=60, + use_color=True, + ) + + @patch('sys.stdout.isatty', return_value=True) + def test_run_main_auto_mode_with_tty( + self, mock_isatty, command, mock_watcher_class, mock_client + ): + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + parsed_args = Mock( + service_arn='arn:aws:ecs:us-west-2:123456789012:service/cluster/service', + resource_view='RESOURCE', + timeout=30, + mode=None, # Auto mode + ) + parsed_globals = Mock( + region='us-west-2', + endpoint_url=None, + verify_ssl=True, + color='auto', + ) + + command._run_main(parsed_args, parsed_globals) + + # When mode is None and TTY is available, should use INTERACTIVE + args = mock_watcher_class.call_args[0] + assert args[3] == 'INTERACTIVE' + + @patch('sys.stdout.isatty', return_value=False) + def test_run_main_auto_mode_without_tty( + self, mock_isatty, command, mock_watcher_class, mock_client + ): + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + parsed_args = Mock( + service_arn='arn:aws:ecs:us-west-2:123456789012:service/cluster/service', + resource_view='RESOURCE', + timeout=30, + mode=None, # Auto mode + ) + parsed_globals = Mock( + region='us-west-2', + endpoint_url=None, + verify_ssl=True, + color='auto', + ) + + command._run_main(parsed_args, parsed_globals) + + # When mode is None and TTY is not available, should use TEXT-ONLY + args = mock_watcher_class.call_args[0] + assert args[3] == 'TEXT-ONLY' + + @patch('sys.stdout.isatty', return_value=False) + def test_run_main_with_color_on( + self, mock_isatty, command, mock_watcher_class, mock_client + ): + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + parsed_args = Mock( + service_arn='arn:aws:ecs:us-west-2:123456789012:service/cluster/service', + resource_view='RESOURCE', + timeout=30, + mode='TEXT-ONLY', + ) + parsed_globals = Mock( + region='us-west-2', + endpoint_url=None, + verify_ssl=True, + color='on', + ) + + command._run_main(parsed_args, parsed_globals) + + # Verify color setting is True when color='on' + call_kwargs = mock_watcher_class.call_args[1] + assert call_kwargs['use_color'] is True + + @patch('sys.stdout.isatty', return_value=False) + def test_run_main_creates_ecs_client( + self, + mock_isatty, + command, + mock_session, + mock_watcher_class, + mock_client, + ): + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + parsed_args = Mock( + service_arn='arn:aws:ecs:us-west-2:123456789012:service/cluster/service', + resource_view='RESOURCE', + timeout=30, + mode='TEXT-ONLY', + ) + parsed_globals = Mock( + region='us-west-2', + endpoint_url=None, + verify_ssl=True, + color='off', + ) + + command._run_main(parsed_args, parsed_globals) + + # Verify ECS client was created with correct parameters + mock_session.create_client.assert_called_once_with( + 'ecs', + region_name='us-west-2', + endpoint_url=None, + verify=True, + ) + + # Verify client was passed to watcher + args = mock_watcher_class.call_args[0] + assert args[0] == mock_client + + @patch('sys.stdout.isatty', return_value=False) + def test_run_main_with_default_resource_view( + self, mock_isatty, command, mock_watcher_class, mock_client + ): + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + parsed_args = Mock( + service_arn='arn:aws:ecs:us-west-2:123456789012:service/cluster/service', + resource_view=None, # Not specified, should use default + timeout=30, + mode='TEXT-ONLY', + ) + parsed_globals = Mock( + region='us-west-2', + endpoint_url=None, + verify_ssl=True, + color='off', + ) + + command._run_main(parsed_args, parsed_globals) + + # Verify default resource view is passed + args = mock_watcher_class.call_args[0] + assert args[2] is None # Resource view is passed as-is + + +class TestCommandRegistration: + def test_inject_commands_registers_monitor_command(self, mock_session): + command_table = {} + + inject_commands(command_table, mock_session) + + # Verify monitor command is registered + assert 'monitor-express-gateway-service' in command_table + command = command_table['monitor-express-gateway-service'] + assert isinstance(command, ECSMonitorExpressGatewayService) diff --git a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py index 8230009cbc4c..18353e6a4168 100644 --- a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py @@ -91,7 +91,7 @@ def test_interactive_mode_requires_tty(self, mock_isatty, capsys): service_arn="test-arn", resource_view="RESOURCE", timeout=30, - mode='interactive', # Explicitly request interactive mode + mode='INTERACTIVE', ) parsed_globals = Mock( region="us-west-2", @@ -137,185 +137,90 @@ def test_text_only_mode_without_tty(self, mock_isatty, capsys): command._run_main(parsed_args, parsed_globals) +@pytest.fixture +def watcher_app_session(): + """Fixture that creates and manages an app session for watcher tests.""" + with create_app_session(output=DummyOutput()) as session: + yield session + + +@pytest.fixture +def service_arn(): + """Fixture that provides a test service ARN.""" + return "arn:aws:ecs:us-west-2:123456789012:service/my-cluster/my-service" + + class TestECSExpressGatewayServiceWatcher: """Test the watcher class through public interface""" - def setup_method(self): - self.app_session = create_app_session(output=DummyOutput()) - self.app_session.__enter__() - self.mock_client = Mock() - self.service_arn = ( - "arn:aws:ecs:us-west-2:123456789012:service/my-cluster/my-service" + def test_init_creates_collector_with_correct_parameters(self): + """Test watcher creates collector with correct client, service_arn, resource_view, use_color""" + mock_client = Mock() + service_arn = "arn:aws:ecs:us-west-2:123456789012:service/test-service" + + watcher = ECSExpressGatewayServiceWatcher( + mock_client, + service_arn, + resource_view="DEPLOYMENT", + display_mode="TEXT-ONLY", + use_color=False, ) - def teardown_method(self): - if hasattr(self, 'app_session'): - self.app_session.__exit__(None, None, None) + # Verify collector was created with correct parameters + assert watcher.collector is not None + assert watcher.collector._client == mock_client + assert watcher.collector.service_arn == service_arn + assert watcher.collector.mode == "DEPLOYMENT" + assert watcher.collector.use_color is False - def _create_watcher_with_mocks( - self, resource_view="RESOURCE", timeout=1, display_mode="interactive" - ): - """Helper to create watcher with mocked display strategy""" - mock_display_strategy = Mock() + def test_init_uses_injected_collector(self): + """Test watcher uses injected collector instead of creating one""" + mock_collector = Mock() - # Create watcher with mocked strategy watcher = ECSExpressGatewayServiceWatcher( - self.mock_client, - self.service_arn, - resource_view, - display_mode, - timeout_minutes=timeout, - display_strategy=mock_display_strategy, + Mock(), + "arn:aws:ecs:us-west-2:123456789012:service/test-service", + "RESOURCE", + "INTERACTIVE", + collector=mock_collector, ) - # Mock exec to call the collector once and print output - collector = watcher.collector - - def mock_exec(): - try: - output = collector.get_current_view("⠋") - print(output) - print("Monitoring Complete!") - except Exception as e: - # Re-raise expected exceptions - if isinstance(e, (ClientError, MonitoringError)): - raise - # For other exceptions, just print and complete - print("Monitoring Complete!") - - watcher.exec = mock_exec - return watcher - - @patch('time.sleep') - def test_exec_successful_all_mode_monitoring(self, mock_sleep, capsys): - """Test successful monitoring in RESOURCE mode with resource parsing""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [{"serviceRevisionArn": "rev-arn"}], - } - } - self.mock_client.describe_service_revisions.return_value = { - "serviceRevisions": [ - { - "arn": "rev-arn", - "ecsManagedResources": { - "ingressPaths": [ - { - "endpoint": "https://api.example.com", - "loadBalancer": { - "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/my-lb/1234567890abcdef", - "status": "ACTIVE", - }, - "targetGroups": [ - { - "arn": "arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/my-tg/1234567890abcdef", - "status": "HEALTHY", - } - ], - } - ], - "serviceSecurityGroups": [ - { - "arn": "arn:aws:ec2:us-west-2:123456789012:security-group/sg-1234567890abcdef0", - "status": "ACTIVE", - } - ], - "logGroups": [ - { - "arn": "arn:aws:logs:us-west-2:123456789012:log-group:/aws/ecs/my-service", - "status": "ACTIVE", - } - ], - }, - } - ] - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Running"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - output_text = captured.out - - # Verify parsed resources appear in output - assert "Cluster" in output_text - assert "Service" in output_text - assert "IngressPath" in output_text - assert "LoadBalancer" in output_text - assert "TargetGroup" in output_text - assert "SecurityGroup" in output_text - assert "LogGroup" in output_text - - # Specific identifiers - assert "https://api.example.com" in output_text # IngressPath endpoint - assert "my-lb" in output_text # LoadBalancer identifier - assert "my-tg" in output_text # TargetGroup identifier - assert ( - "sg-1234567890abcdef0" in output_text - ) # SecurityGroup identifier - assert "/aws/ecs/my-service" in output_text # LogGroup identifier + assert watcher.collector == mock_collector - # Status values - assert "ACTIVE" in output_text # LoadBalancer and SecurityGroup status - assert "HEALTHY" in output_text # TargetGroup status - - @patch('time.sleep') - def test_exec_successful_delta_mode_with_deployment( - self, mock_sleep, capsys + def test_exec_calls_display_strategy_with_correct_parameters( + self, watcher_app_session ): - """Test DEPLOYMENT mode executes successfully""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() - - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [], - } - } - self.mock_client.describe_services.return_value = { - "services": [{"events": [{"message": "Service running"}]}] - } - - watcher.exec() - captured = capsys.readouterr() - - # Verify DEPLOYMENT mode executes successfully - assert captured.out - - @patch('time.sleep') - def test_exec_keyboard_interrupt_handling(self, mock_sleep, capsys): - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() + """Test exec() calls display strategy with collector, start_time, and timeout""" + mock_collector = Mock() + mock_display_strategy = Mock() - self.mock_client.describe_express_gateway_service.return_value = { - "service": { - "serviceArn": self.service_arn, - "cluster": "my-cluster", - "activeConfigurations": [], - } - } + watcher = ECSExpressGatewayServiceWatcher( + Mock(), + "arn:aws:ecs:us-west-2:123456789012:service/test-service", + "RESOURCE", + "INTERACTIVE", + timeout_minutes=15, + display_strategy=mock_display_strategy, + collector=mock_collector, + ) watcher.exec() - captured = capsys.readouterr() - # Verify completion message is printed - assert "Monitoring Complete!" in captured.out + # Verify display strategy was called once + mock_display_strategy.execute_monitoring.assert_called_once() - @patch('time.sleep') - def test_exec_with_service_not_found_error(self, mock_sleep): - """Test exec() with service not found error bubbles up""" - watcher = self._create_watcher_with_mocks() - mock_sleep.side_effect = KeyboardInterrupt() + # Verify correct parameters were passed + call_args = mock_display_strategy.execute_monitoring.call_args + assert call_args.kwargs['collector'] == mock_collector + assert call_args.kwargs['start_time'] == watcher.start_time + assert call_args.kwargs['timeout_minutes'] == 15 - error = ClientError( + def test_exec_propagates_exceptions_from_display_strategy( + self, watcher_app_session + ): + """Test exec() propagates exceptions from display strategy""" + mock_display_strategy = Mock() + mock_display_strategy.execute_monitoring.side_effect = ClientError( error_response={ 'Error': { 'Code': 'ServiceNotFoundException', @@ -324,12 +229,19 @@ def test_exec_with_service_not_found_error(self, mock_sleep): }, operation_name='DescribeExpressGatewayService', ) - self.mock_client.describe_express_gateway_service.side_effect = error + + watcher = ECSExpressGatewayServiceWatcher( + Mock(), + "arn:aws:ecs:us-west-2:123456789012:service/test-service", + "RESOURCE", + "INTERACTIVE", + display_strategy=mock_display_strategy, + collector=Mock(), + ) with pytest.raises(ClientError) as exc_info: watcher.exec() - # Verify the specific error is raised assert ( exc_info.value.response['Error']['Code'] == 'ServiceNotFoundException' @@ -352,14 +264,6 @@ def test_monitoring_error_creation(self): class TestColorSupport: """Test color support functionality""" - def setup_method(self): - self.app_session = create_app_session(output=DummyOutput()) - self.app_session.__enter__() - - def teardown_method(self): - if hasattr(self, 'app_session'): - self.app_session.__exit__(None, None, None) - def test_should_use_color_on(self): """Test _should_use_color returns True when color is 'on'""" command = ECSMonitorExpressGatewayService(Mock()) @@ -396,7 +300,7 @@ def test_should_use_color_auto_no_tty(self, mock_isatty): assert command._should_use_color(parsed_globals) is False - def test_watcher_accepts_use_color_parameter(self): + def test_watcher_accepts_use_color_parameter(self, watcher_app_session): """Test ECSExpressGatewayServiceWatcher accepts use_color parameter""" mock_client = Mock() @@ -405,7 +309,7 @@ def test_watcher_accepts_use_color_parameter(self): mock_client, "arn:aws:ecs:us-east-1:123456789012:service/test-service", "ALL", - "interactive", + "INTERACTIVE", use_color=True, ) assert watcher.collector.use_color is True @@ -415,7 +319,20 @@ def test_watcher_accepts_use_color_parameter(self): mock_client, "arn:aws:ecs:us-east-1:123456789012:service/test-service", "ALL", - "interactive", + "INTERACTIVE", use_color=False, ) assert watcher.collector.use_color is False + + def test_invalid_display_mode_raises_error(self): + """Test that invalid display mode raises ValueError""" + mock_client = Mock() + + with pytest.raises(ValueError) as exc_info: + ECSExpressGatewayServiceWatcher( + mock_client, + "arn:aws:ecs:us-east-1:123456789012:service/test-service", + "RESOURCE", + "INVALID_MODE", + ) + assert "Invalid display mode: INVALID_MODE" in str(exc_info.value) From 7399df11ce3198329637a7bccc8d445ef8ba9ddf Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Wed, 17 Dec 2025 12:22:51 -0500 Subject: [PATCH 12/17] Address PR comments --- .../ecs/expressgateway/display_strategy.py | 19 ++++++++++++------- .../ecs/monitorexpressgatewayservice.py | 7 +++---- .../ecs/test_monitorexpressgatewayservice.py | 10 ++++++++-- .../ecs/test_monitorexpressgatewayservice.py | 1 + .../ecs/test_prompt_toolkit_display.py | 2 +- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/awscli/customizations/ecs/expressgateway/display_strategy.py b/awscli/customizations/ecs/expressgateway/display_strategy.py index 6d3c9ed54b17..607f8e9fa137 100644 --- a/awscli/customizations/ecs/expressgateway/display_strategy.py +++ b/awscli/customizations/ecs/expressgateway/display_strategy.py @@ -17,6 +17,7 @@ import time from botocore.exceptions import ClientError +from colorama import Style from awscli.customizations.ecs.exceptions import MonitoringError from awscli.customizations.ecs.expressgateway.stream_display import ( @@ -63,13 +64,16 @@ def __init__(self, display, use_color): def execute_monitoring(self, collector, start_time, timeout_minutes): """Execute async monitoring with spinner and keyboard controls.""" - final_output, timed_out = asyncio.run( - self._execute_async(collector, start_time, timeout_minutes) - ) - if timed_out: - uni_print(final_output + "\nMonitoring timed out!\n") - else: - uni_print(final_output + "\nMonitoring Complete!\n") + try: + final_output, timed_out = asyncio.run( + self._execute_async(collector, start_time, timeout_minutes) + ) + if timed_out: + uni_print(final_output + "\nMonitoring timed out!\n") + else: + uni_print(final_output + "\nMonitoring Complete!\n") + finally: + uni_print(Style.RESET_ALL) async def _execute_async(self, collector, start_time, timeout_minutes): """Async execution with dual tasks for data and spinner.""" @@ -233,3 +237,4 @@ def execute_monitoring(self, collector, start_time, timeout_minutes): self.stream_display.show_error_message(e) finally: self.stream_display.show_completion_message() + uni_print(Style.RESET_ALL) diff --git a/awscli/customizations/ecs/monitorexpressgatewayservice.py b/awscli/customizations/ecs/monitorexpressgatewayservice.py index addd9502d7a1..66128f619b5e 100644 --- a/awscli/customizations/ecs/monitorexpressgatewayservice.py +++ b/awscli/customizations/ecs/monitorexpressgatewayservice.py @@ -57,7 +57,6 @@ from awscli.customizations.ecs.prompt_toolkit_display import Display from awscli.customizations.ecs.serviceviewcollector import ServiceViewCollector from awscli.customizations.utils import uni_print -from awscli.utils import write_exception class ECSMonitorExpressGatewayService(BasicCommand): @@ -79,7 +78,7 @@ class ECSMonitorExpressGatewayService(BasicCommand): "Choose ``--mode INTERACTIVE`` for real-time display with keyboard navigation (requires TTY), " "or ``--mode TEXT-ONLY`` for text output with timestamps (works without TTY). " "The monitoring session continues until manually stopped by the user or the specified timeout is reached. " - "In interactive mode, use keyboard shortcuts: up/down to scroll through resources, 'q' to quit. " + "In INTERACTIVE mode, use keyboard shortcuts: up/down to scroll through resources, 'q' to quit. " "In TEXT-ONLY mode, press Ctrl+C to stop monitoring." ) @@ -147,7 +146,7 @@ def _run_main(self, parsed_args, parsed_globals): try: display_mode = self._determine_display_mode(parsed_args.mode) except ValueError as e: - write_exception(e, sys.stderr) + uni_print(f"aws: [ERROR]: {str(e)}", sys.stderr) return 1 try: @@ -194,7 +193,7 @@ def _determine_display_mode(self, requested_mode): if requested_mode == 'INTERACTIVE': if not sys.stdout.isatty(): raise ValueError( - "Error: Interactive mode requires a TTY (terminal). " + "Interactive mode requires a TTY (terminal). " "Use --mode TEXT-ONLY for non-interactive environments." ) return 'INTERACTIVE' diff --git a/tests/functional/ecs/test_monitorexpressgatewayservice.py b/tests/functional/ecs/test_monitorexpressgatewayservice.py index 2f3f3e71435d..a0ce06f56743 100644 --- a/tests/functional/ecs/test_monitorexpressgatewayservice.py +++ b/tests/functional/ecs/test_monitorexpressgatewayservice.py @@ -40,12 +40,11 @@ def command(mock_session, mock_watcher_class): def mock_client(mock_session): """Fixture that provides a mock ECS client.""" client = Mock() - mock_session.create_client.return_value = client return client class TestECSMonitorExpressGatewayService: - def test_init(self, command, mock_session, mock_watcher_class): + def test_init(self, command): assert command.name == 'monitor-express-gateway-service' assert command.DESCRIPTION.startswith('Monitors the progress') @@ -70,6 +69,7 @@ def test_add_arguments(self, command): def test_run_main_with_text_only_mode( self, mock_isatty, command, mock_watcher_class, mock_client ): + command._session.create_client.return_value = mock_client mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -105,6 +105,7 @@ def test_run_main_with_text_only_mode( def test_run_main_with_interactive_mode( self, mock_isatty, command, mock_watcher_class, mock_client ): + command._session.create_client.return_value = mock_client mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -137,6 +138,7 @@ def test_run_main_with_interactive_mode( def test_run_main_auto_mode_with_tty( self, mock_isatty, command, mock_watcher_class, mock_client ): + command._session.create_client.return_value = mock_client mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -163,6 +165,7 @@ def test_run_main_auto_mode_with_tty( def test_run_main_auto_mode_without_tty( self, mock_isatty, command, mock_watcher_class, mock_client ): + command._session.create_client.return_value = mock_client mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -189,6 +192,7 @@ def test_run_main_auto_mode_without_tty( def test_run_main_with_color_on( self, mock_isatty, command, mock_watcher_class, mock_client ): + command._session.create_client.return_value = mock_client mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -220,6 +224,7 @@ def test_run_main_creates_ecs_client( mock_watcher_class, mock_client, ): + command._session.create_client.return_value = mock_client mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -254,6 +259,7 @@ def test_run_main_creates_ecs_client( def test_run_main_with_default_resource_view( self, mock_isatty, command, mock_watcher_class, mock_client ): + command._session.create_client.return_value = mock_client mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher diff --git a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py index 18353e6a4168..dcfac0c0acd0 100644 --- a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py @@ -104,6 +104,7 @@ def test_interactive_mode_requires_tty(self, mock_isatty, capsys): captured = capsys.readouterr() assert "Interactive mode requires a TTY" in captured.err + assert "aws: [ERROR]:" in captured.err assert result == 1 @patch('sys.stdout.isatty') diff --git a/tests/unit/customizations/ecs/test_prompt_toolkit_display.py b/tests/unit/customizations/ecs/test_prompt_toolkit_display.py index 85517dafebb7..3c47a5cd86eb 100644 --- a/tests/unit/customizations/ecs/test_prompt_toolkit_display.py +++ b/tests/unit/customizations/ecs/test_prompt_toolkit_display.py @@ -13,7 +13,7 @@ class TestPromptToolkitDisplay: @pytest.fixture def display(self): with create_app_session(output=DummyOutput()): - return Display() + yield Display() def test_init(self, display): """Test Display initialization.""" From 1778b240eb21e5eaaad24b66a2a5d194cb3f43f8 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Wed, 17 Dec 2025 13:38:24 -0500 Subject: [PATCH 13/17] Address PR comments --- .../ecs/test_monitorexpressgatewayservice.py | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/tests/functional/ecs/test_monitorexpressgatewayservice.py b/tests/functional/ecs/test_monitorexpressgatewayservice.py index a0ce06f56743..0d9eab356aa0 100644 --- a/tests/functional/ecs/test_monitorexpressgatewayservice.py +++ b/tests/functional/ecs/test_monitorexpressgatewayservice.py @@ -6,7 +6,7 @@ # # http://aws.amazon.com/apache2.0/ -from unittest.mock import Mock, patch +from unittest.mock import ANY, Mock, patch import pytest @@ -37,10 +37,14 @@ def command(mock_session, mock_watcher_class): @pytest.fixture -def mock_client(mock_session): - """Fixture that provides a mock ECS client.""" +def command_with_mock_session(mock_session, mock_watcher_class): + """Fixture that provides command with mock session and client configured.""" client = Mock() - return client + mock_session.create_client.return_value = client + command = ECSMonitorExpressGatewayService( + mock_session, watcher_class=mock_watcher_class + ) + return command class TestECSMonitorExpressGatewayService: @@ -67,9 +71,9 @@ def test_add_arguments(self, command): @patch('sys.stdout.isatty', return_value=False) def test_run_main_with_text_only_mode( - self, mock_isatty, command, mock_watcher_class, mock_client + self, mock_isatty, command_with_mock_session, mock_watcher_class ): - command._session.create_client.return_value = mock_client + command = command_with_mock_session mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -90,7 +94,7 @@ def test_run_main_with_text_only_mode( # Verify watcher was created with correct parameters (positional) mock_watcher_class.assert_called_once_with( - mock_client, + ANY, parsed_args.service_arn, 'RESOURCE', 'TEXT-ONLY', @@ -103,9 +107,9 @@ def test_run_main_with_text_only_mode( @patch('sys.stdout.isatty', return_value=True) def test_run_main_with_interactive_mode( - self, mock_isatty, command, mock_watcher_class, mock_client + self, mock_isatty, command_with_mock_session, mock_watcher_class ): - command._session.create_client.return_value = mock_client + command = command_with_mock_session mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -126,7 +130,7 @@ def test_run_main_with_interactive_mode( # Verify watcher was created with correct mode mock_watcher_class.assert_called_once_with( - mock_client, + ANY, parsed_args.service_arn, 'DEPLOYMENT', 'INTERACTIVE', @@ -136,9 +140,9 @@ def test_run_main_with_interactive_mode( @patch('sys.stdout.isatty', return_value=True) def test_run_main_auto_mode_with_tty( - self, mock_isatty, command, mock_watcher_class, mock_client + self, mock_isatty, command_with_mock_session, mock_watcher_class ): - command._session.create_client.return_value = mock_client + command = command_with_mock_session mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -163,9 +167,9 @@ def test_run_main_auto_mode_with_tty( @patch('sys.stdout.isatty', return_value=False) def test_run_main_auto_mode_without_tty( - self, mock_isatty, command, mock_watcher_class, mock_client + self, mock_isatty, command_with_mock_session, mock_watcher_class ): - command._session.create_client.return_value = mock_client + command = command_with_mock_session mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -190,9 +194,9 @@ def test_run_main_auto_mode_without_tty( @patch('sys.stdout.isatty', return_value=False) def test_run_main_with_color_on( - self, mock_isatty, command, mock_watcher_class, mock_client + self, mock_isatty, command_with_mock_session, mock_watcher_class ): - command._session.create_client.return_value = mock_client + command = command_with_mock_session mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -219,12 +223,11 @@ def test_run_main_with_color_on( def test_run_main_creates_ecs_client( self, mock_isatty, - command, mock_session, + command_with_mock_session, mock_watcher_class, - mock_client, ): - command._session.create_client.return_value = mock_client + command = command_with_mock_session mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher @@ -253,13 +256,13 @@ def test_run_main_creates_ecs_client( # Verify client was passed to watcher args = mock_watcher_class.call_args[0] - assert args[0] == mock_client + assert args[0] is not None # Client was created and passed @patch('sys.stdout.isatty', return_value=False) def test_run_main_with_default_resource_view( - self, mock_isatty, command, mock_watcher_class, mock_client + self, mock_isatty, command_with_mock_session, mock_watcher_class ): - command._session.create_client.return_value = mock_client + command = command_with_mock_session mock_watcher = Mock() mock_watcher_class.return_value = mock_watcher From 802aa0b4c8479ab6ae4a39fd4bb9fd4a14cec255 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Wed, 17 Dec 2025 14:36:22 -0500 Subject: [PATCH 14/17] Fix Windows unit tests --- .../customizations/ecs/test_monitorexpressgatewayservice.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py index dcfac0c0acd0..f6900905ad0e 100644 --- a/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py @@ -177,6 +177,7 @@ def test_init_creates_collector_with_correct_parameters(self): def test_init_uses_injected_collector(self): """Test watcher uses injected collector instead of creating one""" mock_collector = Mock() + mock_display_strategy = Mock() watcher = ECSExpressGatewayServiceWatcher( Mock(), @@ -184,6 +185,7 @@ def test_init_uses_injected_collector(self): "RESOURCE", "INTERACTIVE", collector=mock_collector, + display_strategy=mock_display_strategy, ) assert watcher.collector == mock_collector From c5c7cb8e5b25e9aa5e2170394ab66af161014146 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Tue, 9 Dec 2025 20:54:03 -0500 Subject: [PATCH 15/17] Add monitor-mode to monitor mutating commands --- .../next-release/enhancement-ecs-81617.json | 5 + .../ecs/monitormutatinggatewayservice.py | 126 ++++-- .../ecs/test_monitormutatinggatewayservice.py | 380 +++++++++++++++++- .../ecs/test_monitormutatinggatewayservice.py | 142 ++++--- 4 files changed, 559 insertions(+), 94 deletions(-) create mode 100644 .changes/next-release/enhancement-ecs-81617.json diff --git a/.changes/next-release/enhancement-ecs-81617.json b/.changes/next-release/enhancement-ecs-81617.json new file mode 100644 index 000000000000..e22a9bb1e872 --- /dev/null +++ b/.changes/next-release/enhancement-ecs-81617.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "``ecs``", + "description": "Introduces a text-only mode to the existing ECS Express Mode service commands. Text-only mode can be enabled via ``ecs monitor-express-mode-service --mode TEXT-ONLY``, ``ecs create-express-mode-service --monitor-mode TEXT-ONLY``, ``ecs update-express-mode-service --monitor-mode TEXT-ONLY``, or ``ecs delete-express-mode-service --monitor-mode TEXT-ONLY``." +} diff --git a/awscli/customizations/ecs/monitormutatinggatewayservice.py b/awscli/customizations/ecs/monitormutatinggatewayservice.py index 682eb53e6678..72b71590f625 100644 --- a/awscli/customizations/ecs/monitormutatinggatewayservice.py +++ b/awscli/customizations/ecs/monitormutatinggatewayservice.py @@ -64,7 +64,7 @@ def __call__(self, parser, namespace, values, option_string=None): class MonitoringResourcesArgument(CustomArgument): - """Custom CLI argument for enabling resource monitoring. + """Custom CLI argument for enabling resource monitoring with optional mode. Adds the --monitor-resources flag to gateway service commands, allowing users to opt into real-time monitoring of resource changes. @@ -74,14 +74,13 @@ def __init__(self, name): super().__init__( name, help_text=( - 'Enable live monitoring of service resource status. ' + 'Enable monitoring of service resource status. ' 'Specify ``DEPLOYMENT`` to show only resources that are being added or removed ' 'as part of the latest service deployment, or ``RESOURCE`` to show all resources ' 'from all active configurations of the service. ' 'Defaults based on operation type: create-express-gateway-service and ' 'update-express-gateway-service default to ``DEPLOYMENT`` mode. ' - 'delete-express-gateway-service defaults to ``RESOURCE`` mode. ' - 'Requires a terminal (TTY) to run.' + 'delete-express-gateway-service defaults to ``RESOURCE`` mode.' ), choices=['DEPLOYMENT', 'RESOURCE'], nargs='?', @@ -90,6 +89,23 @@ def __init__(self, name): ) +class MonitoringModeArgument(CustomArgument): + """Custom CLI argument for monitor display mode. Only used when --monitor-resources is specified.""" + + def __init__(self, name): + super().__init__( + name, + help_text=( + 'Display mode for monitoring output (requires --monitor-resources). ' + 'INTERACTIVE (default if TTY available) - Real-time display with spinner and keyboard navigation. ' + 'TEXT-ONLY - Text output with timestamps, suitable for logging and non-interactive environments.' + ), + choices=['INTERACTIVE', 'TEXT-ONLY'], + nargs='?', + dest='monitor_mode', + ) + + class MonitorMutatingGatewayService: """Event handler for monitoring gateway service mutations. @@ -110,6 +126,7 @@ def __init__(self, api, default_resource_view, watcher_class=None): self.session = None self.parsed_globals = None self.effective_resource_view = None + self.effective_mode = None self._watcher_class = watcher_class or ECSExpressGatewayServiceWatcher def before_building_argument_table_parser(self, session, **kwargs): @@ -131,6 +148,7 @@ def building_argument_table(self, argument_table, session, **kwargs): argument_table['monitor-resources'] = MonitoringResourcesArgument( 'monitor-resources' ) + argument_table['monitor-mode'] = MonitoringModeArgument('monitor-mode') def operation_args_parsed(self, parsed_args, parsed_globals, **kwargs): """Store monitoring flag state and globals after argument parsing. @@ -139,21 +157,71 @@ def operation_args_parsed(self, parsed_args, parsed_globals, **kwargs): parsed_args: Parsed command line arguments parsed_globals: Global CLI configuration """ + self._parse_and_validate_monitoring_args(parsed_args, parsed_globals) + + def _parse_and_validate_monitoring_args(self, parsed_args, parsed_globals): + """Parse and validate monitoring-related arguments. + + Extracts monitor_resources and monitor_mode from parsed_args, + validates their combination, and sets effective_resource_view + and effective_mode. + + Args: + parsed_args: Parsed command line arguments + parsed_globals: Global CLI configuration + + Raises: + ValueError: If monitor-mode is used without monitor-resources + """ # Store parsed_globals for later use self.parsed_globals = parsed_globals - # Get monitor_resources value and determine actual monitoring mode + # Parse monitor_resources to determine if monitoring is enabled monitor_value = getattr(parsed_args, 'monitor_resources', None) + self.effective_resource_view = self._parse_monitor_resources( + monitor_value + ) + # Parse and validate monitor_mode + mode_value = getattr(parsed_args, 'monitor_mode', None) + self.effective_mode = self._validate_and_parse_mode( + mode_value, self.effective_resource_view + ) + + def _parse_monitor_resources(self, monitor_value): + """Parse monitor_resources value to determine resource view. + + Args: + monitor_value: Value from --monitor-resources flag + + Returns: + str or None: Resource view mode (DEPLOYMENT/RESOURCE) or None + """ if monitor_value is None: - # Not specified, no monitoring - self.effective_resource_view = None + return None elif monitor_value == '__DEFAULT__': - # Flag specified without value, use default based on operation - self.effective_resource_view = self.default_resource_view + return self.default_resource_view else: - # Explicit choice provided (DEPLOYMENT or RESOURCE) - self.effective_resource_view = monitor_value + return monitor_value + + def _validate_and_parse_mode(self, mode_value, resource_view): + """Validate and parse the monitor mode value. + + Args: + mode_value: Value from --monitor-mode flag + resource_view: Effective resource view (None if not monitoring) + + Returns: + str: Display mode ('interactive' or 'text-only') + + Raises: + ValueError: If mode is specified without resource monitoring + """ + if mode_value is not None and resource_view is None: + raise ValueError( + "--monitor-mode can only be used with --monitor-resources" + ) + return mode_value if mode_value else 'INTERACTIVE' def after_call(self, parsed, context, http_response, **kwargs): """Start monitoring after successful API call if flag is enabled. @@ -171,13 +239,20 @@ def after_call(self, parsed, context, http_response, **kwargs): ).get('serviceArn'): return - # Check monitoring availability - if not self._watcher_class.is_monitoring_available(): + # Interactive mode requires TTY, text-only does not + # Default to text-only if no TTY available + if self.effective_mode == 'INTERACTIVE' and not sys.stdout.isatty(): uni_print( - "Monitoring is not available (requires TTY). Skipping monitoring.\n", - out_file=sys.stderr, + "aws: [ERROR]: Interactive mode requires a TTY (terminal). " + "Monitoring skipped. Use --monitor-mode TEXT-ONLY for non-interactive environments.", + sys.stderr, ) return + elif self.effective_mode == 'INTERACTIVE' and sys.stdout.isatty(): + pass # Interactive mode with TTY - OK + elif not sys.stdout.isatty(): + # No TTY - force text-only mode + self.effective_mode = 'TEXT-ONLY' if not self.session or not self.parsed_globals: uni_print( @@ -199,20 +274,13 @@ def after_call(self, parsed, context, http_response, **kwargs): # Clear output when monitoring is invoked parsed.clear() - try: - self._watcher_class( - ecs_client, - service_arn, - self.effective_resource_view, - use_color=self._should_use_color(self.parsed_globals), - ).exec() - except Exception as e: - uni_print( - "Encountered an error, terminating monitoring\n" - + str(e) - + "\n", - out_file=sys.stderr, - ) + self._watcher_class( + ecs_client, + service_arn, + self.effective_resource_view, + self.effective_mode, + use_color=self._should_use_color(self.parsed_globals), + ).exec() def _should_use_color(self, parsed_globals): """Determine if color output should be used based on global settings.""" diff --git a/tests/functional/ecs/test_monitormutatinggatewayservice.py b/tests/functional/ecs/test_monitormutatinggatewayservice.py index b4c34281e872..21ab543d6bdb 100644 --- a/tests/functional/ecs/test_monitormutatinggatewayservice.py +++ b/tests/functional/ecs/test_monitormutatinggatewayservice.py @@ -11,8 +11,11 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. -from unittest.mock import Mock +from unittest.mock import Mock, patch +import pytest + +from awscli.customizations.ecs import inject_commands from awscli.customizations.ecs.monitormutatinggatewayservice import ( MUTATION_HANDLERS, MonitoringResourcesArgument, @@ -21,6 +24,30 @@ ) +@pytest.fixture +def mock_watcher_class(): + """Fixture that provides a mock watcher class.""" + watcher_class = Mock() + watcher_class.is_monitoring_available.return_value = True + return watcher_class + + +@pytest.fixture +def mock_session(): + """Fixture that provides a mock session.""" + return Mock() + + +@pytest.fixture +def handler(mock_watcher_class): + """Fixture that provides a MonitorMutatingGatewayService handler.""" + return MonitorMutatingGatewayService( + 'create-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + class TestMonitoringResourcesArgument: def test_add_to_parser(self): parser = Mock() @@ -98,6 +125,7 @@ def test_operation_args_parsed_no_monitor_resources_attr(self): parsed_args = Mock() # Remove the attribute del parsed_args.monitor_resources + del parsed_args.monitor_mode parsed_globals = Mock() self.handler.operation_args_parsed(parsed_args, parsed_globals) @@ -264,3 +292,353 @@ def test_register_monitor_mutating_gateway_service(self): assert ( 'after-call.ecs.CreateExpressGatewayService' in registered_events ) + + +class TestMonitorModeParameter: + """Tests for --monitor-mode parameter functionality.""" + + def test_monitor_mode_argument_added_to_table(self, handler): + """Test that --monitor-mode is added to argument table.""" + argument_table = {} + session = Mock() + + handler.building_argument_table(argument_table, session) + + assert 'monitor-mode' in argument_table + + @patch('sys.stdout.isatty', return_value=True) + def test_operation_args_parsed_with_monitor_mode_and_resources( + self, mock_isatty, handler, mock_session + ): + """Test operation_args_parsed with both --monitor-mode and --monitor-resources.""" + handler.session = mock_session + parsed_args = Mock() + parsed_args.monitor_resources = 'RESOURCE' + parsed_args.monitor_mode = 'TEXT-ONLY' + parsed_globals = Mock() + + # Should not raise + handler.operation_args_parsed(parsed_args, parsed_globals) + + assert handler.effective_resource_view == 'RESOURCE' + assert handler.effective_mode == 'TEXT-ONLY' + + @patch('sys.stdout.isatty', return_value=True) + def test_operation_args_parsed_with_monitor_mode_without_resources_raises( + self, mock_isatty, handler, mock_session + ): + """Test operation_args_parsed with --monitor-mode but no --monitor-resources raises ValueError.""" + handler.session = mock_session + parsed_args = Mock() + parsed_args.monitor_resources = None + parsed_args.monitor_mode = 'TEXT-ONLY' + parsed_globals = Mock() + + with pytest.raises(ValueError) as exc_info: + handler.operation_args_parsed(parsed_args, parsed_globals) + + assert ( + '--monitor-mode can only be used with --monitor-resources' + in str(exc_info.value) + ) + + @patch('sys.stdout.isatty', return_value=True) + def test_operation_args_parsed_defaults_mode_to_interactive( + self, mock_isatty, handler, mock_session + ): + """Test operation_args_parsed defaults mode to INTERACTIVE when not specified.""" + handler.session = mock_session + parsed_args = Mock() + parsed_args.monitor_resources = 'DEPLOYMENT' + parsed_args.monitor_mode = None + parsed_globals = Mock() + + handler.operation_args_parsed(parsed_args, parsed_globals) + + assert handler.effective_mode == 'INTERACTIVE' + + @patch('sys.stdout.isatty', return_value=True) + def test_operation_args_parsed_without_monitor_resources( + self, mock_isatty, handler, mock_session + ): + """Test operation_args_parsed disables monitoring when --monitor-resources not provided.""" + handler.session = mock_session + parsed_args = Mock() + parsed_args.monitor_resources = None + parsed_args.monitor_mode = None + parsed_globals = Mock() + + handler.operation_args_parsed(parsed_args, parsed_globals) + + assert handler.effective_resource_view is None + + @patch('sys.stdout.isatty', return_value=True) + def test_after_call_with_interactive_mode( + self, mock_isatty, mock_session, mock_watcher_class + ): + """Test monitoring starts with interactive mode when specified.""" + handler = MonitorMutatingGatewayService( + 'create-express-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + mock_parsed_globals = Mock() + mock_parsed_globals.region = 'us-west-2' + mock_parsed_globals.endpoint_url = None + mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'auto' + + mock_ecs_client = Mock() + mock_session.create_client.return_value = mock_ecs_client + + handler.session = mock_session + handler.parsed_globals = mock_parsed_globals + handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'INTERACTIVE' + + parsed = { + 'service': { + 'serviceArn': 'arn:aws:ecs:us-west-2:123456789:service/test-service' + } + } + context = {} + http_response = Mock() + http_response.status_code = 200 + + handler.after_call(parsed, context, http_response) + + # Verify watcher was created with correct parameters + call_args = mock_watcher_class.call_args + assert call_args is not None + + # Check positional arguments + assert ( + call_args[0][1] + == 'arn:aws:ecs:us-west-2:123456789:service/test-service' + ) # service_arn + assert call_args[0][2] == 'DEPLOYMENT' # resource_view + assert call_args[0][3] == 'INTERACTIVE' + + @patch('sys.stdout.isatty', return_value=False) + def test_after_call_with_text_only_mode( + self, mock_isatty, mock_session, mock_watcher_class + ): + """Test monitoring starts with text-only mode when specified.""" + handler = MonitorMutatingGatewayService( + 'create-express-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + mock_parsed_globals = Mock() + mock_parsed_globals.region = 'us-west-2' + mock_parsed_globals.endpoint_url = None + mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'off' + + mock_ecs_client = Mock() + mock_session.create_client.return_value = mock_ecs_client + + handler.session = mock_session + handler.parsed_globals = mock_parsed_globals + handler.effective_resource_view = 'RESOURCE' + handler.effective_mode = 'TEXT-ONLY' + + parsed = { + 'service': { + 'serviceArn': 'arn:aws:ecs:us-west-2:123456789:service/test-service' + } + } + context = {} + http_response = Mock() + http_response.status_code = 200 + + handler.after_call(parsed, context, http_response) + + # Verify watcher was created with correct parameters + call_args = mock_watcher_class.call_args + assert call_args is not None + + # Check positional arguments + assert ( + call_args[0][1] + == 'arn:aws:ecs:us-west-2:123456789:service/test-service' + ) # service_arn + assert call_args[0][2] == 'RESOURCE' # resource_view + assert call_args[0][3] == 'TEXT-ONLY' + + @patch('sys.stdout.isatty', return_value=True) + def test_after_call_with_color_on( + self, mock_isatty, mock_session, mock_watcher_class + ): + """Test use_color=True when color='on'.""" + handler = MonitorMutatingGatewayService( + 'create-express-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + mock_parsed_globals = Mock() + mock_parsed_globals.region = 'us-west-2' + mock_parsed_globals.endpoint_url = None + mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'on' + + mock_ecs_client = Mock() + mock_session.create_client.return_value = mock_ecs_client + + handler.session = mock_session + handler.parsed_globals = mock_parsed_globals + handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'INTERACTIVE' + + parsed = { + 'service': { + 'serviceArn': 'arn:aws:ecs:us-west-2:123456789:service/test-service' + } + } + context = {} + http_response = Mock() + http_response.status_code = 200 + + handler.after_call(parsed, context, http_response) + + # Check keyword arguments + call_args = mock_watcher_class.call_args + assert call_args[1]['use_color'] is True + + @patch('sys.stdout.isatty', return_value=False) + def test_after_call_with_color_off( + self, mock_isatty, mock_session, mock_watcher_class + ): + """Test use_color=False when color='off'.""" + handler = MonitorMutatingGatewayService( + 'create-express-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + mock_parsed_globals = Mock() + mock_parsed_globals.region = 'us-west-2' + mock_parsed_globals.endpoint_url = None + mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'off' + + mock_ecs_client = Mock() + mock_session.create_client.return_value = mock_ecs_client + + handler.session = mock_session + handler.parsed_globals = mock_parsed_globals + handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'TEXT-ONLY' + + parsed = { + 'service': { + 'serviceArn': 'arn:aws:ecs:us-west-2:123456789:service/test-service' + } + } + context = {} + http_response = Mock() + http_response.status_code = 200 + + handler.after_call(parsed, context, http_response) + + call_args = mock_watcher_class.call_args + assert call_args[1]['use_color'] is False + + @patch('sys.stdout.isatty', return_value=True) + def test_after_call_with_color_auto_with_tty( + self, mock_isatty, mock_session, mock_watcher_class + ): + """Test use_color=True when color='auto' with TTY.""" + handler = MonitorMutatingGatewayService( + 'create-express-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + mock_parsed_globals = Mock() + mock_parsed_globals.region = 'us-west-2' + mock_parsed_globals.endpoint_url = None + mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'auto' + + mock_ecs_client = Mock() + mock_session.create_client.return_value = mock_ecs_client + + handler.session = mock_session + handler.parsed_globals = mock_parsed_globals + handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'INTERACTIVE' + + parsed = { + 'service': { + 'serviceArn': 'arn:aws:ecs:us-west-2:123456789:service/test-service' + } + } + context = {} + http_response = Mock() + http_response.status_code = 200 + + handler.after_call(parsed, context, http_response) + + call_args = mock_watcher_class.call_args + assert call_args[1]['use_color'] is True + + @patch('sys.stdout.isatty', return_value=False) + def test_after_call_with_color_auto_without_tty( + self, mock_isatty, mock_session, mock_watcher_class + ): + """Test use_color=False when color='auto' without TTY.""" + handler = MonitorMutatingGatewayService( + 'create-express-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + mock_watcher = Mock() + mock_watcher_class.return_value = mock_watcher + + mock_parsed_globals = Mock() + mock_parsed_globals.region = 'us-west-2' + mock_parsed_globals.endpoint_url = None + mock_parsed_globals.verify_ssl = True + mock_parsed_globals.color = 'auto' + + mock_ecs_client = Mock() + mock_session.create_client.return_value = mock_ecs_client + + handler.session = mock_session + handler.parsed_globals = mock_parsed_globals + handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'TEXT-ONLY' + + parsed = { + 'service': { + 'serviceArn': 'arn:aws:ecs:us-west-2:123456789:service/test-service' + } + } + context = {} + http_response = Mock() + http_response.status_code = 200 + + handler.after_call(parsed, context, http_response) + + call_args = mock_watcher_class.call_args + assert call_args[1]['use_color'] is False diff --git a/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py b/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py index 37afe0a78324..f65c5617eeb7 100644 --- a/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py +++ b/tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py @@ -68,25 +68,34 @@ def test_call_with_explicit_value(self): assert namespace.monitor_resources == 'RESOURCE' +@pytest.fixture +def mock_watcher_class(): + """Fixture that provides a mock watcher class.""" + watcher_class = Mock() + watcher_class.is_monitoring_available.return_value = True + return watcher_class + + +@pytest.fixture +def handler(mock_watcher_class): + """Fixture that provides a MonitorMutatingGatewayService handler.""" + return MonitorMutatingGatewayService( + 'create-gateway-service', + 'DEPLOYMENT', + watcher_class=mock_watcher_class, + ) + + class TestMonitorMutatingGatewayService: """Test the event handler for monitoring gateway service mutations.""" - def setup_method(self): - self.mock_watcher_class = Mock() - self.mock_watcher_class.is_monitoring_available.return_value = True - self.handler = MonitorMutatingGatewayService( - 'create-gateway-service', - 'DEPLOYMENT', - watcher_class=self.mock_watcher_class, - ) - - def test_init(self): + def test_init(self, handler): """Test proper initialization of the handler.""" - assert self.handler.api == 'create-gateway-service' - assert self.handler.default_resource_view == 'DEPLOYMENT' - assert self.handler.api_pascal_case == 'CreateGatewayService' - assert self.handler.session is None - assert self.handler.parsed_globals is None + assert handler.api == 'create-gateway-service' + assert handler.default_resource_view == 'DEPLOYMENT' + assert handler.api_pascal_case == 'CreateGatewayService' + assert handler.session is None + assert handler.parsed_globals is None def test_pascal_case_conversion(self): """Test API name conversion to PascalCase.""" @@ -102,82 +111,83 @@ def test_pascal_case_conversion(self): handler = MonitorMutatingGatewayService(api_name, 'RESOURCE') assert handler.api_pascal_case == expected_pascal - def test_before_building_argument_table_parser(self): + def test_before_building_argument_table_parser(self, handler): """Test storing session for later use.""" session = Mock() - self.handler.before_building_argument_table_parser(session) + handler.before_building_argument_table_parser(session) - assert self.handler.session == session + assert handler.session == session - def test_building_argument_table(self): + def test_building_argument_table(self, handler): """Test adding monitoring argument to the command's argument table.""" argument_table = {} session = Mock() - self.handler.building_argument_table(argument_table, session) + handler.building_argument_table(argument_table, session) assert 'monitor-resources' in argument_table assert isinstance( argument_table['monitor-resources'], MonitoringResourcesArgument ) - def test_operation_args_parsed_with_flag(self): + def test_operation_args_parsed_with_flag(self, handler): """Test storing monitoring flag when enabled with default.""" parsed_args = Mock() parsed_args.monitor_resources = '__DEFAULT__' parsed_globals = Mock() - self.handler.operation_args_parsed(parsed_args, parsed_globals) + handler.operation_args_parsed(parsed_args, parsed_globals) - assert self.handler.effective_resource_view == 'DEPLOYMENT' + assert handler.effective_resource_view == 'DEPLOYMENT' - def test_operation_args_parsed_with_explicit_choice(self): + def test_operation_args_parsed_with_explicit_choice(self, handler): """Test storing monitoring flag with explicit choice.""" parsed_args = Mock() parsed_args.monitor_resources = 'RESOURCE' parsed_globals = Mock() - self.handler.operation_args_parsed(parsed_args, parsed_globals) + handler.operation_args_parsed(parsed_args, parsed_globals) - assert self.handler.effective_resource_view == 'RESOURCE' + assert handler.effective_resource_view == 'RESOURCE' - def test_operation_args_parsed_without_flag(self): + def test_operation_args_parsed_without_flag(self, handler): """Test storing monitoring flag when disabled.""" parsed_args = Mock() parsed_args.monitor_resources = None + parsed_args.monitor_mode = None parsed_globals = Mock() - self.handler.operation_args_parsed(parsed_args, parsed_globals) + handler.operation_args_parsed(parsed_args, parsed_globals) - assert self.handler.effective_resource_view is None + assert handler.effective_resource_view is None - def test_operation_args_parsed_missing_attribute(self): + def test_operation_args_parsed_missing_attribute(self, handler): """Test handling missing monitor_resources attribute.""" # Mock without monitor_resources attribute parsed_args = Mock(spec=[]) parsed_globals = Mock() - self.handler.operation_args_parsed(parsed_args, parsed_globals) + handler.operation_args_parsed(parsed_args, parsed_globals) - assert self.handler.effective_resource_view is None + assert handler.effective_resource_view is None - def test_after_call_monitoring_disabled(self): + def test_after_call_monitoring_disabled(self, handler): """Test that monitoring is skipped when flag is disabled.""" - self.handler.effective_resource_view = None + handler.effective_resource_view = None parsed = {} context = Mock() http_response = Mock() http_response.status_code = 200 # Should return early without doing anything - self.handler.after_call(parsed, context, http_response) + handler.after_call(parsed, context, http_response) # No assertions needed - just verify no exceptions - def test_after_call_http_error(self): + def test_after_call_http_error(self, handler): """Test that monitoring is skipped on HTTP errors.""" - self.handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_resource_view = 'DEPLOYMENT' parsed = { 'service': {'serviceArn': 'arn:aws:ecs:us-west-2:123:service/test'} } @@ -186,13 +196,13 @@ def test_after_call_http_error(self): http_response.status_code = 400 # Should return early without doing anything - self.handler.after_call(parsed, context, http_response) + handler.after_call(parsed, context, http_response) # No assertions needed - just verify no exceptions - def test_after_call_missing_service_arn(self): + def test_after_call_missing_service_arn(self, handler): """Test that monitoring is skipped when service ARN is missing.""" - self.handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_resource_view = 'DEPLOYMENT' # Missing serviceArn parsed = {'service': {}} context = Mock() @@ -200,15 +210,15 @@ def test_after_call_missing_service_arn(self): http_response.status_code = 200 # Should return early without doing anything - self.handler.after_call(parsed, context, http_response) + handler.after_call(parsed, context, http_response) # No assertions needed - just verify no exceptions - def test_after_call_missing_session(self, capsys): + def test_after_call_missing_session(self, handler, capsys): """Test handling when session is not available.""" - self.handler.effective_resource_view = 'DEPLOYMENT' - self.handler.session = None - self.handler.parsed_globals = None + handler.effective_resource_view = 'DEPLOYMENT' + handler.session = None + handler.parsed_globals = None parsed = { 'service': {'serviceArn': 'arn:aws:ecs:us-west-2:123:service/test'} @@ -217,7 +227,7 @@ def test_after_call_missing_session(self, capsys): http_response = Mock() http_response.status_code = 200 - self.handler.after_call(parsed, context, http_response) + handler.after_call(parsed, context, http_response) captured = capsys.readouterr() assert ( @@ -238,6 +248,7 @@ def test_after_call_successful_monitoring(self): ) handler.monitor_resources = '__DEFAULT__' handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'TEXT-ONLY' mock_session = Mock() mock_parsed_globals = Mock() @@ -277,17 +288,20 @@ def test_after_call_successful_monitoring(self): mock_client, service_arn, 'DEPLOYMENT', + 'TEXT-ONLY', use_color=False, ) mock_watcher.exec.assert_called_once() # Verify parsed response was cleared assert parsed == {} - def test_after_call_monitoring_not_available(self, capsys): - """Test that monitoring is skipped when not available (no TTY).""" + @patch('sys.stdout.isatty', return_value=False) + def test_after_call_interactive_mode_without_tty( + self, mock_isatty, capsys + ): + """Test that interactive mode is skipped when TTY is not available.""" # Setup handler state mock_watcher_class = Mock() - mock_watcher_class.is_monitoring_available.return_value = False handler = MonitorMutatingGatewayService( 'create-gateway-service', @@ -295,6 +309,7 @@ def test_after_call_monitoring_not_available(self, capsys): watcher_class=mock_watcher_class, ) handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'INTERACTIVE' mock_session = Mock() mock_parsed_globals = Mock() @@ -314,7 +329,6 @@ def test_after_call_monitoring_not_available(self, capsys): # Setup call parameters service_arn = 'arn:aws:ecs:us-west-2:123456789012:service/test-service' parsed = {'service': {'serviceArn': service_arn}} - original_parsed = dict(parsed) context = Mock() http_response = Mock() http_response.status_code = 200 @@ -322,17 +336,19 @@ def test_after_call_monitoring_not_available(self, capsys): # Execute handler.after_call(parsed, context, http_response) - # Verify parsed response was not cleared - assert parsed == original_parsed - - # Verify warning message was printed + # Verify error message was printed captured = capsys.readouterr() assert ( - "Monitoring is not available (requires TTY). Skipping monitoring.\n" + "aws: [ERROR]: Interactive mode requires a TTY (terminal)." in captured.err ) + assert "Use --monitor-mode TEXT-ONLY" in captured.err - def test_after_call_exception_handling(self, capsys): + # Verify watcher was not called + mock_watcher_class.assert_not_called() + + @patch('sys.stdout.isatty', return_value=True) + def test_after_call_exception_handling(self, mock_isatty, capsys): """Test exception handling in after_call method.""" # Setup handler state mock_watcher_class = Mock() @@ -346,6 +362,7 @@ def test_after_call_exception_handling(self, capsys): watcher_class=mock_watcher_class, ) handler.effective_resource_view = 'DEPLOYMENT' + handler.effective_mode = 'INTERACTIVE' mock_session = Mock() mock_parsed_globals = Mock() @@ -369,16 +386,13 @@ def test_after_call_exception_handling(self, capsys): http_response = Mock() http_response.status_code = 200 - # Execute - should not raise exception - handler.after_call(parsed, context, http_response) - - captured = capsys.readouterr() - assert "Encountered an error, terminating monitoring" in captured.err - assert "Test exception" in captured.err + # Execute - exception should propagate + with pytest.raises(Exception, match="Test exception"): + handler.after_call(parsed, context, http_response) - def test_events(self): + def test_events(self, handler): """Test that correct events are returned for CLI integration.""" - events = self.handler.events() + events = handler.events() expected_events = [ "before-building-argument-table-parser.ecs.create-gateway-service", From 63badb52bfaa5845f337247fd6b7cdd2499a1ce4 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Wed, 17 Dec 2025 17:57:49 -0500 Subject: [PATCH 16/17] Address PR comments --- .../ecs/monitormutatinggatewayservice.py | 14 +++++++------- .../ecs/test_monitormutatinggatewayservice.py | 11 ----------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/awscli/customizations/ecs/monitormutatinggatewayservice.py b/awscli/customizations/ecs/monitormutatinggatewayservice.py index 72b71590f625..2bd8b88baec7 100644 --- a/awscli/customizations/ecs/monitormutatinggatewayservice.py +++ b/awscli/customizations/ecs/monitormutatinggatewayservice.py @@ -96,8 +96,8 @@ def __init__(self, name): super().__init__( name, help_text=( - 'Display mode for monitoring output (requires --monitor-resources). ' - 'INTERACTIVE (default if TTY available) - Real-time display with spinner and keyboard navigation. ' + 'Display mode for monitoring output (requires ``--monitor-resources``). ' + 'INTERACTIVE (default if TTY available) - Real-time display with keyboard navigation. ' 'TEXT-ONLY - Text output with timestamps, suitable for logging and non-interactive environments.' ), choices=['INTERACTIVE', 'TEXT-ONLY'], @@ -182,9 +182,9 @@ def _parse_and_validate_monitoring_args(self, parsed_args, parsed_globals): monitor_value ) - # Parse and validate monitor_mode + # Validate monitor_mode mode_value = getattr(parsed_args, 'monitor_mode', None) - self.effective_mode = self._validate_and_parse_mode( + self.effective_mode = self._validate_mode( mode_value, self.effective_resource_view ) @@ -204,15 +204,15 @@ def _parse_monitor_resources(self, monitor_value): else: return monitor_value - def _validate_and_parse_mode(self, mode_value, resource_view): - """Validate and parse the monitor mode value. + def _validate_mode(self, mode_value, resource_view): + """Validate the monitor mode value. Args: mode_value: Value from --monitor-mode flag resource_view: Effective resource view (None if not monitoring) Returns: - str: Display mode ('interactive' or 'text-only') + str: Display mode ('INTERACTIVE' or 'TEXT-ONLY') Raises: ValueError: If mode is specified without resource monitoring diff --git a/tests/functional/ecs/test_monitormutatinggatewayservice.py b/tests/functional/ecs/test_monitormutatinggatewayservice.py index 21ab543d6bdb..b3e693ab0a9b 100644 --- a/tests/functional/ecs/test_monitormutatinggatewayservice.py +++ b/tests/functional/ecs/test_monitormutatinggatewayservice.py @@ -121,17 +121,6 @@ def test_operation_args_parsed_with_monitor_resources_false(self): assert not self.handler.effective_resource_view - def test_operation_args_parsed_no_monitor_resources_attr(self): - parsed_args = Mock() - # Remove the attribute - del parsed_args.monitor_resources - del parsed_args.monitor_mode - parsed_globals = Mock() - - self.handler.operation_args_parsed(parsed_args, parsed_globals) - - assert not self.handler.effective_resource_view - def test_after_call_with_monitoring_enabled(self): # Setup mock_watcher_class = Mock() From 87fcd83ec25024960b8fb059141e7a4e99c22258 Mon Sep 17 00:00:00 2001 From: Tyler Jung Date: Thu, 18 Dec 2025 10:51:48 -0500 Subject: [PATCH 17/17] Address PR comments --- .changes/next-release/enhancement-ecs-81617.json | 2 +- awscli/customizations/ecs/monitormutatinggatewayservice.py | 6 +++--- awscli/customizations/ecs/serviceviewcollector.py | 4 +++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.changes/next-release/enhancement-ecs-81617.json b/.changes/next-release/enhancement-ecs-81617.json index e22a9bb1e872..f82cf000587a 100644 --- a/.changes/next-release/enhancement-ecs-81617.json +++ b/.changes/next-release/enhancement-ecs-81617.json @@ -1,5 +1,5 @@ { "type": "enhancement", "category": "``ecs``", - "description": "Introduces a text-only mode to the existing ECS Express Mode service commands. Text-only mode can be enabled via ``ecs monitor-express-mode-service --mode TEXT-ONLY``, ``ecs create-express-mode-service --monitor-mode TEXT-ONLY``, ``ecs update-express-mode-service --monitor-mode TEXT-ONLY``, or ``ecs delete-express-mode-service --monitor-mode TEXT-ONLY``." + "description": "Introduces a text-only mode to the existing ECS Express Mode service commands. Text-only mode can be enabled via using the ``--mode TEXT-ONLY`` flag with the ``ecs monitor-express-gateway-service`` command, or via using the ``--monitor-mode TEXT-ONLY`` and ``--monitor-resources`` flags with the ``ecs create-express-gateway-service``, ``ecs update-express-gateway-service``, or ``ecs delete-express-gateway-service`` commands." } diff --git a/awscli/customizations/ecs/monitormutatinggatewayservice.py b/awscli/customizations/ecs/monitormutatinggatewayservice.py index 2bd8b88baec7..9f055ee5dd94 100644 --- a/awscli/customizations/ecs/monitormutatinggatewayservice.py +++ b/awscli/customizations/ecs/monitormutatinggatewayservice.py @@ -92,9 +92,9 @@ def __init__(self, name): class MonitoringModeArgument(CustomArgument): """Custom CLI argument for monitor display mode. Only used when --monitor-resources is specified.""" - def __init__(self, name): + def __init__(self): super().__init__( - name, + 'monitor-mode', help_text=( 'Display mode for monitoring output (requires ``--monitor-resources``). ' 'INTERACTIVE (default if TTY available) - Real-time display with keyboard navigation. ' @@ -148,7 +148,7 @@ def building_argument_table(self, argument_table, session, **kwargs): argument_table['monitor-resources'] = MonitoringResourcesArgument( 'monitor-resources' ) - argument_table['monitor-mode'] = MonitoringModeArgument('monitor-mode') + argument_table['monitor-mode'] = MonitoringModeArgument() def operation_args_parsed(self, parsed_args, parsed_globals, **kwargs): """Store monitoring flag state and globals after argument parsing. diff --git a/awscli/customizations/ecs/serviceviewcollector.py b/awscli/customizations/ecs/serviceviewcollector.py index 1dc308f7e2ab..ea799fff0dbe 100644 --- a/awscli/customizations/ecs/serviceviewcollector.py +++ b/awscli/customizations/ecs/serviceviewcollector.py @@ -248,7 +248,9 @@ def _diff_service_view(self, describe_gateway_service_response): source_sr_resources_combined = ManagedResourceGroup() updating_resources, disassociating_resources = ( - target_sr_resources.diff(source_sr_resources_combined) + target_sr_resources.compare_resource_sets( + source_sr_resources_combined + ) ) updating_resources.resource_type = "Updating" disassociating_resources.resource_type = "Disassociating"