Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding CRUD azure cli commands for Managed instance and Managed database resources #6428

Merged
merged 11 commits into from
May 31, 2018
17 changes: 17 additions & 0 deletions src/command_modules/azure-cli-sql/HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,23 @@
Release History
===============

2.0.27
++++++
* Added new Managed instance and Managed database CRUD commands.
* Managed instance commands:
* az sql managed instance create
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update this too :)

* az sql managed instance show
* az sql managed instance list
* az sql managed instance update
* az sql managed instance delete

* Managed database commands:
* az sql managed db create
* az sql managed db show
* az sql managed db list
* az sql managed instance restore
* az sql managed instance delete
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming these should say "db" instead of "instance" right?


2.0.26
++++++
* BREAKING CHANGES: Updated database, data warehouse, and elastic pool commands to use Azure-standard SKU properties for configuring performance level. This has resulted in some changes to the respose objects returned from db, dw, and elastic-pool commands.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,3 +426,83 @@
- name: Create a vnet rule by providing the vnet and subnet name. The subnet id is created by taking the resource group name and subscription id of the SQL server.
text: az sql server vnet-rule create --subnet subnetName --vnet-name vnetName
"""
helps['sql managed instance'] = """
type: group
short-summary: Manage SQL managed instances.
"""
helps['sql managed instance create'] = """
type: command
short-summary: Create a managed instance.
examples:
- name: Create a managed instance with specified parameters and with identity
text: az sql managed instance create -g mygroup -mi myinstance -l mylocation -i -u myusername -p mypassword --license-type mylicensetype --subnet-id mysubnetid --vcores vcorenum --storage-size-in-gb storagesizenum --sku-name skuname
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For managed instance commands, the "managed instance name parameter" must be -n

"""
helps['sql managed instance list'] = """
type: command
short-summary: List available managed instances.
examples:
- name: List all managed instances in the current subscription.
text: az sql managed instance list
- name: List all managed instances in a resource group.
text: az sql managed instance list -g mygroup
"""
helps['sql managed instance show'] = """
type: command
short-summary: Get the details for a managed instance.
examples:
- name: Get the details for a managed instance
text: az sql managed instance show -g mygroup -mi myinstance
"""
helps['sql managed instance update'] = """
type: command
short-summary: Update a managed instance.
examples:
- name: Updates a managed instance with specified parameters and with identity
text: az sql managed instance update -g mygroup -mi myinstance -i -p mypassword --license-type mylicensetype --vcores vcorenum --storage-size-in-gb storagesizenum
"""
helps['sql managed instance delete'] = """
type: command
short-summary: Delete a managed instance.
examples:
- name: Delete a managed instance
text: az sql managed instance delete -g mygroup -mi myinstance --yes
"""
helps['sql managed db'] = """
type: group
short-summary: Manage managed databases.
"""
helps['sql managed db create'] = """
type: command
short-summary: Create a managed database.
examples:
- name: Create a managed database with specified collation
text: az sql managed db create -g mygroup -mi myinstance -name mymanageddb --collation collationname
"""
helps['sql managed db list'] = """
type: command
short-summary: List maanged databases on a managed instance.
examples:
- name: List maanged databases on a managed instance
text: az sql managed db list -g mygroup -mi myinstance
"""
helps['sql managed db show'] = """
type: command
short-summary: Get the details for a managed database.
examples:
- name: Get the details for a managed database
text: az sql managed db show -g mygroup -mi myinstance -name mymanageddb
"""
helps['sql managed db restore'] = """
type: command
short-summary: Restores a managed database.
examples:
- name: Restores a managed database using Point in time restore
text: az sql managed db restore -g mygroup -mi myinstance -name mymanageddb --target-managed-database-name targetmidb --time pointintime
"""
helps['sql managed db delete'] = """
type: command
short-summary: Delete a managed database.
examples:
- name: Delete a managed database
text: az sql managed db delete -g mygroup -mi myinstance -name mymanageddb --yes
"""
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
ExportRequest,
Server,
ServerAzureADAdministrator,
Sku
Sku,
ManagedInstance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make alphabetical please

)

from azure.mgmt.sql.models.sql_management_client_enums import (
Expand Down Expand Up @@ -140,9 +141,13 @@ def __repr__(self):
help='Specifies whether to enable zone redundancy',
arg_type=get_three_state_flag())

managed_instance_param_type = CLIArgumentType(
options_list=['--managed_instance', '-mi'],
help='Name of the Azure SQL managed instance.')

db_service_objective_examples = 'Basic, S0, P1, GP_Gen4_1, BC_Gen5_2.'
dw_service_objective_examples = 'DW100, DW1000c'
mi_service_objective_examples = 'GP_Gen4, GP_Gen5'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unused, indeed, removed it



###############################################
Expand Down Expand Up @@ -1005,3 +1010,133 @@ def _configure_security_policy_storage_params(arg_ctx):
c.extra('vnet_name',
options_list=['--vnet-name'],
help='The virtual network name')

###############################################
# sql managed instance #
###############################################
with self.argument_context('sql managed instance') as c:
c.argument('managed_instance_name',
help='The managed instance name',
options_list=['--name', '-n'],
# Allow --ids command line argument. id_part=name is 1st name in uri
id_part='name')

with self.argument_context('sql managed instance create') as c:
# Create args that will be used to build up the ManagedInstance object
create_args_for_complex_type(
c, 'parameters', ManagedInstance, [
'administrator_login',
'administrator_login_password',
'location',
'license_type',
'subnet_id',
'vcores',
'storage_size_in_gb',
'sku_name',
])

c.argument('administrator_login',
options_list=['--admin-user', '-u'],
required=True)

c.argument('administrator_login_password',
options_list=['--admin-password', '-p'],
required=True)

c.argument('subnet_id',
required=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all these parameters really required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, subnet_id, admin-user and admin-password are required. license-type, vcores, storage and sku_name are not required and they get default values if not specified in request. I will talk to the PMs to see if we want to force explicit setting of this parameters, or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameters really should not be required if they are not required. More required parameters = more difficult for new users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, checked, will remove requirement where it is not needed.

options_list=['--subnet-id', '-subn'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to remove or change -subn. These abbreviations are supposed to be just 1 character :) Also let's make sure this parameter aligns with az sql server vnet-rule create command. You can reused the param validator that the vnet team wrote.

    --subnet            [Required]: Name or ID of the subnet that allows access to an Azure Sql
                                    Server. If subnet name is provided, --vnet-name must be
                                    provided.
    --vnet-name                   : The virtual network name.

Actually I just realized that --vnet-name should just be --vnet. I'll make that update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it, left only --subnet


c.argument('license_type',
required=True,
arg_type=get_enum_type(DatabaseLicenseType))

c.argument('vcores',
required=True,
help='Number of vcores to search for. If unspecified, all vcore sizes are shown.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like MI to be consistent with DB and pool. So this parameter should be --capacity. Also the help text here was copied from capabilities command which is searching - you should use help text similar to db create.


c.argument('storage_size_in_gb',
required=True,
help='Number of vcores to search for. If unspecified, all vcore sizes are shown.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use --storage and use unit converter like db and pool commands


c.argument('sku_name',
required=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use same design as DB & pool: --edition/--tier and --family parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will do. I have --family, --tier and --service-objective parameters now, same as for SQL db.
But, now service-objective has to be in sync with --family and --tier in order for command to work (or to have some unsupported value). For example, if I have set --family Gen4 and --tier GeneralPurpose then --service-objective has to be GP_Gen4 in order for command to work. So this is redundant parameter in that case. Is this ok behaviour? Do we have something like parameter sets in Azure CLI, similar to what we have in PowerShell?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO you don't need --service-objective. The reason for having --family and --tier is because those are just basic strings with a small number of known values, but SKU name is a combination of those which is not obvious to new users. We have service objective for db create commands for backwards compatibility, I did not add --service-objective to elastic-pool create even though I could have.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer your question, no there are no parameter sets like in PowerShell. If you look in custom.py you will see that there is code to look up the sku name in capabilities based on family and tier. It's a little complicated, but I strongly feel that it substantially improves the new user experience because the values are more discoverable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also having separate params allows syntax like this: az sql elastic-pool update -e BusinessCritical which will update the edition but keep the family the same - very convenient for users :)

Copy link
Contributor Author

@petrajkogit petrajkogit May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I am trying to remove --service-objective parameter which maps to sku.name, but sku.name is required parameter it seems. So I would need to populate sku.name even if not requiring it in command from other two parameters (--family and --tier)? Or I am missing something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it needs to be populated in custom.py. See _find_elastic_pool_sku_from_capabilities for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your MI code can be mostly the same, except that:

  • MI has family capability and perf level capability at different levels so your code to find perf level will be simpler
  • All MI's have family (as opposed to DTU dbs/pools which have empty family) so you don't need allow_reset_family param.
    This should reduce some code complexity.

Something like this:

def _find_managed_instance_sku_from_capabilities(cli_ctx, location, sku):
    '''
    Given a requested sku which may have some properties filled in
    (e.g. tier and capacity), finds the canonical matching sku
    from the given location's capabilities.
    '''

    logger.debug('_find_managed_instance_sku_from_capabilities input: %s', sku)

    if sku.name:
        # User specified sku.name, so nothing else needs to be resolved.
        logger.debug('_find_managed_instance_sku_from_capabilities return sku as is')
        return sku

    if not _any_sku_values_specified(sku):
        # User did not request any properties of sku, so just wipe it out.
        # Server side will pick a default.
        logger.debug('_find_managed_instance_sku_from_capabilities return None')
        return None

    # Some properties of sku are specified, but not name. Use the requested properties
    # to find a matching capability and copy the sku from there.

    # Get default server version capability
    capabilities_client = get_sql_capabilities_operations(cli_ctx, None)
    capabilities = capabilities_client.list_by_location(location, CapabilityGroup.supported_managed_instance_versions)
    managed_instance_version_capability = _get_default_capability(capabilities.supported_managed_instance_versions)

    # Find edition capability, based on requested sku properties
    edition_capability = _find_edition_capability(sku, server_version_capability.supported_editions)

    # Find family level capability, based on requested sku properties
    family_capability = _find_family_capability(sku, edition_capability.supported_families) # need to implement this function

    # Copy sku object from capability
    result = Sku(...)
    logger.debug('_find_managed_instance_sku_from_capabilities return: %s', result)
    return result

Note that db & pool capabilities return the "canonical sku" so we can copy the sku object directly from the chosen capability. You don't have this so you have to construct the sku object. This might be something Nemanja could add in managed instance capabilities.


c.argument('assign_identity',
options_list=['--assign-identity', '-i'],
help='Generate and assign an Azure Active Directory Identity for this managed instance'
'for use with key management services like Azure KeyVault.')

with self.argument_context('sql managed instance update') as c:
# Create args that will be used to build up the ManagedInstance object
create_args_for_complex_type(
c, 'parameters', ManagedInstance, [
'administrator_login_password',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not have sku arguments?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can sku not be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not currently. We cannot update to different edition. We can only update password, storage, capacity and identity info

'license_type',
'v_cores',
'storage_size_in_gb',
])

c.argument('administrator_login_password',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of repeating c.argument('administrator_login_password', and c.argument('assign_identity',, you can just define them once in in with self.argument_context('sql managed-instance') as c:, section

Copy link
Contributor Author

@petrajkogit petrajkogit May 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not actually repeating them. They are different in properties. administrator_login_password is required when creating instance, and optional when updating it. And assign_identity provides additional help info when doing update instance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

options_list=['--admin-password', '-p'],
required=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems wrong that this is required. There is no way to not update the password?

Copy link
Contributor Author

@petrajkogit petrajkogit May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not be required. This was copied from create parameters where password is required. Fixed


c.argument('license_type',
required=False,
arg_type=get_enum_type(DatabaseLicenseType))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a ManagedInstanceLicenseType?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not. Will have to use this one I guess


c.argument('v_cores',
required=False,
help='Number of vcores to search for. If unspecified, all vcore sizes are shown.')

c.argument('storage_size_in_gb',
required=False,
help='Number of vcores to search for. If unspecified, all vcore sizes are shown.')

c.argument('assign_identity',
options_list=['--assign-identity', '-i'],
help='Generate and assign an Azure Active Directory Identity for this managed instance'
'for use with key management services like Azure KeyVault.')

###############################################
# sql managed db #
###############################################
with self.argument_context('sql managed db') as c:
c.argument('managed_instance_name',
arg_type=managed_instance_param_type,
# Allow --ids command line argument. id_part=name is 1st name in uri
id_part='name')

c.argument('database_name',
options_list=['--name', '-n'],
help='The name of the Azure SQL Managed Database.',
# Allow --ids command line argument. id_part=child_name_1 is 2nd name in uri
id_part='child_name_1')

with self.argument_context('sql managed db create') as c:
create_args_for_complex_type(
c, 'parameters', ManagedInstance, [
'collation',
])

c.argument('collation',
required=False,
help='The collation of the Azure SQL Managed Database collation to use.')

with self.argument_context('sql managed db restore') as c:
create_args_for_complex_type(
c, 'parameters', ManagedInstance, [
'target_managed_database_name',
'restore_point_in_time'
])
c.argument('target_managed_database_name',
help='Name of the database that will be created as the restore destination.')

restore_point_arg_group = 'Restore Point'

c.argument('restore_point_in_time',
options_list=['--time', '-t'],
arg_group=restore_point_arg_group,
help='The point in time of the source database that will be restored to create the'
' new database. Must be greater than or equal to the source database\'s'
' earliestRestoreDate value.')
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,11 @@ def get_sql_subscription_usages_operations(cli_ctx, _):

def get_sql_virtual_network_rules_operations(cli_ctx, _):
return get_sql_management_client(cli_ctx).virtual_network_rules


def get_sql_managed_instances_operations(cli_ctx, _):
return get_sql_management_client(cli_ctx).managed_instances


def get_sql_managed_databases_operations(cli_ctx, _):
return get_sql_management_client(cli_ctx).managed_databases
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
get_sql_server_usages_operations,
get_sql_subscription_usages_operations,
get_sql_virtual_network_rules_operations,
get_sql_managed_instances_operations,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetical please :)

get_sql_managed_databases_operations
)

from ._validators import (
Expand Down Expand Up @@ -406,3 +408,41 @@ def load_command_table(self, _):
c.command('create', 'create_or_update')
c.command('delete', 'delete')
c.custom_command('set', 'server_dns_alias_set')

###############################################
# sql managed instance #
###############################################

managed_instances_operations = CliCommandType(
operations_tmpl='azure.mgmt.sql.operations.managed_instances_operations#ManagedInstancesOperations.{}',
client_factory=get_sql_managed_instances_operations)

with self.command_group('sql managed instance',
managed_instances_operations,
client_factory=get_sql_managed_instances_operations) as g:

g.custom_command('create', 'managed_instance_create')
g.command('delete', 'delete', confirmation=True)
g.command('show', 'get')
g.custom_command('list', 'managed_instance_list')
g.generic_update_command('update', custom_func_name='managed_instance_update')

###############################################
# sql managed db #
###############################################

managed_databases_operations = CliCommandType(
operations_tmpl='azure.mgmt.sql.operations.managed_databases_operations#ManagedDatabasesOperations.{}',
client_factory=get_sql_managed_databases_operations)

with self.command_group('sql managed db',
managed_databases_operations,
client_factory=get_sql_managed_databases_operations) as g:

g.custom_command('create', 'managed_db_create',
supports_no_wait=True)
g.custom_command('restore', 'managed_db_restore',
supports_no_wait=True)
g.command('show', 'get')
g.custom_command('list', 'managed_db_list')
g.command('delete', 'delete', confirmation=True, supports_no_wait=True)
Loading