Skip to content

[Compute] az vmss create/update: support automatic repairs #12374

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.

Sign up for GitHub

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

Merged
arrownj merged 6 commits into from
Mar 5, 2020
Merged

[Compute] az vmss create/update: support automatic repairs #12374

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f82897a
[Compute] az vmss create/update: support automatic repairs
arrownj Feb 27, 2020
9795d9e
{Compute} fix automatic repairs style error
arrownj Feb 27, 2020
f1ff97d
{Compute} update help message for --automatic-repairs-grace-period
arrownj Feb 28, 2020
81b79a4
{Compute} update validator error message for automatic repairs
arrownj Mar 3, 2020
1efdc92
{Compute} add automatic repairs arg group in vmss update
arrownj Mar 5, 2020
4b3219f
{Compute} fix sytle error in _params.py
arrownj Mar 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/azure-cli/azure/cli/command_modules/vm/_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ def load_arguments(self, _):
c.argument('instance_count', help='Number of VMs in the scale set.', type=int)
c.argument('disable_overprovision', help='Overprovision option (see https://azure.microsoft.com/documentation/articles/virtual-machine-scale-sets-overview/ for details).', action='store_true')
c.argument('upgrade_policy_mode', help=None, arg_type=get_enum_type(UpgradeMode))
c.argument('health_probe', help='Probe name from the existing load balancer, mainly used for rolling upgrade')
c.argument('health_probe', help='Probe name from the existing load balancer, mainly used for rolling upgrade or automatic repairs')
c.argument('vm_sku', help='Size of VMs in the scale set. Default to "Standard_DS1_v2". See https://azure.microsoft.com/pricing/details/virtual-machines/ for size info.')
c.argument('nsg', help='Name or ID of an existing Network Security Group.', arg_group='Network')
c.argument('eviction_policy', resource_type=ResourceType.MGMT_COMPUTE, min_api='2017-12-01', arg_type=get_enum_type(VirtualMachineEvictionPolicyTypes, default=None),
Expand Down Expand Up @@ -542,11 +542,15 @@ def load_arguments(self, _):
help='Enable terminate notification')
c.argument('ultra_ssd_enabled', ultra_ssd_enabled_type)
c.argument('scale_in_policy', scale_in_policy_type)
c.argument('enable_automatic_repairs', min_api='2018-10-01', arg_type=get_three_state_flag(),
Copy link
Member

@fengzhou-msft fengzhou-msft Mar 2, 2020

Choose a reason for hiding this comment

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

Shall we use action='store_true' instead of get_three_state_flag?

Copy link
Member

@fengzhou-msft fengzhou-msft Mar 2, 2020

Choose a reason for hiding this comment

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

Maybe get_three_state_flag is also fine. automaticRepairsPolicy is a property of the resource, and enable_automatic_repairs could be considered as the enabled property of automaticRepairsPolicy.

Copy link
Member

@fengzhou-msft fengzhou-msft Mar 2, 2020

Choose a reason for hiding this comment

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

I suggest to put enable_automatic_repairs and automatic_repairs_grace_period in an argument group so customers know that these two arguments need to be used together.

Copy link
Contributor Author

@arrownj arrownj Mar 2, 2020

Choose a reason for hiding this comment

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

yes, they are in the same argument group. enable_automatic_repairs and automatic_repairs_grace_period are in az vmss update. While only automatic_repairs_grace_period in az vmss create. You can reference above discussion with Catherine, I explain why we design like this. And want to hear your options. Thanks

Copy link
Member

@fengzhou-msft fengzhou-msft Mar 3, 2020

Choose a reason for hiding this comment

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

I mean you can add something like arg_group='Automatic Repair Policy', otherwise the two options are apart from each other in alpabetic order in help message. Usually we name the second level property with a prefix of first level property, they will be showed together in help message, for instance, the names could be automatic_repairs_policy_enable, automatic_reapirs_policy_grace_period, but the first name looks weird with enable as suffix, so I think arg_group can work well in this case.

Copy link
Contributor Author

@arrownj arrownj Mar 5, 2020

Choose a reason for hiding this comment

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

arg_group added

help='Enable automatic repairs', is_preview=True)

for scope in ['vmss create', 'vmss update']:
with self.argument_context(scope) as c:
c.argument('terminate_notification_time', min_api='2019-03-01',
help='Length of time (in minutes, between 5 and 15) a notification to be sent to the VM on the instance metadata server till the VM gets deleted')
c.argument('automatic_repairs_grace_period', min_api='2018-10-01', is_preview=True,
help='The amount of time (in minutes, between 30 and 90) for which automatic repairs are suspended due to a state change on VM.')

for scope, help_prefix in [('vmss update', 'Update the'), ('vmss wait', 'Wait on the')]:
with self.argument_context(scope) as c:
Expand Down
9 changes: 8 additions & 1 deletion src/azure-cli/azure/cli/command_modules/vm/_template_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ def build_vmss_resource(cmd, name, naming_prefix, location, tags, overprovision,
application_security_groups=None, ultra_ssd_enabled=None, proximity_placement_group=None,
terminate_notification_time=None, max_price=None, scale_in_policy=None,
os_disk_encryption_set=None, data_disk_encryption_sets=None,
data_disk_iops=None, data_disk_mbps=None):
data_disk_iops=None, data_disk_mbps=None, automatic_repairs_grace_period=None):

# Build IP configuration
ip_configuration = {
Expand Down Expand Up @@ -877,6 +877,13 @@ def build_vmss_resource(cmd, name, naming_prefix, location, tags, overprovision,
}
vmss_properties['virtualMachineProfile']['scheduledEventsProfile'] = scheduled_events_profile

if automatic_repairs_grace_period is not None:
automatic_repairs_policy = {
'enabled': 'true',
'gracePeriod': automatic_repairs_grace_period
}
vmss_properties['automaticRepairsPolicy'] = automatic_repairs_policy

if scale_in_policy:
vmss_properties['scaleInPolicy'] = {'rules': scale_in_policy}

Expand Down
26 changes: 26 additions & 0 deletions src/azure-cli/azure/cli/command_modules/vm/_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,7 @@ def process_vmss_create_namespace(cmd, namespace):
_validate_vm_vmss_msi(cmd, namespace)
_validate_proximity_placement_group(cmd, namespace)
_validate_vmss_terminate_notification(cmd, namespace)
_validate_vmss_create_automatic_repairs(cmd, namespace)

if namespace.secrets:
_validate_secrets(namespace.secrets, namespace.os_type)
Expand All @@ -1429,6 +1430,7 @@ def validate_vmss_update_namespace(cmd, namespace): # pylint: disable=unused-ar
raise CLIError("usage error: protection policies can only be applied to VM instances within a VMSS."
" Please use --instance-id to specify a VM instance")
_validate_vmss_update_terminate_notification_related(cmd, namespace)
_validate_vmss_update_automatic_repairs(cmd, namespace)
# endregion


Expand Down Expand Up @@ -1633,3 +1635,27 @@ def _validate_vmss_terminate_notification(cmd, namespace): # pylint: disable=un
"""
if namespace.terminate_notification_time is not None:
namespace.terminate_notification_time = 'PT' + namespace.terminate_notification_time + 'M'


def _validate_vmss_create_automatic_repairs(cmd, namespace): # pylint: disable=unused-argument
if namespace.automatic_repairs_grace_period is not None:
if namespace.load_balancer is None or namespace.health_probe is None:
raise CLIError("usage error: --load-balancer and --health-probe are required "
"when creating vmss with automatic repairs")
_validate_vmss_automatic_repairs(cmd, namespace)


def _validate_vmss_update_automatic_repairs(cmd, namespace): # pylint: disable=unused-argument
if namespace.enable_automatic_repairs is False and namespace.automatic_repairs_grace_period is not None:
raise CLIError("usage error: please enable --enable-automatic-repairs")
if namespace.enable_automatic_repairs is True and namespace.automatic_repairs_grace_period is None:
Copy link
Contributor

@mmyyrroonn mmyyrroonn Mar 2, 2020

Choose a reason for hiding this comment

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

Is it necessary? Based on your description, I think we can drop this check here.

Copy link
Contributor Author

@arrownj arrownj Mar 2, 2020

Choose a reason for hiding this comment

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

you mean we leave this validation to service side ?

Copy link
Member

@qwordy qwordy Mar 5, 2020

Choose a reason for hiding this comment

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

default value 30? maybe you can omit this validation

Copy link
Contributor Author

@arrownj arrownj Mar 5, 2020

Choose a reason for hiding this comment

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

per discussed offline, let's just keep this validation.

raise CLIError("usage error: please set --automatic-repairs-grace-period")
_validate_vmss_automatic_repairs(cmd, namespace)


def _validate_vmss_automatic_repairs(cmd, namespace): # pylint: disable=unused-argument
"""
Transform minutes to ISO 8601 formmat
"""
if namespace.automatic_repairs_grace_period is not None:
namespace.automatic_repairs_grace_period = 'PT' + namespace.automatic_repairs_grace_period + 'M'
13 changes: 10 additions & 3 deletions src/azure-cli/azure/cli/command_modules/vm/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,8 @@ def create_vmss(cmd, vmss_name, resource_group_name, image=None,
application_security_groups=None, ultra_ssd_enabled=None, ephemeral_os_disk=None,
proximity_placement_group=None, aux_subscriptions=None, terminate_notification_time=None,
max_price=None, computer_name_prefix=None, orchestration_mode='ScaleSetVM', scale_in_policy=None,
os_disk_encryption_set=None, data_disk_encryption_sets=None, data_disk_iops=None, data_disk_mbps=None):
os_disk_encryption_set=None, data_disk_encryption_sets=None, data_disk_iops=None, data_disk_mbps=None,
automatic_repairs_grace_period=None):
from azure.cli.core.commands.client_factory import get_subscription_id
from azure.cli.core.util import random_string, hash_string
from azure.cli.core.commands.arm import ArmTemplateBuilder
Expand Down Expand Up @@ -2403,7 +2404,7 @@ def _get_public_ip_address_allocation(value, sku):
terminate_notification_time=terminate_notification_time, max_price=max_price,
scale_in_policy=scale_in_policy, os_disk_encryption_set=os_disk_encryption_set,
data_disk_encryption_sets=data_disk_encryption_sets, data_disk_iops=data_disk_iops,
data_disk_mbps=data_disk_mbps)
data_disk_mbps=data_disk_mbps, automatic_repairs_grace_period=automatic_repairs_grace_period)

vmss_resource['dependsOn'] = vmss_dependencies

Expand Down Expand Up @@ -2653,7 +2654,8 @@ def update_vmss_instances(cmd, resource_group_name, vm_scale_set_name, instance_
def update_vmss(cmd, resource_group_name, name, license_type=None, no_wait=False, instance_id=None,
protect_from_scale_in=None, protect_from_scale_set_actions=None,
enable_terminate_notification=None, terminate_notification_time=None, ultra_ssd_enabled=None,
scale_in_policy=None, priority=None, max_price=None, proximity_placement_group=None, **kwargs):
scale_in_policy=None, priority=None, max_price=None, proximity_placement_group=None,
enable_automatic_repairs=None, automatic_repairs_grace_period=None, **kwargs):
vmss = kwargs['parameters']
client = _compute_client_factory(cmd.cli_ctx)

Expand Down Expand Up @@ -2686,6 +2688,11 @@ def update_vmss(cmd, resource_group_name, name, license_type=None, no_wait=False
TerminateNotificationProfile(not_before_timeout=terminate_notification_time,
enable=enable_terminate_notification)

if enable_automatic_repairs is not None or automatic_repairs_grace_period is not None:
AutomaticRepairsPolicy = cmd.get_models('AutomaticRepairsPolicy')
vmss.automatic_repairs_policy = \
AutomaticRepairsPolicy(enabled="true", grace_period=automatic_repairs_grace_period)

if ultra_ssd_enabled is not None:
if cmd.supported_api_version(min_api='2019-03-01', operation_group='virtual_machine_scale_sets'):
if vmss.additional_capabilities is None:
Expand Down
Loading