-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Changes from 9 commits
1e8b0d2
00a3e49
ed33cdc
ef4e608
8a0bf79
bdb1c99
e38567a
5e6c067
2e27e38
7ea3a7c
5186243
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,8 @@ | |
ElasticPoolPerDatabaseSettings, | ||
ImportExtensionRequest, | ||
ExportRequest, | ||
ManagedDatabase, | ||
ManagedInstance, | ||
Server, | ||
ServerAzureADAdministrator, | ||
Sku | ||
|
@@ -54,7 +56,9 @@ | |
|
||
from ._validators import ( | ||
create_args_for_complex_type, | ||
validate_elastic_pool_id | ||
validate_elastic_pool_id, | ||
validate_managed_instance_storage_size, | ||
validate_subnet | ||
) | ||
|
||
|
||
|
@@ -82,7 +86,7 @@ def __call__(self, value): | |
try: | ||
uvals = (self.unit_map[unit_part] if unit_part else 1) / \ | ||
(self.unit_map[self.unit] if self.unit else 1) | ||
return self.result_type(uvals) * self.result_type(numeric_part) | ||
return self.result_type(uvals * self.result_type(numeric_part)) | ||
except KeyError: | ||
raise ValueError() | ||
|
||
|
@@ -140,6 +144,19 @@ 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.') | ||
|
||
storage_param_type = CLIArgumentType( | ||
options_list=['--storage'], | ||
type=SizeWithUnitConverter('GB', result_type=int, unit_map=dict(B=1.0 / (1024 * 1024 * 1024), | ||
kB=1.0 / (1024 * 1024), | ||
MB=1.0 / 1024, | ||
GB=1, | ||
TB=1024)), | ||
help='The storage size. If no unit is specified, defaults to gigabytes (GB).', | ||
validator=validate_managed_instance_storage_size) | ||
|
||
db_service_objective_examples = 'Basic, S0, P1, GP_Gen4_1, BC_Gen5_2.' | ||
dw_service_objective_examples = 'DW100, DW1000c' | ||
|
@@ -1005,3 +1022,160 @@ 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 mi') 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') | ||
|
||
c.argument('tier', | ||
arg_type=tier_param_type, | ||
help='The edition component of the sku. Allowed value is GeneralPurpose.') | ||
|
||
c.argument('family', | ||
arg_type=family_param_type, | ||
help='The compute generation component of the sku. ' | ||
'Allowed values include: Gen4, Gen5.') | ||
|
||
c.argument('storage_size_in_gb', | ||
options_list=['--storage'], | ||
arg_type=storage_param_type, | ||
help='The storage size of the managed instance. ' | ||
'Storage size must be specified in increments of 32 GB') | ||
|
||
c.argument('license_type', | ||
arg_type=get_enum_type(DatabaseLicenseType), | ||
help='The license type to apply for this managed instance.') | ||
|
||
c.argument('vcores', | ||
options_list=['--capacity', '-c'], | ||
help='The capacity of the managed instance in vcores.') | ||
|
||
with self.argument_context('sql mi 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', | ||
'license_type', | ||
'virtual_network_subnet_id', | ||
'vcores', | ||
'storage_size_in_gb' | ||
]) | ||
|
||
# Create args that will be used to build up the Managed Instance's Sku object | ||
create_args_for_complex_type( | ||
c, 'sku', Sku, [ | ||
'family', | ||
'name', | ||
'tier', | ||
]) | ||
|
||
c.ignore('name') # Hide 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.extra('vnet_name', | ||
options_list=['--vnet-name'], | ||
help='The virtual network name', | ||
validator=validate_subnet) | ||
|
||
c.argument('virtual_network_subnet_id', | ||
options_list=['--subnet'], | ||
required=True, | ||
help='Name or ID of the subnet that allows access to an Azure Sql Managed Instance. ' | ||
'If subnet name is provided, --vnet-name must be provided.') | ||
|
||
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 mi 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', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can sku not be updated? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
]) | ||
|
||
c.argument('administrator_login_password', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of repeating There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good |
||
options_list=['--admin-password', '-p']) | ||
|
||
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. If identity is already assigned - do nothing.') | ||
|
||
############################################### | ||
# sql managed db # | ||
############################################### | ||
with self.argument_context('sql midb') 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 midb create') as c: | ||
create_args_for_complex_type( | ||
c, 'parameters', ManagedDatabase, [ | ||
'collation', | ||
]) | ||
|
||
c.argument('collation', | ||
required=False, | ||
help='The collation of the Azure SQL Managed Database collation to use, ' | ||
'e.g.: SQL_Latin1_General_CP1_CI_AS or Latin1_General_100_CS_AS_SC') | ||
|
||
with self.argument_context('sql midb restore') as c: | ||
create_args_for_complex_type( | ||
c, 'parameters', ManagedDatabase, [ | ||
'target_managed_database_name', | ||
'target_managed_instance_name', | ||
'restore_point_in_time' | ||
]) | ||
|
||
c.argument('target_managed_database_name', | ||
options_list=['--dest-name'], | ||
required=True, | ||
help='Name of the managed database that will be created as the restore destination.') | ||
|
||
c.argument('target_managed_instance_name', | ||
options_list=['--dest-mi'], | ||
help='Name of the managed instance to restore managed database to. ' | ||
'This can be same managed instance, or another managed instance on same subscription. ' | ||
'When not specified it defaults to source managed instance.') | ||
|
||
c.argument('target_resource_group_name', | ||
options_list=['--dest-resource-group'], | ||
help='Name of the resource group of the managed instance to restore managed database to. ' | ||
'When not specified it defaults to source resource group.') | ||
|
||
restore_point_arg_group = 'Restore Point' | ||
|
||
c.argument('restore_point_in_time', | ||
options_list=['--time', '-t'], | ||
arg_group=restore_point_arg_group, | ||
required=True, | ||
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. Time should be in following format: "YYYY-MM-DDTHH:MM:SS"') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very nice! |
||
|
||
with self.argument_context('sql midb list') as c: | ||
c.argument('managed_instance_name', id_part=None) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This supresses the --ids parameter from sql managed-db list command as per: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add comment explaining what this does and why? That will help the next person to read this :) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,3 +123,16 @@ def validate_subnet(cmd, namespace): | |
else: | ||
raise CLIError('incorrect usage: [--subnet ID | --subnet NAME --vnet-name NAME]') | ||
delattr(namespace, 'vnet_name') | ||
|
||
|
||
############################################### | ||
# sql managed instance # | ||
############################################### | ||
|
||
|
||
def validate_managed_instance_storage_size(cmd, namespace): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why validate on client side? There is already validation on server side, and perhaps in the future more storage options will be available There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, the error when customer specifies wrong size is not well propagated back and customer cannot see actual reason for failure. We have a task to fix this, but in the meantime I was thinking to prevent this issue by validating it on client size. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. Please do fix and remote this check when you can. |
||
# Validate if entered storage size value is an increment of 32 if provided | ||
if (not namespace.storage_size_in_gb) or (namespace.storage_size_in_gb and namespace.storage_size_in_gb % 32 == 0): | ||
pass | ||
else: | ||
raise CLIError('incorrect usage: --storage must be specified in increments of 32 GB') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
result_type and unit_map don't need to be explicitly specified here, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we have to, otherwise wrong dict will be used. In our case storage should be specified in GB and converted to it before sending the request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't see that the multipliers are different. Cool!