From c100f1fce5f2d2736ab959137cc59d1df4c289ae Mon Sep 17 00:00:00 2001 From: Haroon Feisal Date: Tue, 26 Apr 2022 14:48:15 -0400 Subject: [PATCH 1/4] Search for acr before creating one. --- .../azext_containerapp/_up_utils.py | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 72d3589c718..fdebbed338e 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -274,12 +274,22 @@ def create(self, no_registry=False): ) def create_acr_if_needed(self): + self.acr.name, self.acr.resource_group.name, acr_found = find_existing_acr(self.cmd, self.env.resource_group.name, self.env.name, self.acr.name, self.acr.resource_group.name, self) if self.should_create_acr: logger.warning( f"Creating Azure Container Registry {self.acr.name} in resource group " f"{self.acr.resource_group.name}" ) self.create_acr() + if acr_found: + self.registry_user, self.registry_pass, _ = _get_acr_cred( + self.cmd.cli_ctx, self.acr.name + ) + self.registry_server = self.acr.name + ".azurecr.io" + logger.warning( + f"Using Azure Container Registry {self.acr.name} in resource group " + f"{self.acr.resource_group.name}" + ) def create_acr(self): registry_rg = self.resource_group @@ -606,7 +616,7 @@ def _get_registry_details(cmd, app: "ContainerApp"): else: registry_rg = app.resource_group.name user = get_profile_username() - registry_name = app.name.replace("-", "").lower() + registry_name = app.env.name.replace("-", "").lower() registry_name = ( registry_name + str(hash((registry_rg, user, app.name))) @@ -713,3 +723,22 @@ def up_output(app): logger.warning( f"See full output using: az containerapp show -n {app.name} -g {app.resource_group.name} \n" ) + + +def find_existing_acr(cmd, resource_group_name, env_name, default_name, default_rg, app: "ContainerApp"): + from azure.cli.command_modules.acr.custom import acr_list as list_acr + from azure.cli.command_modules.acr._client_factory import cf_acr_registries + client = cf_acr_registries(cmd.cli_ctx) + acr_list = list_acr(client, resource_group_name) + acr_list = list(acr_list) + acr = None + acr_list = [a for a in acr_list if env_name.lower().replace('-','') in a.name.lower()] + + if len(acr_list) > 0: + acr = acr_list[0] + + if acr: + app.should_create_acr = False + return acr.name, parse_resource_id(acr.id)["resource_group"], True + else: + return default_name, default_rg, False \ No newline at end of file From 44faaf753b31c375010c78f4064404a77940d114 Mon Sep 17 00:00:00 2001 From: Haroon Feisal Date: Tue, 26 Apr 2022 15:13:46 -0400 Subject: [PATCH 2/4] Fixed bug where only --environment is passed. Changed hash on acr name to make it more unique. Tiny change in find_existing_acr. --- .../azext_containerapp/_up_utils.py | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index fdebbed338e..97dba3a96ad 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -64,8 +64,6 @@ def __init__(self, cmd, name: str, location: str, exists: bool = None): self.check_exists() def create(self): - if not self.name: - self.name = get_randomized_name(get_profile_username()) g = create_resource_group(self.cmd, self.name, self.location) self.exists = True return g @@ -90,6 +88,8 @@ def check_exists(self) -> bool: def create_if_needed(self): if not self.check_exists(): + if not self.name: + self.name = get_randomized_name(get_profile_username()) logger.warning(f"Creating resoure group '{self.name}'") self.create() else: @@ -155,9 +155,10 @@ def __init__( super().__init__(cmd, name, resource_group, exists) if is_valid_resource_id(name): self.name = parse_resource_id(name)["name"] - rg = parse_resource_id(name)["resource_group"] - if resource_group.name != rg: - self.resource_group = ResourceGroup(cmd, rg, location) + if "resource_group" in parse_resource_id(name): + rg = parse_resource_id(name)["resource_group"] + if resource_group.name != rg: + self.resource_group = ResourceGroup(cmd, rg, location) self.location = _get_default_containerapps_location(cmd, location) self.logs_key = logs_key self.logs_customer_id = logs_customer_id @@ -165,13 +166,14 @@ def __init__( def set_name(self, name_or_rid): if is_valid_resource_id(name_or_rid): self.name = parse_resource_id(name_or_rid)["name"] - rg = parse_resource_id(name_or_rid)["resource_group"] - if self.resource_group.name != rg: - self.resource_group = ResourceGroup( - self.cmd, - rg, - _get_default_containerapps_location(self.cmd, self.location), - ) + if "resource_group" in parse_resource_id(name_or_rid): + rg = parse_resource_id(name_or_rid)["resource_group"] + if self.resource_group.name != rg: + self.resource_group = ResourceGroup( + self.cmd, + rg, + _get_default_containerapps_location(self.cmd, self.location), + ) else: self.name = name_or_rid @@ -619,7 +621,7 @@ def _get_registry_details(cmd, app: "ContainerApp"): registry_name = app.env.name.replace("-", "").lower() registry_name = ( registry_name - + str(hash((registry_rg, user, app.name))) + + str(hash((app.env.resource_group.name, app.env.name))) .replace("-", "") .replace(".", "")[:10] ) # cap at 15 characters total @@ -734,7 +736,7 @@ def find_existing_acr(cmd, resource_group_name, env_name, default_name, default_ acr = None acr_list = [a for a in acr_list if env_name.lower().replace('-','') in a.name.lower()] - if len(acr_list) > 0: + if acr_list: acr = acr_list[0] if acr: From 595cdf5d7c74da365d15bf61cbadc004355944f7 Mon Sep 17 00:00:00 2001 From: Haroon Feisal Date: Tue, 26 Apr 2022 16:38:30 -0400 Subject: [PATCH 3/4] Fixed bug with --image. Changed logger warning output. Disabled warnings on the registry update code for containerapp up. Added HELLOWORLD constant. --- .../azext_containerapp/_up_utils.py | 35 ++++++++++--------- src/containerapp/azext_containerapp/custom.py | 13 ++++--- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 97dba3a96ad..4ee5a2515c2 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -276,22 +276,23 @@ def create(self, no_registry=False): ) def create_acr_if_needed(self): - self.acr.name, self.acr.resource_group.name, acr_found = find_existing_acr(self.cmd, self.env.resource_group.name, self.env.name, self.acr.name, self.acr.resource_group.name, self) - if self.should_create_acr: - logger.warning( - f"Creating Azure Container Registry {self.acr.name} in resource group " - f"{self.acr.resource_group.name}" - ) - self.create_acr() - if acr_found: - self.registry_user, self.registry_pass, _ = _get_acr_cred( - self.cmd.cli_ctx, self.acr.name - ) - self.registry_server = self.acr.name + ".azurecr.io" - logger.warning( - f"Using Azure Container Registry {self.acr.name} in resource group " - f"{self.acr.resource_group.name}" - ) + if self.acr: + self.acr.name, self.acr.resource_group.name, acr_found = find_existing_acr(self.cmd, self.env.resource_group.name, self.env.name, self.acr.name, self.acr.resource_group.name, self) + if self.should_create_acr: + logger.warning( + f"Creating Azure Container Registry {self.acr.name} in resource group " + f"{self.acr.resource_group.name}" + ) + self.create_acr() + if acr_found: + self.registry_user, self.registry_pass, _ = _get_acr_cred( + self.cmd.cli_ctx, self.acr.name + ) + self.registry_server = self.acr.name + ".azurecr.io" + logger.warning( + f"Using Azure Container Registry {self.acr.name} in resource group " + f"{self.acr.resource_group.name}" + ) def create_acr(self): registry_rg = self.resource_group @@ -716,7 +717,7 @@ def up_output(app): url = f"http://{url}" logger.warning( - f"\nYour container app ({app.name}) has been created and deployed! Congrats! \n" + f"\nYour container app {app.name} has been created and deployed! Congrats! \n" ) url and logger.warning(f"Browse to your container app at: {url} \n") logger.warning( diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index ac8f89288a7..6da0e5ea34b 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -2004,7 +2004,7 @@ def containerapp_up(cmd, 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, AzureContainerRegistry) - + HELLOWORLD = "mcr.microsoft.com/azuredocs/containerapps-helloworld" dockerfile = "Dockerfile" # for now the dockerfile name must be "Dockerfile" (until GH actions API is updated) _validate_up_args(source, image, repo) @@ -2012,14 +2012,14 @@ def containerapp_up(cmd, image = _reformat_image(source, repo, image) token = None if not repo else get_github_access_token(cmd, ["admin:repo_hook", "repo", "workflow"], token) - if image and "mcr.microsoft.com/azuredocs/containerapps-helloworld" in image.lower(): + if image and HELLOWORLD in image.lower(): ingress = "external" if not ingress else ingress target_port = 80 if not target_port else target_port if image: if ingress and not target_port: target_port = 80 - logger.warning("No ingress provided, defaulting to port 80.") + logger.warning("No ingress provided, defaulting to port 80. Try specifying --target-port or run `az containerapp ingress enable` after your containerapp has been created.") dockerfile_content = _get_dockerfile_content(repo, branch, token, source, context_path, dockerfile) ingress, target_port = _get_ingress_and_target_port(ingress, target_port, dockerfile_content) @@ -2072,6 +2072,7 @@ def containerapp_up_logic(cmd, resource_group_name, name, managed_env, image, en if containerapp_def: ca_exists = True + # When using repo, image is not passed, so we have to assign it a value (will be overwritten with gh-action) if image is None: image = "mcr.microsoft.com/azuredocs/containerapps-helloworld:latest" @@ -2153,7 +2154,8 @@ def containerapp_up_logic(cmd, resource_group_name, name, managed_env, image, en r["username"], r["server"], registry_pass, - update_existing_secret=True) + update_existing_secret=True, + disable_warnings=True) # If not updating existing registry, add as new registry if not updating_existing_registry: @@ -2165,7 +2167,8 @@ def containerapp_up_logic(cmd, resource_group_name, name, managed_env, image, en registry_user, registry_server, registry_pass, - update_existing_secret=True) + update_existing_secret=True, + disable_warnings=True) registries_def.append(registry) From 272146b123bb40cb09ca384f84f34fb8a0c0b43e Mon Sep 17 00:00:00 2001 From: Haroon Feisal Date: Tue, 26 Apr 2022 17:46:18 -0400 Subject: [PATCH 4/4] Disabled no_wait. Added better error handling for up API calls. Updated ingress infer warning text. Fixed typo. Moved create_if_needed to environment. --- .../azext_containerapp/_up_utils.py | 40 +++++++++---------- .../azext_containerapp/commands.py | 2 +- src/containerapp/azext_containerapp/custom.py | 16 +++++--- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 4ee5a2515c2..8552ef07fa7 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -90,10 +90,10 @@ def create_if_needed(self): if not self.check_exists(): if not self.name: self.name = get_randomized_name(get_profile_username()) - logger.warning(f"Creating resoure group '{self.name}'") + logger.warning(f"Creating resource group '{self.name}'") self.create() else: - logger.warning(f"Using resoure group '{self.name}'") # TODO use .info() + logger.warning(f"Using resource group '{self.name}'") # TODO use .info() class Resource: @@ -128,17 +128,6 @@ def check_exists(self): self.exists = self.get() is not None return self.exists - def create_if_needed(self, *args, **kwargs): - if not self.check_exists(): - logger.warning( - f"Creating {type(self).__name__} '{self.name}' in resource group {self.resource_group.name}" - ) - self.create(*args, **kwargs) - else: - logger.warning( - f"Using {type(self).__name__} '{self.name}' in resource group {self.resource_group.name}" - ) # TODO use .info() - class ContainerAppEnvironment(Resource): def __init__( @@ -182,9 +171,20 @@ def _get(self): self.cmd, self.resource_group.name, self.name ) - def create(self, app_name): - if self.name is None: - self.name = "{}-env".format(app_name).replace("_", "-") + def create_if_needed(self, app_name): + if not self.check_exists(): + if self.name is None: + self.name = "{}-env".format(app_name).replace("_", "-") + logger.warning( + f"Creating {type(self).__name__} '{self.name}' in resource group {self.resource_group.name}" + ) + self.create() + else: + logger.warning( + f"Using {type(self).__name__} '{self.name}' in resource group {self.resource_group.name}" + ) # TODO use .info() + + def create(self): env = create_managed_environment( self.cmd, self.name, @@ -618,7 +618,6 @@ def _get_registry_details(cmd, app: "ContainerApp"): registry_rg = _get_acr_rg(app) else: registry_rg = app.resource_group.name - user = get_profile_username() registry_name = app.env.name.replace("-", "").lower() registry_name = ( registry_name @@ -734,14 +733,11 @@ def find_existing_acr(cmd, resource_group_name, env_name, default_name, default_ client = cf_acr_registries(cmd.cli_ctx) acr_list = list_acr(client, resource_group_name) acr_list = list(acr_list) - acr = None - acr_list = [a for a in acr_list if env_name.lower().replace('-','') in a.name.lower()] + acr_list = [a for a in acr_list if env_name.lower().replace('-', '') in a.name.lower()] if acr_list: acr = acr_list[0] - - if acr: app.should_create_acr = False return acr.name, parse_resource_id(acr.id)["resource_group"], True else: - return default_name, default_rg, False \ No newline at end of file + return default_name, default_rg, False diff --git a/src/containerapp/azext_containerapp/commands.py b/src/containerapp/azext_containerapp/commands.py index 57fd157ae7f..158616ca0b2 100644 --- a/src/containerapp/azext_containerapp/commands.py +++ b/src/containerapp/azext_containerapp/commands.py @@ -51,7 +51,7 @@ def load_command_table(self, _): g.custom_command('update', 'update_containerapp', supports_no_wait=True, exception_handler=ex_handler_factory(), table_transformer=transform_containerapp_output) g.custom_command('delete', 'delete_containerapp', supports_no_wait=True, confirmation=True, exception_handler=ex_handler_factory()) g.custom_command('exec', 'containerapp_ssh', validator=validate_ssh) - g.custom_command('up', 'containerapp_up', supports_no_wait=True, exception_handler=ex_handler_factory()) + g.custom_command('up', 'containerapp_up', supports_no_wait=False, exception_handler=ex_handler_factory()) g.custom_command('browse', 'open_containerapp_in_browser') with self.command_group('containerapp replica', is_preview=True) as g: diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index 6da0e5ea34b..f5e5cc393d8 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -2019,7 +2019,7 @@ def containerapp_up(cmd, if image: if ingress and not target_port: target_port = 80 - logger.warning("No ingress provided, defaulting to port 80. Try specifying --target-port or run `az containerapp ingress enable` after your containerapp has been created.") + logger.warning("No ingress provided, defaulting to port 80. Try `az containerapp up --ingress %s --target-port ` to set a custom port.", ingress) dockerfile_content = _get_dockerfile_content(repo, branch, token, source, context_path, dockerfile) ingress, target_port = _get_ingress_and_target_port(ingress, target_port, dockerfile_content) @@ -2034,12 +2034,13 @@ def containerapp_up(cmd, if app.get()["properties"]["provisioningState"] == "InProgress": raise ValidationError("Containerapp has an existing provisioning in progress. Please wait until provisioning has completed and rerun the command.") + resource_group.create_if_needed() + env.create_if_needed(name) + if source or repo: _get_registry_from_app(app) # if the app exists, get the registry _get_registry_details(cmd, app) # fetch ACR creds from arguments registry arguments - resource_group.create_if_needed() - env.create_if_needed(name) app.create_acr_if_needed() if source: @@ -2172,6 +2173,9 @@ def containerapp_up_logic(cmd, resource_group_name, name, managed_env, image, en registries_def.append(registry) - if ca_exists: - return ContainerAppClient.update(cmd, resource_group_name, name, containerapp_def) - return ContainerAppClient.create_or_update(cmd, resource_group_name, name, containerapp_def) + try: + if ca_exists: + return ContainerAppClient.update(cmd, resource_group_name, name, containerapp_def) + return ContainerAppClient.create_or_update(cmd, resource_group_name, name, containerapp_def) + except Exception as e: + handle_raw_exception(e)