Updates from PR feedback

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_arm_utils.py

@@ -5,17 +5,16 @@
 
 from azure.cli.command_modules.acr.containerregistry.models import RegistryParameters
 
+from ._constants import RESOURCE_TYPE
 from ._factory import get_arm_service_client
 
 from azure.cli.command_modules.acr.containerregistry import VERSION
 
-resource_type = 'Microsoft.ContainerRegistry/registries'
-
 def arm_get_registries_in_subscription():
     '''Returns the list of container registries in the current subscription.
     '''
     client = get_arm_service_client()
-    filter_str = "resourceType eq '{}'".format(resource_type)
+    filter_str = "resourceType eq '{}'".format(RESOURCE_TYPE)
     result = list(client.resources.list(filter=filter_str))
 
     return [RegistryParameters(item.id, item.name, item.location, item.tags) for item in result]
@@ -25,7 +24,7 @@ def arm_get_registries_in_resource_group(resource_group):
     :param str resource_group: The name of resource group
     '''
     client = get_arm_service_client()
-    filter_str = "resourceType eq '{}'".format(resource_type)
+    filter_str = "resourceType eq '{}'".format(RESOURCE_TYPE)
     result = list(client.resource_groups.list_resources(resource_group, filter=filter_str))
 
     return [RegistryParameters(item.id, item.name, item.location, item.tags) for item in result]
@@ -44,15 +43,12 @@ def arm_get_registry_by_name(registry_name):
     else:
         raise ValueError('More than one container registries are found with name: ' + registry_name)
 
-def arm_deploy_template(resource_group, registry_name, location, #pylint: disable=too-many-arguments
-                        storage_account_name, deployment_name, mode='incremental'):
+def arm_deploy_template(resource_group, registry_name, location, storage_account_name):
     '''Deploys ARM template to create a container registry.
     :param str resource_group: The name of resource group
     :param str registry_name: The name of container registry
     :param str location: The name of location
     :param str storage_account_name: The name of storage account
-    :param str deployment_name: The name of deployment
-    :param str mode: The mode of deployment
     '''
     from azure.mgmt.resource.resources.models import DeploymentProperties
     from azure.cli.core._util import get_file_json
@@ -61,9 +57,10 @@ def arm_deploy_template(resource_group, registry_name, location, #pylint: disabl
     file_path = os.path.join(os.path.dirname(__file__), 'template.json')
     template = get_file_json(file_path)
     parameters = _parameters(registry_name, location, storage_account_name)
-    properties = DeploymentProperties(template=template, parameters=parameters, mode=mode)
+    properties = DeploymentProperties(template=template, parameters=parameters, mode='incremental')
 
     client = get_arm_service_client()
+    deployment_name = 'Deployment.' + registry_name
 
     return client.deployments.create_or_update(resource_group, deployment_name, properties)
 

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_constants.py

@@ -0,0 +1,6 @@
+#---------------------------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for license information.
+#---------------------------------------------------------------------------------------------
+
+RESOURCE_TYPE = 'Microsoft.ContainerRegistry/registries'

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_factory.py

@@ -4,6 +4,9 @@
 #---------------------------------------------------------------------------------------------
 
 from azure.cli.core._profile import Profile
+from azure.mgmt.resource.resources import ResourceManagementClient
+from azure.storage._constants import SERVICE_HOST_BASE
+
 from azure.cli.core.commands.client_factory import (
     configure_common_settings,
     get_mgmt_service_client
@@ -15,9 +18,6 @@
     VERSION
 )
 
-from azure.mgmt.resource.resources import ResourceManagementClient
-from azure.storage._constants import SERVICE_HOST_BASE
-
 def get_arm_service_client():
     '''Returns the client for managing ARM resources.
     '''

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_format.py

@@ -5,6 +5,7 @@
 
 from collections import OrderedDict
 
+from ._constants import RESOURCE_TYPE
 from ._utils import get_resource_group_by_registry
 
 _basic_map = {
@@ -60,7 +61,7 @@ def _format(item):
         return _format_deployment(item)
     elif isinstance(item, dict) and \
          'id' in item and \
-         '/providers/Microsoft.ContainerRegistry/registries/' in item['id']:
+         ('/providers/' + RESOURCE_TYPE + '/') in item['id']:
         return _format_registry(item)
     else:
         raise ValueError('Unknown item: ' + str(item))

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_params.py

@@ -9,12 +9,19 @@
 )
 
 from azure.cli.core.commands.parameters import (
-    name_type,
     resource_group_name_type,
     location_type,
     tags_type
 )
 
+from ._validators import validate_registry_name
+
+registry_name_type = CliArgumentType(
+    options_list=('--name', '-n'),
+    help='Name of container registry',
+    validator=validate_registry_name
+)
+
 storage_account_name_type = CliArgumentType(
     options_list=('--storage-account-name', '-s'),
     help='Name of storage account.'
@@ -25,7 +32,7 @@
     help='Key of storage account.'
 )
 
-register_cli_argument('acr', 'registry_name', arg_type=name_type)
+register_cli_argument('acr', 'registry_name', arg_type=registry_name_type)
 register_cli_argument('acr', 'resource_group', arg_type=resource_group_name_type)
 register_cli_argument('acr', 'location', arg_type=location_type)
 register_cli_argument('acr', 'tags', arg_type=tags_type)

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py

@@ -3,15 +3,10 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 #---------------------------------------------------------------------------------------------
 
-import re
-
 from azure.cli.command_modules.acr.containerregistry.models import RegistryParameters
 
 from ._factory import get_registry_service_client
 
-import azure.cli.core._logging as _logging
-logger = _logging.get_az_logger(__name__)
-
 def _get_registries_in_subscription():
     '''Returns the list of container registries in the current subscription.
     '''
@@ -76,17 +71,3 @@ def _get_resource_group_keyword(resource_id):
         return '/resourceGroups/'
     else:
         raise ValueError('Invalid resource id: ' + resource_id)
-
-def validate_registry_name(registry_name):
-    '''Returns if the registry name is allowed.
-    :param str registry_name: The name of container registry
-    '''
-    if len(registry_name) < 5 or len(registry_name) > 60:
-        logger.error('The registry name must be between 5 and 60 characters.')
-        raise SystemExit(1)
-
-    p = re.compile('^([A-Za-z0-9]+)$')
-
-    if not p.match(registry_name):
-        logger.error('The registry name can contain only letters and numbers.')
-        raise SystemExit(1)

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_validators.py

@@ -0,0 +1,19 @@
+#---------------------------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for license information.
+#---------------------------------------------------------------------------------------------
+
+import re
+
+from azure.cli.core._util import CLIError
+
+def validate_registry_name(namespace):
+    if namespace.registry_name:
+        registry_name = namespace.registry_name
+        if len(registry_name) < 5 or len(registry_name) > 60:
+            raise CLIError('The registry name must be between 5 and 60 characters.')
+
+        p = re.compile('^([A-Za-z0-9]+)$')
+
+        if not p.match(registry_name):
+            raise CLIError('The registry name can contain only letters and numbers.')

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py

@@ -4,6 +4,7 @@
 #---------------------------------------------------------------------------------------------
 
 from azure.cli.core.commands import cli_command
+from azure.cli.core._util import CLIError
 
 from azure.cli.command_modules.acr.containerregistry.models import (
     RegistryParameters,
@@ -25,15 +26,11 @@
 
 from ._utils import (
     get_registry_by_name,
-    get_resource_group_by_registry,
-    validate_registry_name
+    get_resource_group_by_registry
 )
 
 from ._format import output_format
 
-import azure.cli.core._logging as _logging
-logger = _logging.get_az_logger(__name__)
-
 def acr_list(resource_group=None):
     '''Returns the list of container registries.
     :param str resource_group: The name of resource group
@@ -43,17 +40,15 @@ def acr_list(resource_group=None):
     else:
         return arm_get_registries_in_subscription()
 
-def acr_create(resource_group, registry_name, location, storage_account_name=None, #pylint: disable=too-many-arguments
-               storage_account_key=None, deployment_name="Microsoft.ContainerRegistry"):
+def acr_create(resource_group, registry_name, location, #pylint: disable=too-many-arguments
+               storage_account_name=None, storage_account_key=None):
     '''Returns the created container registry.
     :param str resource_group: The name of resource group
     :param str registry_name: The name of container registry
     :param str location: The name of location
     :param str storage_account_name: The name of storage account
     :param str storage_account_key: The key of storage account
-    :param str deployment_name: The name of deployment
     '''
-    validate_registry_name(registry_name)
     if storage_account_name:
         if storage_account_key:
             storage_account_properties = \
@@ -69,7 +64,7 @@ def acr_create(resource_group, registry_name, location, storage_account_name=Non
                 resource_group, registry_name, registry_parameters)
         else:
             return arm_deploy_template(
-                resource_group, registry_name, location, storage_account_name, deployment_name)
+                resource_group, registry_name, location, storage_account_name)
     else:
         return get_registry_service_client().create(
             resource_group, registry_name, RegistryParameters(location=location))
@@ -78,11 +73,9 @@ def acr_delete(registry_name):
     '''Deletes the container registry that matches the registry name.
     :param str registry_name: The name of container registry
     '''
-    validate_registry_name(registry_name)
     registry = arm_get_registry_by_name(registry_name)
     if registry is None:
-        logger.error('No container registry can be found with name: ' + registry_name)
-        raise SystemExit(1)
+        raise CLIError('No container registry can be found with name: ' + registry_name)
 
     resource_group = get_resource_group_by_registry(registry)
     return get_registry_service_client().delete(resource_group, registry_name)
@@ -91,11 +84,9 @@ def acr_show(registry_name):
     '''Returns the container registry that matches the registry name.
     :param str registry_name: The name of container registry
     '''
-    validate_registry_name(registry_name)
     registry = arm_get_registry_by_name(registry_name)
     if registry is None:
-        logger.error('No container registry can be found with name: ' + registry_name)
-        raise SystemExit(1)
+        raise CLIError('No container registry can be found with name: ' + registry_name)
 
     resource_group = get_resource_group_by_registry(registry)
     return get_registry_service_client().get_properties(resource_group, registry_name)
@@ -104,11 +95,9 @@ def acr_update(registry_name, tags=None):
     '''Returns the updated container registry that matches the registry name.
     :param str registry_name: The name of container registry
     '''
-    validate_registry_name(registry_name)
     registry = get_registry_by_name(registry_name)
     if registry is None:
-        logger.error('No container registry can be found with name: ' + registry_name)
-        raise SystemExit(1)
+        raise CLIError('No container registry can be found with name: ' + registry_name)
 
     resource_group = get_resource_group_by_registry(registry)
     newTags = registry.tags

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/repository.py

@@ -6,17 +6,12 @@
 import requests
 
 from azure.cli.core.commands import cli_command
+from azure.cli.core._util import CLIError
 
-from ._utils import (
-    get_registry_by_name,
-    validate_registry_name
-)
+from ._utils import get_registry_by_name
 
-import azure.cli.core._logging as _logging
-logger = _logging.get_az_logger(__name__)
-
-def _obtain_data_from_registry(registry_name, path, resultIndex, username, password):
-    registryEndpoint = 'https://' + registry_name + '.azurecr.io'
+def _obtain_data_from_registry(login_server, path, resultIndex, username, password):
+    registryEndpoint = 'https://' + login_server
     resultList = []
     executeNextHttpCall = True
 
@@ -45,40 +40,38 @@ def _obtain_data_from_registry(registry_name, path, resultIndex, username, passw
 
     return {resultIndex: resultList}
 
-def _validate_user_credentials(registry_name, path, resultIndex, username=None, password=None):
+def _validate_user_credentials(login_server, path, resultIndex, username=None, password=None):
     if username and password:
-        return _obtain_data_from_registry(registry_name, path, resultIndex, username, password)
+        return _obtain_data_from_registry(login_server, path, resultIndex, username, password)
 
     try:
+        registry_name = login_server[0:login_server.index('.')]
         registry = get_registry_by_name(registry_name)
         username = registry.properties.username
         password = registry.properties.key
-        return _obtain_data_from_registry(registry_name, path, resultIndex, username, password)
+        return _obtain_data_from_registry(login_server, path, resultIndex, username, password)
     except: #pylint: disable=W0702
-        logger.error('No container registry can be found with name: ' + registry_name)
-        logger.error('Please switch subscription or enter username/password')
-        raise SystemExit(1)
+        raise CLIError('No container registry can be found with name: ' + registry_name +
+                       '\nPlease switch subscription or enter username/password')
 
-def acr_catalog(registry_name, username=None, password=None):
+def acr_catalog(login_server, username=None, password=None):
     '''Returns the catalog of repositories in the specified registry.
-    :param str registry_name: The name of your Azure container registry
+    :param str login_server: The URL of registry login server
     :param str username: The username used to log into the container registry
     :param str password: The password used to log into the container registry
     '''
-    validate_registry_name(registry_name)
     path = '/v2/_catalog'
-    return _validate_user_credentials(registry_name, path, 'repositories', username, password)
+    return _validate_user_credentials(login_server, path, 'repositories', username, password)
 
-def acr_tags(registry_name, repository, username=None, password=None):
+def acr_tags(login_server, repository, username=None, password=None):
     '''Returns the list of tags for a given repository in the specified registry.
-    :param str registry_name: The name of your Azure container registry
+    :param str login_server: The URL of registry login server
     :param str repository: The repository to obtain tags from
     :param str username: The username used to log into the container registry
     :param str password: The password used to log into the container registry
     '''
-    validate_registry_name(registry_name)
     path = '/v2/' + repository + '/tags/list'
-    return _validate_user_credentials(registry_name, path, 'tags', username, password)
+    return _validate_user_credentials(login_server, path, 'tags', username, password)
 
 cli_command('acr catalog', acr_catalog)
 cli_command('acr tags', acr_tags)