Merge pull request Azure#77 from haroonf/findacr2

ACR changes + bug fixes

src/containerapp/azext_containerapp/_up_utils.py

@@ -65,8 +65,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
@@ -91,10 +89,12 @@ def check_exists(self) -> bool:
 
     def create_if_needed(self):
         if not self.check_exists():
-            logger.warning(f"Creating resoure group '{self.name}'")
+            if not self.name:
+                self.name = get_randomized_name(get_profile_username())
+            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:
@@ -129,17 +129,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__(
@@ -156,23 +145,25 @@ 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
 
     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
 
@@ -181,9 +172,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,
@@ -275,12 +277,23 @@ def create(self, no_registry=False):
         )
 
     def create_acr_if_needed(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 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
@@ -608,11 +621,10 @@ 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.name.replace("-", "").lower()
+        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
@@ -715,7 +727,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(
@@ -724,3 +736,19 @@ 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_list = [a for a in acr_list if env_name.lower().replace('-', '') in a.name.lower()]
+
+    if acr_list:
+        acr = acr_list[0]
+        app.should_create_acr = False
+        return acr.name, parse_resource_id(acr.id)["resource_group"], True
+    else:
+        return default_name, default_rg, False

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:

src/containerapp/azext_containerapp/custom.py

@@ -2016,7 +2016,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)
@@ -2025,14 +2025,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 `az containerapp up --ingress %s --target-port <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)
@@ -2047,12 +2047,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:
@@ -2085,6 +2086,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"
 
@@ -2166,7 +2168,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:
@@ -2178,10 +2181,14 @@ 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)
 
-    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)