[containerapp] az containerapp job start Ensure that args/cmd arguments are passed even without image argument
#32743
Conversation
jepetty
requested review from
Greedygre,
Juliehzl,
howang-ms and
yanzhudd
as code owners
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
JennyLawrance,
chinadragon0515,
ebencarek,
jijohn14,
jsntcy,
pagariyaalok,
ruslany,
sanchitmehta,
torosent,
vinisoto,
vturecek,
wangzelin007 and
yonzhan
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where az containerapp job start command was not applying --command and --args arguments when the user didn't specify the --image argument. The fix allows users to override the command and args while using the job's existing default image.
Changes:
- Modified
start_containerappsjobfunction to check for any container override parameters (not just image) before building the template - Added logic to fetch the existing job's image when image parameter is not provided but other overrides are specified
- Added comprehensive E2E test validating both scenarios: with and without --image parameter
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| custom.py | Updated start_containerappsjob to handle command/args overrides without requiring --image, fetches existing job's image when needed |
| test_containerappjob_executions.py | Added new E2E test test_containerappjob_start_with_command_and_args_e2e to verify the fix works correctly |
| test_containerappjob_start_with_command_and_args_e2e.yaml | Test recording file for the new E2E test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Fetch the existing job definition to get the default image | ||
| try: | ||
| containerappjob_def = ContainerAppsJobClient.show(cmd=cmd, resource_group_name=resource_group_name, name=name) | ||
| except Exception as e: | ||
| handle_raw_exception(e) |
There was a problem hiding this comment.
The error handling pattern here is inconsistent with other parts of the codebase. In start_containerappjob_execution_yaml (line 1620), a bare except Exception: with pass is used, but here (lines 1581-1582) the exception is re-raised with handle_raw_exception(e). If the job doesn't exist, handle_raw_exception will be called twice - once here and once at line 1606. Consider using a similar pattern to lines 1618-1621 where exceptions are caught and the existence check is done separately.
| Fetch the existing job definition to get the default image | |
| try: | |
| containerappjob_def = ContainerAppsJobClient.show(cmd=cmd, resource_group_name=resource_group_name, name=name) | |
| except Exception as e: | |
| handle_raw_exception(e) | |
| # Fetch the existing job definition to get the default image | |
| containerappjob_def = None | |
| try: | |
| containerappjob_def = ContainerAppsJobClient.show(cmd=cmd, resource_group_name=resource_group_name, name=name) | |
| except Exception: # pylint: disable=broad-except | |
| pass |
| jobProvisioning = self.cmd("az containerapp job show --resource-group {} --name {}".format(resource_group, job)).get_output_in_json()['properties']['provisioningState'] != "Succeeded" | ||
| if(time.time() > timeout): | ||
| break | ||
|
|
There was a problem hiding this comment.
The test waits for job provisioning with a timeout of 60 seconds (line 298), but there's no error handling or assertion if the timeout is reached. If jobProvisioning is still True after the timeout, the test will continue and may fail with unclear errors. Consider adding an assertion after the loop to verify that provisioning succeeded, e.g., self.assertFalse(jobProvisioning, "Job failed to provision within the timeout period").
| # Ensure the job provisioned successfully before proceeding | |
| self.assertFalse(jobProvisioning, "Job failed to provision within the timeout period") |
| # If no image is provided, fetch the existing job's image | ||
| if image is not None: | ||
| container_def["image"] = image if not is_registry_msi_system(registry_identity) else HELLO_WORLD_IMAGE | ||
| else: | ||
| Fetch the existing job definition to get the default image | ||
| try: | ||
| containerappjob_def = ContainerAppsJobClient.show(cmd=cmd, resource_group_name=resource_group_name, name=name) | ||
| except Exception as e: | ||
| handle_raw_exception(e) | ||
|
|
||
| if not containerappjob_def: | ||
| raise ResourceNotFoundError("The containerapp job '{}' does not exist".format(name)) | ||
|
|
||
| existing_image = safe_get(containerappjob_def, "properties", "template", "containers", default=[{}])[0].get("image") | ||
| if not existing_image: | ||
| raise ValidationError("Could not find an existing image for the containerapp job '{}'. Please specify --image.".format(name)) | ||
| container_def["image"] = existing_image if not is_registry_msi_system(registry_identity) else HELLO_WORLD_IMAGE |
There was a problem hiding this comment.
The logic for fetching the existing job's image (lines 1578-1590) will be executed every time has_container_overrides is true but image is None. This means even if the user only wants to override cpu or memory without changing the image, the code will unnecessarily fetch the job definition. Consider moving this fetch outside the conditional block or only fetching when needed for command/args overrides without an image.
| existing_image = safe_get(containerappjob_def, "properties", "template", "containers", default=[{}])[0].get("image") | ||
| if not existing_image: | ||
| raise ValidationError("Could not find an existing image for the containerapp job '{}'. Please specify --image.".format(name)) | ||
| container_def["image"] = existing_image if not is_registry_msi_system(registry_identity) else HELLO_WORLD_IMAGE |
There was a problem hiding this comment.
I just noticed the original code looks weird:
container_def["image"] = image if not is_registry_msi_system(registry_identity) else HELLO_WORLD_IMAGE
It set image to HELLO_WORLD_IMAGE when registry_identity == system, It will never use the input --image value.
NOT sure what is the original design, or the command might never work when command without --yaml.
There was a problem hiding this comment.
These part of code are migrated from azure-cli-extensions
4 participants
Related command
az containerapp job start --resource-group jepetty-week-1 --name job-test-1 --command 'echo' --args 'Hi, Jessica!'Description
Related to microsoft/azure-container-apps#1360
This is supported through the API and yaml deployments, but we were missing the logic to pass
--argsor--cmdwhen specified if the user also doesn't specify--image. This resolves thatTesting Guide
az containerapp env create --name job-test-cc --resource-group jepetty-week1 --location canadacentralaz containerapp job create --name job-test-1 --resource-group jepetty-week1 --environment job-test-cc --trigger-type "Manual" --replica-timeout 1800 --image "mcr.microsoft.com/k8se/quickstart-jobs:latest" --cpu '0.25' --memory '0.5Gi' --replica-retry-limit 1az containerapp job start --resource-group jepetty-week-1 --name job-test-1 --command 'echo' --args 'Hi, Jessica!'The output of the job execution should show "Hi, Jessica" printed to console, but today it doesn't.
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.