Updated registry creation experience

src/command_modules/azure-cli-acr/README.rst

@@ -33,7 +33,7 @@ Create a container registry
         --resource-group -g [Required]: Name of resource group.
         --app-id                      : The app id of an existing service principal. If provided, no
                                         --new-sp or -p should be specified.
-        --disable-admin               : Disable admin user.
+        --enable-admin                : Enable admin user.
         --new-sp                      : Create a new service principal. If provided, no --app-id should
                                         be specified. Optional: Use -p to specify a password.
         --password -p                 : Password used to log into a container registry.
@@ -44,7 +44,7 @@ Create a container registry
     Examples
         Create a container registry with a new storage account
             az acr create -n myRegistry -g myResourceGroup -l southus
-        Create a container registry with a specified new/existing storage account
+        Create a container registry with an existing storage account
             az acr create -n myRegistry -g myResourceGroup -l southus -s myStorageAccount
         Create a container registry with a new service principal
             az acr create -n myRegistry -g myResourceGroup -l southus --new-sp -p myPassword -r Owner

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

@@ -35,20 +35,34 @@ def arm_get_registries_in_resource_group(resource_group_name):
     result = get_resources_in_resource_group(resource_group_name, RESOURCE_TYPE)
     return [Registry(item.id, item.name, item.location, item.tags) for item in result]
 
-def arm_get_registry_by_name(registry_name):
-    '''Returns the container registry that matches the registry name.
-    :param str registry_name: The name of container registry
+def _arm_get_resource_by_name(resource_name, resource_type):
+    '''Returns the ARM resource in the current subscription with resource_name.
+    :param str resource_name: The name of resource
+    :param str resource_type: The type of resource
     '''
-    registries = arm_get_registries_in_subscription()
-    elements = [item for item in registries if item.name.lower() == registry_name.lower()]
+    result = get_resources_in_subscription(resource_type)
+    elements = [item for item in result if item.name.lower() == resource_name.lower()]
 
     if len(elements) == 0:
         return None
     elif len(elements) == 1:
         return elements[0]
     else:
         raise CLIError(
-            'More than one container registries are found with name: {}'.format(registry_name))
+            'More than one resources with type {} are found with name: {}'.format(
+                resource_type, resource_name))
+
+def arm_get_registry_by_name(registry_name):
+    '''Returns the named container registry.
+    :param str registry_name: The name of container registry
+    '''
+    return _arm_get_resource_by_name(registry_name, RESOURCE_TYPE)
+
+def arm_get_storage_account_by_name(storage_account_name):
+    '''Returns the named storage account.
+    :param str storage_account_name: The name of storage account
+    '''
+    return _arm_get_resource_by_name(storage_account_name, 'Microsoft.Storage/storageAccounts')
 
 def arm_deploy_template(resource_group_name,
                         registry_name,
@@ -67,7 +81,7 @@ def arm_deploy_template(resource_group_name,
     import os
 
     parameters = _parameters(registry_name, location, storage_account_name, admin_user_enabled)
-    storage_account_resource_group, _ = _arm_get_storage_account(storage_account_name)
+    storage_account_resource_group, _ = _parse_storage_account(storage_account_name)
 
     if storage_account_resource_group:
         file_path = os.path.join(os.path.dirname(__file__), 'template.existing.json')
@@ -128,29 +142,25 @@ def _parameters(registry_name,
     }
     return parameters
 
-def _arm_get_storage_account(storage_account_name):
-    '''Returns the dict of tags in the storage account.
+def _parse_storage_account(storage_account_name):
+    '''Returns resource group and tags in the storage account.
     :param str storage_account_name: The name of storage account
     '''
-    result = get_resources_in_subscription('Microsoft.Storage/storageAccounts')
-    elements = [item for item in result if item.name.lower() == storage_account_name.lower()]
+    storage_account = arm_get_storage_account_by_name(storage_account_name)
 
-    if len(elements) == 0:
-        return None, None
-    elif len(elements) == 1:
-        storage_account_resource_group = get_resource_group_name_by_resource_id(elements[0].id)
-        return storage_account_resource_group, elements[0].tags
+    if storage_account:
+        storage_account_resource_group = get_resource_group_name_by_resource_id(storage_account.id)
+        return storage_account_resource_group, storage_account.tags
     else:
-        raise CLIError(
-            'More than one storage accounts are found with name: {}'.format(storage_account_name))
+        return None, None
 
 def add_tag_storage_account(storage_account_name, registry_name):
     '''Add a new tag (key, value) to the storage account.
     :param str storage_account_name: The name of storage account
     :param str registry_name: The name of container registry
     '''
     from azure.mgmt.storage.models import StorageAccountUpdateParameters
-    storage_account_resource_group, tags = _arm_get_storage_account(storage_account_name)
+    storage_account_resource_group, tags = _parse_storage_account(storage_account_name)
 
     tags[registry_name.lower()] = 'acr'
     client = get_storage_service_client().storage_accounts
@@ -165,7 +175,7 @@ def delete_tag_storage_account(storage_account_name, registry_name):
     :param str registry_name: The name of container registry
     '''
     from azure.mgmt.storage.models import StorageAccountUpdateParameters
-    storage_account_resource_group, tags = _arm_get_storage_account(storage_account_name)
+    storage_account_resource_group, tags = _parse_storage_account(storage_account_name)
     registry_name = registry_name.lower()
 
     if registry_name in tags and tags[registry_name] == 'acr':

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

@@ -44,7 +44,7 @@
                 - name: Create a container registry with a new storage account
                   text:
                     az acr create -n myRegistry -g myResourceGroup -l southus
-                - name: Create a container registry with a specified new/existing storage account
+                - name: Create a container registry with an existing storage account
                   text:
                     az acr create -n myRegistry -g myResourceGroup -l southus -s myStorageAccount
                 - name: Create a container registry with a new service principal

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

@@ -6,8 +6,6 @@
 import re
 import uuid
 
-from azure.mgmt.resource.resources.models.resource_group import ResourceGroup
-
 from azure.cli.command_modules.acr.mgmt_acr.models import RegistryNameCheckRequest
 
 from azure.cli.core._util import CLIError
@@ -21,6 +19,10 @@
     get_acr_service_client,
     get_arm_service_client
 )
+from ._arm_utils import arm_get_storage_account_by_name
+
+import azure.cli.core._logging as _logging
+logger = _logging.get_az_logger(__name__)
 
 def validate_registry_name(namespace):
     if namespace.registry_name:
@@ -50,20 +52,39 @@ def validate_registry_name_create(namespace):
 def validate_storage_account_name(namespace):
     client = storage_client_factory().storage_accounts
 
-    if namespace.storage_account_name is None:
+    if namespace.storage_account_name:
+        storage_account_name = namespace.storage_account_name
+        if arm_get_storage_account_by_name(storage_account_name) is None:
+            logger.warning('Command to create a storage account:')
+            logger.warning(
+                '  az storage account create ' +\
+                '-n <name> -g <resource-group> -l <location> --sku Standard_LRS')
+            logger.warning(
+                'The container registry must be at the same location as the storage account.')
+            raise CLIError(
+                'The storage account {} does not exist in the current subscription.'\
+                .format(storage_account_name))
+    else:
         while True:
             storage_account_name = str(uuid.uuid4()).replace('-', '')[:24]
             if client.check_name_availability(storage_account_name).name_available is True: #pylint: disable=E1101
                 namespace.storage_account_name = storage_account_name
-                break
+                logger.warning(
+                    'New storage account with name %s will be created and used.',
+                    storage_account_name)
+                return
 
 def validate_resource_group_name(namespace):
-    client = get_arm_service_client()
-
     if namespace.resource_group_name:
-        if not client.resource_groups.check_existence(namespace.resource_group_name):
-            parameters = ResourceGroup(location=namespace.location)
-            client.resource_groups.create_or_update(namespace.resource_group_name, parameters)
+        client = get_arm_service_client()
+        resource_group_name = namespace.resource_group_name
+
+        if not client.resource_groups.check_existence(resource_group_name):
+            logger.warning('Command to create a resource group:')
+            logger.warning('  az resource group create -n <name> -l <location>')
+            raise CLIError(
+                'The resource group {} does not exist in the current subscription.'\
+                .format(resource_group_name))
 
 def validate_password(namespace):
     if namespace.password and not namespace.new_sp:

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

@@ -3,7 +3,10 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 #---------------------------------------------------------------------------------------------
 
-from azure.cli.core.commands import cli_command
+from azure.cli.core.commands import (
+    cli_command,
+    LongRunningOperation
+)
 from azure.cli.core._util import CLIError
 from azure.cli.command_modules.role.custom import _create_role_assignment
 
@@ -53,7 +56,7 @@ def acr_create(registry_name, #pylint: disable=too-many-arguments
                app_id=None,
                password=None,
                role=DEFAULT_ROLE,
-               disable_admin=False):
+               enable_admin=False):
     '''Create a container registry.
     :param str registry_name: The name of container registry
     :param str resource_group_name: The name of resource group
@@ -63,7 +66,7 @@ def acr_create(registry_name, #pylint: disable=too-many-arguments
     :param str app_id: The app id of an existing service principal
     :param str password: The password used to log into the container registry
     :param str role: The name of role
-    :param bool disable_admin: Disable admin user
+    :param bool enable_admin: Enable admin user
     '''
     if new_sp and app_id:
         raise CLIError('new-service-principal and app-id should not be specified together.')
@@ -76,26 +79,37 @@ def acr_create(registry_name, #pylint: disable=too-many-arguments
          session_key) = create_service_principal(registry_name, password)
 
     # Create a container registry
-    arm_deploy_template(resource_group_name,
-                        registry_name,
-                        location,
-                        storage_account_name,
-                        not disable_admin).wait() # wait for the template deployment to finish
+    LongRunningOperation()(
+        arm_deploy_template(resource_group_name,
+                            registry_name,
+                            location,
+                            storage_account_name,
+                            enable_admin)
+    )
 
     client = get_acr_service_client().registries
     registry = client.get_properties(resource_group_name, registry_name)
     add_tag_storage_account(storage_account_name, registry_name)
 
+    logger.warning('\nCreate a new service principal and assign access:')
+    logger.warning(
+        '  az ad sp create-for-rbac --scopes %s --role Owner --secret <password>',
+        registry.id) #pylint: disable=E1101
+    logger.warning('\nUse an existing service principal and assign access:')
+    logger.warning(
+        '  az role assignment create --scope %s --role Owner --assignee <app-id>',
+        registry.id) #pylint: disable=E1101
+
     # Create role assignment
     if app_id:
         _create_role_assignment(role,
                                 app_id,
                                 scope=registry.id, #pylint: disable=E1101
                                 ocp_aad_session_key=session_key)
-        logger.warning("Service principal has been configured.")
-        logger.warning("  id(client_id):           " + app_id)
+        logger.warning('Service principal has been configured.')
+        logger.warning('  id(client_id):           %s', app_id)
         if password:
-            logger.warning("  password(client_secret): " + password)
+            logger.warning('  password(client_secret): %s', password)
 
     return registry
 
@@ -185,10 +199,10 @@ def acr_update(registry_name, #pylint: disable=too-many-arguments
                                 app_id,
                                 scope=registry.id, #pylint: disable=E1101
                                 ocp_aad_session_key=session_key)
-        logger.warning("Service principal has been configured.")
-        logger.warning("  id(client_id):           " + app_id)
+        logger.warning('Service principal has been configured.')
+        logger.warning('  id(client_id):           %s', app_id)
         if password:
-            logger.warning("  password(client_secret): " + password)
+            logger.warning('  password(client_secret): %s', password)
 
     # Set admin_user_enabled
     admin_user_enabled = None

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

@@ -3,7 +3,10 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 #---------------------------------------------------------------------------------------------
 
-from azure.cli.core.commands import cli_command
+from azure.cli.core.commands import (
+    cli_command,
+    LongRunningOperation
+)
 
 from ._factory import get_acr_service_client
 from ._arm_utils import (
@@ -16,12 +19,8 @@
     get_resource_group_name_by_resource_id,
     registry_not_found
 )
-
 from ._format import output_format
 
-import azure.cli.core._logging as _logging
-logger = _logging.get_az_logger(__name__)
-
 def acr_storage_update(registry_name,
                        storage_account_name,
                        resource_group_name=None):
@@ -39,13 +38,14 @@ def acr_storage_update(registry_name,
 
     old_storage_account_name = registry.properties.storage_account.name
 
-    # Create a container registry
-    arm_deploy_template(resource_group_name,
-                        registry_name,
-                        registry.location,
-                        storage_account_name,
-                        registry.properties.admin_user_enabled).wait()
-                        # wait for the template deployment to finish
+    # Update a container registry
+    LongRunningOperation()(
+        arm_deploy_template(resource_group_name,
+                            registry_name,
+                            registry.location,
+                            storage_account_name,
+                            registry.properties.admin_user_enabled)
+    )
 
     client = get_acr_service_client().registries
     registry = client.get_properties(resource_group_name, registry_name)

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/template.existing.json

@@ -35,7 +35,7 @@
         },
         "adminUserEnabled": {
             "type": "bool",
-            "defaultValue": true,
+            "defaultValue": false,
             "metadata": {
                 "description": "Enable admin user"
             }

src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/template.new.json

@@ -35,7 +35,7 @@
         },
         "adminUserEnabled": {
             "type": "bool",
-            "defaultValue": true,
+            "defaultValue": false,
             "metadata": {
                 "description": "Enable admin user"
             }