[containerapp] az containerapp job start Ensure that args/cmd arguments are passed even without image argument#32743
[containerapp] az containerapp job start Ensure that args/cmd arguments are passed even without image argument#32743
az containerapp job start Ensure that args/cmd arguments are passed even without image argument#32743Conversation
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.
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 |
...e-cli/azure/cli/command_modules/containerapp/tests/latest/test_containerappjob_executions.py
Show resolved
Hide resolved
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.
There was a problem hiding this comment.
You're right, if the image isn't specified, there's no reason to fetch it - the RP should handle this case
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
There was a problem hiding this comment.
@Greedygre yeah this code really confused me. Looking more closely I'm seeing that this behavior is a bit different than container apps (although I think functionally it's the same). The default for the registry-identity is None, so unless the user explicitly passes --registry-identity system it should function as expected, using the specified ACR image. (I did test this with a few different configurations and found this to be true)
What are your thoughts? It seems reasonable to me to remove this logic about setting the default image to HelloWorld in the case that --registry-identity system` is passed (and to more closely align with container apps create). Thoughts? If you agree, I can perform this change in a subsequent PR (I think this would technically be breaking?)
jepetty
force-pushed
the
fixArgCmdPassingForJobs
branch
from
bee6b4d to
26baeb4
Compare
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.