Skip to content

Conversation

@xfz11
Copy link
Contributor

@xfz11 xfz11 commented Apr 6, 2021

Description
resolve #16038
Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


This checklist is used to make sure that common guidelines for a pull request are followed.

@xfz11 xfz11 requested review from houk-ms, qwordy and yungezz as code owners April 6, 2021 08:48
@xfz11 xfz11 changed the title add VMSS Rolling Upgrade opt [Compute] add VMSS Rolling Upgrade opt Apr 6, 2021
if proximity_placement_group is not None:
vmss.proximity_placement_group = {'id': proximity_placement_group}

if max_batch_instance_percent is not None or max_unhealthy_instance_percent is not None \
Copy link
Member

Choose a reason for hiding this comment

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

why new condition check? it's better to have comments

@houk-ms
Copy link
Contributor

houk-ms commented Apr 7, 2021

Do we have any conventions when adding a short option for the parameter? @qwordy

If not, maybe we should have some.

@qwordy
Copy link
Member

qwordy commented Apr 7, 2021

Do we have any conventions when adding a short option for the parameter? @qwordy

If not, maybe we should have some.

It is an opportunity that you can draft one document.

Comment on lines +921 to +929
'mode': upgrade_policy_mode,
'rollingUpgradePolicy': {
'maxBatchInstancePercent': max_batch_instance_percent,
'maxUnhealthyInstancePercent': max_unhealthy_instance_percent,
'maxUnhealthyUpgradedInstancePercent': max_unhealthy_upgraded_instance_percent,
'pauseTimeBetweenBatches': pause_time_between_batches,
'enableCrossZoneUpgrade': enable_cross_zone_upgrade,
'prioritizeUnhealthyInstances': prioritize_unhealthy_instances
}
Copy link
Member

Choose a reason for hiding this comment

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

If they are all not specified, do we need to set these Nones? Does it break existing VM commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to my test, None value will not change the properties' original values

@xfz11 xfz11 merged commit 711cbca into Azure:dev Apr 9, 2021
Comment on lines +613 to +624
c.argument('max_batch_instance_percent', type=int, min_api='2020-12-01',
help='The maximum percent of total virtual machine instances that will be upgraded simultaneously by the rolling upgrade in one batch. Default: 20%')
c.argument('max_unhealthy_instance_percent', type=int, min_api='2020-12-01',
help='The maximum percentage of the total virtual machine instances in the scale set that can be simultaneously unhealthy. Default: 20%')
c.argument('max_unhealthy_upgraded_instance_percent', type=int, min_api='2020-12-01',
help='The maximum percentage of upgraded virtual machine instances that can be found to be in an unhealthy state. Default: 20%')
c.argument('pause_time_between_batches', min_api='2020-12-01',
help='The wait time between completing the update for all virtual machines in one batch and starting the next batch. Default: 0 seconds')
c.argument('enable_cross_zone_upgrade', arg_type=get_three_state_flag(), min_api='2020-12-01',
help='Set this Boolean property will allow VMSS to ignore AZ boundaries when constructing upgrade batches, and only consider Update Domain and maxBatchInstancePercent to determine the batch size')
c.argument('prioritize_unhealthy_instances', arg_type=get_three_state_flag(), min_api='2020-12-01',
help='Set this Boolean property will lead to all unhealthy instances in a scale set getting upgraded before any healthy instances')
Copy link
Contributor

Choose a reason for hiding this comment

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

@xfz11 Hi, I see that the min_api of these parameters is 2020-12-01, but in fact, these parameters are already supported in version 2020-06-01.
Swagger link: code link

So I would like to confirm that those parameters are expected to be supported only after version 2020-12-01, right? (in this case, profile 2020-09-01-hybrid does not support those parameters)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VMSS Rolling Upgrade opt in flags

5 participants