From 0f49ce0dc9cabf8d2c072efb5a3e23b45f9ebbda Mon Sep 17 00:00:00 2001 From: Silas Strawn Date: Thu, 21 Apr 2022 13:17:36 -0700 Subject: [PATCH 1/4] minor bug fixes --- src/containerapp/azext_containerapp/_up_utils.py | 10 ++++++---- src/containerapp/azext_containerapp/custom.py | 3 +-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 1a4e521682e..f13ba3414a6 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -223,8 +223,8 @@ def create(self, no_registry=False): managed_env=self.env.get_rid(), target_port=self.target_port, registry_server=None if no_registry else self.registry_server, - registry_pass=None if no_registry else self.registry_server, - registry_user=None if no_registry else self.registry_server, + registry_pass=None if no_registry else self.registry_pass, + registry_user=None if no_registry else self.registry_user, env_vars=self.env_vars, ingress=self.ingress, disable_warnings=True) @@ -418,8 +418,9 @@ def _get_registry_details(cmd, app: 'ContainerApp'): else: registry_rg = app.resource_group.name user = get_profile_username() - registry_name = "{}acr".format(app.name).replace('-','') - registry_name = registry_name + str(hash((registry_rg, user, app.name))).replace("-", "") + registry_name = app.name.replace('-','').lower() + registry_name = registry_name + str(hash((registry_rg, user, app.name))).replace("-", "").replace(".", "")[:10] # cap at 15 characters total + registry_name = f"ca{registry_name}acr" # ACR names must start + end in a letter app.registry_server = registry_name + ".azurecr.io" app.should_create_acr = True app.acr = AzureContainerRegistry(registry_name, ResourceGroup(cmd, registry_rg, None, None)) @@ -469,6 +470,7 @@ def _create_github_action(app:'ContainerApp', image=app.image, context_path=context_path) + def up_output(app): url = safe_get(ContainerAppClient.show(app.cmd, app.resource_group.name, app.name), "properties", "configuration", diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index 96ebf46951d..082e1b2a0c8 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -1130,7 +1130,7 @@ def create_or_update_github_action(cmd, registry_name = (parsed.netloc if parsed.scheme else parsed.path).split('.')[0] try: - registry_username, registry_password = _get_acr_cred(cmd.cli_ctx, registry_name) + registry_username, registry_password, _ = _get_acr_cred(cmd.cli_ctx, registry_name) except Exception as ex: raise RequiredArgumentMissingError('Failed to retrieve credentials for container registry. Please provide the registry username and password') from ex @@ -1725,7 +1725,6 @@ def set_secrets(cmd, name, resource_group_name, secrets, # yaml=None, no_wait=False): _validate_subscription_registered(cmd, "Microsoft.App") - # if not yaml and not secrets: # raise RequiredArgumentMissingError('Usage error: --secrets is required if not using --yaml') From b3aae19ace335ee9df9ed8e4f4bb8348096b36cd Mon Sep 17 00:00:00 2001 From: Silas Strawn Date: Thu, 21 Apr 2022 14:32:14 -0700 Subject: [PATCH 2/4] fixes for sisira's comments --- src/containerapp/HISTORY.rst | 2 +- src/containerapp/azext_containerapp/_clients.py | 6 +++--- src/containerapp/azext_containerapp/_help.py | 10 +++++----- src/containerapp/azext_containerapp/_params.py | 8 +++----- src/containerapp/azext_containerapp/_utils.py | 6 +++--- src/containerapp/setup.py | 2 +- 6 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/containerapp/HISTORY.rst b/src/containerapp/HISTORY.rst index 3a2c9605bd1..0e0e05ee905 100644 --- a/src/containerapp/HISTORY.rst +++ b/src/containerapp/HISTORY.rst @@ -3,7 +3,7 @@ Release History =============== -0.4.0 +0.3.2 ++++++ * Create or update a container app and all associated resources (container app environment, ACR, Github Actions, resource group, etc.) with 'az containerapp up' * Open an ssh-like shell in a Container App with 'az containerapp exec' diff --git a/src/containerapp/azext_containerapp/_clients.py b/src/containerapp/azext_containerapp/_clients.py index 7986ca00df8..0034b61bd85 100644 --- a/src/containerapp/azext_containerapp/_clients.py +++ b/src/containerapp/azext_containerapp/_clients.py @@ -367,7 +367,7 @@ def list_replicas(cls, cmd, resource_group_name, container_app_name, revision_na resource_group_name, container_app_name, revision_name, - PREVIEW_API_VERSION) + STABLE_API_VERSION) r = send_raw_request(cmd.cli_ctx, "GET", request_url) j = r.json() @@ -395,7 +395,7 @@ def get_replica(cls, cmd, resource_group_name, container_app_name, revision_name container_app_name, revision_name, replica_name, - PREVIEW_API_VERSION) + STABLE_API_VERSION) r = send_raw_request(cmd.cli_ctx, "GET", request_url) return r.json() @@ -410,7 +410,7 @@ def get_auth_token(cls, cmd, resource_group_name, name): sub_id, resource_group_name, name, - PREVIEW_API_VERSION) + STABLE_API_VERSION) r = send_raw_request(cmd.cli_ctx, "POST", request_url) return r.json() diff --git a/src/containerapp/azext_containerapp/_help.py b/src/containerapp/azext_containerapp/_help.py index d3b0bef5cb6..6ee0f6274c8 100644 --- a/src/containerapp/azext_containerapp/_help.py +++ b/src/containerapp/azext_containerapp/_help.py @@ -88,7 +88,7 @@ helps['containerapp exec'] = """ type: command - short-summary: Open an SSH-like interactive shell within a container app replica (pod) + short-summary: Open an SSH-like interactive shell within a container app replica examples: - name: exec into a container app text: | @@ -132,7 +132,7 @@ helps['containerapp logs show'] = """ type: command - short-summary: Show past logs and/or print logs in real time (with the --follow parameter). Note that the logs are only taken from one revision, replica (pod), and container. + short-summary: Show past logs and/or print logs in real time (with the --follow parameter). Note that the logs are only taken from one revision, replica, and container. examples: - name: Fetch the past 20 lines of logs from an app and return text: | @@ -148,12 +148,12 @@ # Replica Commands helps['containerapp replica'] = """ type: group - short-summary: Manage container app replicas (pods) + short-summary: Manage container app replicas """ helps['containerapp replica list'] = """ type: command - short-summary: List a container app revision's replicas (pods) + short-summary: List a container app revision's replica examples: - name: List a container app's replicas in the latest revision text: | @@ -165,7 +165,7 @@ helps['containerapp replica show'] = """ type: command - short-summary: Show a container app replica (pod) + short-summary: Show a container app replica examples: - name: Show a replica from the latest revision text: | diff --git a/src/containerapp/azext_containerapp/_params.py b/src/containerapp/azext_containerapp/_params.py index 2755a7bf18c..01c6a4afa05 100644 --- a/src/containerapp/azext_containerapp/_params.py +++ b/src/containerapp/azext_containerapp/_params.py @@ -33,7 +33,7 @@ def load_arguments(self, _): with self.argument_context('containerapp exec') as c: c.argument('container', help="The name of the container to ssh into") - c.argument('replica', help="The name of the replica (pod) to ssh into. List replicas with 'az containerapp replica list'. A replica may not exist if there is not traffic to your app.") + c.argument('replica', help="The name of the replica to ssh into. List replicas with 'az containerapp replica list'. A replica may not exist if there is not traffic to your app.") c.argument('revision', help="The name of the container app revision to ssh into. Defaults to the latest revision.") c.argument('startup_command', options_list=["--command"], help="The startup command (bash, zsh, sh, etc.).") c.argument('name', name_type, id_part=None, help="The name of the Containerapp.") @@ -44,21 +44,20 @@ def load_arguments(self, _): c.argument('tail', help="The number of past logs to print (0-300)", type=int, default=20) c.argument('container', help="The name of the container") c.argument('output_format', options_list=["--format"], help="Log output format", arg_type=get_enum_type(["json", "text"]), default="json") - c.argument('replica', help="The name of the replica (pod). List replicas with 'az containerapp replica list'. A replica may not exist if there is not traffic to your app.") + c.argument('replica', help="The name of the replica. List replicas with 'az containerapp replica list'. A replica may not exist if there is not traffic to your app.") c.argument('revision', help="The name of the container app revision. Defaults to the latest revision.") c.argument('name', name_type, id_part=None, help="The name of the Containerapp.") c.argument('resource_group_name', arg_type=resource_group_name_type, id_part=None) # Replica with self.argument_context('containerapp replica') as c: - c.argument('replica', help="The name of the replica (pod). ") + c.argument('replica', help="The name of the replica. ") c.argument('revision', help="The name of the container app revision. Defaults to the latest revision.") c.argument('name', name_type, id_part=None, help="The name of the Containerapp.") c.argument('resource_group_name', arg_type=resource_group_name_type, id_part=None) # Container with self.argument_context('containerapp', arg_group='Container') as c: - # c.argument('image', type=str, options_list=['--image', '-i'], help="Container image, e.g. publisher/image-name:tag.") c.argument('container_name', type=str, help="Name of the container.") c.argument('cpu', type=float, validator=validate_cpu, help="Required CPU in cores from 0.25 - 2.0, e.g. 0.5") c.argument('memory', type=str, validator=validate_memory, help="Required memory from 0.5 - 4.0 ending with \"Gi\", e.g. 1.0Gi") @@ -225,7 +224,6 @@ def load_arguments(self, _): c.argument('name', configured_default='name', id_part=None) c.argument('managed_env', configured_default='managed_env') c.argument('registry_server', configured_default='registry_server') - c.argument('dryrun', help="Show summary of the operation instead of executing it.") c.argument('source', type=str, help='Local directory path to upload to Azure container registry.') c.argument('image', type=str, options_list=['--image', '-i'], help="Container image, e.g. publisher/image-name:tag.") c.argument('browse', help='Open the app in a web browser after creation and deployment, if possible.') diff --git a/src/containerapp/azext_containerapp/_utils.py b/src/containerapp/azext_containerapp/_utils.py index e72b829acde..d7091624cb9 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -156,7 +156,7 @@ def is_int(s): return False -def await_github_action(cmd, token, repo, branch, name, resource_group_name, timeout=300): +def await_github_action(cmd, token, repo, branch, name, resource_group_name, timeout_secs=300): from .custom import show_github_action from github import Github from time import sleep @@ -187,7 +187,7 @@ def await_github_action(cmd, token, repo, branch, name, resource_group_name, tim sleep(1) animation.tick() - if (datetime.utcnow() - start).seconds >= timeout: + if (datetime.utcnow() - start).seconds >= timeout_secs: raise CLIInternalError("Timed out while waiting for the Github action to start.") animation.flush() @@ -202,7 +202,7 @@ def await_github_action(cmd, token, repo, branch, name, resource_group_name, tim animation.tick() status = [wf.status for wf in workflow.get_runs() if wf.id == run_id][0] animation.flush() - if (datetime.utcnow() - start).seconds >= timeout: + if (datetime.utcnow() - start).seconds >= timeout_secs: raise CLIInternalError("Timed out while waiting for the Github action to start.") if status != "completed": diff --git a/src/containerapp/setup.py b/src/containerapp/setup.py index 3276459f468..d0f615849f3 100644 --- a/src/containerapp/setup.py +++ b/src/containerapp/setup.py @@ -17,7 +17,7 @@ # TODO: Confirm this is the right version number you want and it matches your # HISTORY.rst entry. -VERSION = '0.4.0' +VERSION = '0.3.2' # The full list of classifiers is available at From 78b1c50e25ad8a37b84f66e8a660ef939e65b53a Mon Sep 17 00:00:00 2001 From: Silas Strawn Date: Thu, 21 Apr 2022 15:16:53 -0700 Subject: [PATCH 3/4] fix minor typo --- src/containerapp/azext_containerapp/_up_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index f13ba3414a6..084399ed77a 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -481,9 +481,9 @@ def up_output(app): output = (f"\nYour container app ({app.name}) has been created a deployed! Congrats! \n\n" f"Browse to your container app at: {url} \n\n" f"Stream logs for your container with: az containerapp logs -n {app.name} -g {app.resource_group.name} \n\n" - f"See full output using: az containerapp show n {app.name} -g {app.resource_group.name} \n") + f"See full output using: az containerapp show -n {app.name} -g {app.resource_group.name} \n") else: output = (f"\nYour container app ({app.name}) has been created a deployed! Congrats! \n\n" f"Stream logs for your container with: az containerapp logs -n {app.name} -g {app.resource_group.name} \n\n" - f"See full output using: az containerapp show n {app.name} -g {app.resource_group.name} \n") + f"See full output using: az containerapp show -n {app.name} -g {app.resource_group.name} \n") return output From 0ba71853bd89b31d35dd90d8dbc88f8da47d73bc Mon Sep 17 00:00:00 2001 From: Silas Strawn Date: Thu, 21 Apr 2022 18:47:50 -0700 Subject: [PATCH 4/4] bug fix where commands fail if providing registry creds --- src/containerapp/azext_containerapp/.flake8 | 4 +++ .../azext_containerapp/_up_utils.py | 35 +++++++++++++------ src/containerapp/azext_containerapp/custom.py | 4 ++- 3 files changed, 32 insertions(+), 11 deletions(-) create mode 100644 src/containerapp/azext_containerapp/.flake8 diff --git a/src/containerapp/azext_containerapp/.flake8 b/src/containerapp/azext_containerapp/.flake8 new file mode 100644 index 00000000000..777d0ca9ecd --- /dev/null +++ b/src/containerapp/azext_containerapp/.flake8 @@ -0,0 +1,4 @@ +[flake8] +ignore = + W503 # line break before binary operator, not compliant with PEP 8 + E203 # whitespace before ':', not compliant with PEP 8 \ No newline at end of file diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 084399ed77a..44f6665158c 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -15,6 +15,9 @@ MutuallyExclusiveArgumentError) from azure.cli.core.commands.client_factory import get_subscription_id from azure.cli.command_modules.appservice._create_util import check_resource_group_exists +from azure.cli.command_modules.acr.custom import acr_show +from azure.cli.core.commands.client_factory import get_mgmt_service_client +from azure.mgmt.containerregistry import ContainerRegistryManagementClient from knack.log import get_logger from msrestazure.tools import parse_resource_id, is_valid_resource_id, resource_id @@ -382,16 +385,19 @@ def _get_env_and_group_from_log_analytics(cmd, resource_group_name, env:'Contain def _get_acr_from_image(cmd, app): if app.image is not None and "azurecr.io" in app.image: + app.registry_server = app.image.split('/')[0] # TODO what if this conflicts with registry_server param? + parsed = urlparse(app.image) + registry_name = (parsed.netloc if parsed.scheme else parsed.path).split('.')[0] if app.registry_user is None or app.registry_pass is None: logger.info('No credential was provided to access Azure Container Registry. Trying to look up...') - app.registry_server = app.image.split('/')[0] # TODO what if this conflicts with registry_server param? - parsed = urlparse(app.image) - registry_name = (parsed.netloc if parsed.scheme else parsed.path).split('.')[0] - try: - app.registry_user, app.registry_pass, registry_rg = _get_acr_cred(cmd.cli_ctx, registry_name) - app.acr = AzureContainerRegistry(registry_name, ResourceGroup(cmd, registry_rg, None, None)) - except Exception as ex: - raise RequiredArgumentMissingError('Failed to retrieve credentials for container registry. Please provide the registry username and password') from ex + try: + app.registry_user, app.registry_pass, registry_rg = _get_acr_cred(cmd.cli_ctx, registry_name) + app.acr = AzureContainerRegistry(registry_name, ResourceGroup(cmd, registry_rg, None, None)) + except Exception as ex: + raise RequiredArgumentMissingError('Failed to retrieve credentials for container registry. Please provide the registry username and password') from ex + else: + acr_rg = _get_acr_rg(app) + app.acr = AzureContainerRegistry(name=registry_name, resource_group=ResourceGroup(app.cmd, acr_rg, None, None)) def _get_registry_from_app(app): @@ -401,20 +407,27 @@ def _get_registry_from_app(app): app.registry_server = containerapp_def["properties"]["configuration"]["registries"][0]["server"] +def _get_acr_rg(app): + registry_name = app.registry_server[:app.registry_server.rindex(".azurecr.io")] + client = get_mgmt_service_client(app.cmd.cli_ctx, ContainerRegistryManagementClient).registries + return parse_resource_id(acr_show(app.cmd, client, registry_name).id)["resource_group"] + def _get_registry_details(cmd, app: 'ContainerApp'): registry_rg = None registry_name = None if app.registry_server: if "azurecr.io" not in app.registry_server: raise ValidationError("Cannot supply non-Azure registry when using --source.") + parsed = urlparse(app.registry_server) + registry_name = (parsed.netloc if parsed.scheme else parsed.path).split('.')[0] if app.registry_user is None or app.registry_pass is None: logger.info('No credential was provided to access Azure Container Registry. Trying to look up...') - parsed = urlparse(app.registry_server) - registry_name = (parsed.netloc if parsed.scheme else parsed.path).split('.')[0] try: app.registry_user, app.registry_pass, registry_rg = _get_acr_cred(cmd.cli_ctx, registry_name) except Exception as ex: raise RequiredArgumentMissingError('Failed to retrieve credentials for container registry. Please provide the registry username and password') from ex + else: + registry_rg = _get_acr_rg(app) else: registry_rg = app.resource_group.name user = get_profile_username() @@ -423,6 +436,7 @@ def _get_registry_details(cmd, app: 'ContainerApp'): registry_name = f"ca{registry_name}acr" # ACR names must start + end in a letter app.registry_server = registry_name + ".azurecr.io" app.should_create_acr = True + app.acr = AzureContainerRegistry(registry_name, ResourceGroup(cmd, registry_rg, None, None)) @@ -438,6 +452,7 @@ def _set_up_defaults(cmd, name, resource_group_name, logs_customer_id, location, # get ACR details from --image, if possible _get_acr_from_image(cmd, app) + def _create_github_action(app:'ContainerApp', env:'ContainerAppEnvironment', service_principal_client_id, service_principal_client_secret, service_principal_tenant_id, diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index 082e1b2a0c8..a3515ca44df 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -2007,7 +2007,7 @@ def containerapp_up(cmd, service_principal_tenant_id=None): from ._up_utils import (_validate_up_args, _reformat_image, _get_dockerfile_content, _get_ingress_and_target_port, ResourceGroup, ContainerAppEnvironment, ContainerApp, _get_registry_from_app, - _get_registry_details, _create_github_action, _set_up_defaults, up_output) + _get_registry_details, _create_github_action, _set_up_defaults, up_output, AzureContainerRegistry) dockerfile="Dockerfile" # for now the dockerfile name must be "Dockerfile" (until GH actions API is updated) @@ -2033,6 +2033,8 @@ def containerapp_up(cmd, env.create_if_needed(name) app.create_acr_if_needed() + + if source: app.run_acr_build(dockerfile, source)