Skip to content

[Compute] az vmss reimage: Fix the bug that all instances will be reimaged after using --instance-id #25477

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
zhoxing-ms merged 3 commits into from
Feb 21, 2023
Merged

[Compute] az vmss reimage: Fix the bug that all instances will be reimaged after using --instance-id #25477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0788b0
Fix vmss reimage issue
zhoxing-ms Feb 16, 2023
917fba1
Fix linter issue
zhoxing-ms Feb 16, 2023
fd9c7dc
Address comments
zhoxing-ms Feb 16, 2023
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
14 changes: 8 additions & 6 deletions src/azure-cli/azure/cli/command_modules/vm/_help.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3396,14 +3396,16 @@
helps['vmss reimage'] = """
type: command
short-summary: Reimage VMs within a VMSS.
parameters:
- name: --instance-id
short-summary: VM instance ID. If missing, reimage all instances.
examples:
- name: Reimage VMs within a VMSS. (autogenerated)
- name: Reimage a VM instance within a VMSS.
text: |
az vmss reimage --instance-id 1 --name MyScaleSet --resource-group MyResourceGroup --subscription MySubscription
crafted: true
az vmss reimage --instance-ids 1 --name MyScaleSet --resource-group MyResourceGroup --subscription MySubscription
- name: Reimage a batch of VM instances within a VMSS.
text: |
az vmss reimage --instance-ids 1 2 3 --name MyScaleSet --resource-group MyResourceGroup --subscription MySubscription
Comment on lines +3400 to +3405
Copy link
Member

@jiasli jiasli Feb 16, 2023

Choose a reason for hiding this comment

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

Let's provide an example for reimaging all instances.

Copy link
Contributor Author

@zhoxing-ms zhoxing-ms Feb 16, 2023

Choose a reason for hiding this comment

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

OK, done

- name: Reimage all the VM instances within a VMSS.
text: |
az vmss reimage --name MyScaleSet --resource-group MyResourceGroup --subscription MySubscription
"""

helps['vmss restart'] = """
Expand Down
4 changes: 3 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 @@ -677,7 +677,9 @@ def load_arguments(self, _):
c.argument(dest, vmss_name_type, id_part=None) # due to instance-ids parameter

with self.argument_context('vmss reimage') as c:
c.argument('instance_id', nargs='+', help='Space-separated list of VM instance ID. If missing, reimage all instances.')
c.argument('instance_ids', nargs='+',
help='Space-separated list of VM instance ID. If missing, reimage all instances.',
options_list=['--instance-ids', c.deprecate(target='--instance-id', redirect='--instance-ids', hide=True)])

with self.argument_context('vmss create', operation_group='virtual_machine_scale_sets') as c:
VirtualMachineEvictionPolicyTypes = self.get_models('VirtualMachineEvictionPolicyTypes', resource_type=ResourceType.MGMT_COMPUTE)
Expand Down
12 changes: 7 additions & 5 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 @@ -3703,12 +3703,14 @@ def list_vmss_instance_public_ips(cmd, resource_group_name, vm_scale_set_name):
return [r for r in result if r.ip_address]


def reimage_vmss(cmd, resource_group_name, vm_scale_set_name, instance_id=None, no_wait=False):
def reimage_vmss(cmd, resource_group_name, vm_scale_set_name, instance_ids=None, no_wait=False):
client = _compute_client_factory(cmd.cli_ctx)
if instance_id:
for instance in instance_id:
sdk_no_wait(no_wait, client.virtual_machine_scale_set_vms.begin_reimage,
resource_group_name, vm_scale_set_name, instance)
if instance_ids:
VirtualMachineScaleSetVMInstanceIDs = cmd.get_models('VirtualMachineScaleSetVMInstanceIDs')
instance_ids = VirtualMachineScaleSetVMInstanceIDs(instance_ids=instance_ids)
Copy link
Member

@jiasli jiasli Feb 16, 2023

Choose a reason for hiding this comment

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

Better to change the name if its type changes, in order to make the code easier to read.

Copy link
Contributor Author

@zhoxing-ms zhoxing-ms Feb 16, 2023

Choose a reason for hiding this comment

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

Use instance_ids as variable name just to keep consistent with the generally existing code style, such as

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

Lines 3541 to 3558 in 917fba1

def deallocate_vmss(cmd, resource_group_name, vm_scale_set_name, instance_ids=None, no_wait=False):
client = _compute_client_factory(cmd.cli_ctx)
if instance_ids and len(instance_ids) == 1:
return sdk_no_wait(no_wait, client.virtual_machine_scale_set_vms.begin_deallocate,
resource_group_name, vm_scale_set_name, instance_ids[0])
VirtualMachineScaleSetVMInstanceIDs = cmd.get_models('VirtualMachineScaleSetVMInstanceIDs')
vm_instance_i_ds = VirtualMachineScaleSetVMInstanceIDs(instance_ids=instance_ids)
return sdk_no_wait(no_wait, client.virtual_machine_scale_sets.begin_deallocate,
resource_group_name, vm_scale_set_name, vm_instance_i_ds)
def delete_vmss_instances(cmd, resource_group_name, vm_scale_set_name, instance_ids, no_wait=False):
client = _compute_client_factory(cmd.cli_ctx)
VirtualMachineScaleSetVMInstanceRequiredIDs = cmd.get_models('VirtualMachineScaleSetVMInstanceRequiredIDs')
instance_ids = VirtualMachineScaleSetVMInstanceRequiredIDs(instance_ids=instance_ids)
return sdk_no_wait(no_wait, client.virtual_machine_scale_sets.begin_delete_instances,
resource_group_name, vm_scale_set_name, instance_ids)

Copy link
Member

@jiasli jiasli Feb 21, 2023

Choose a reason for hiding this comment

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

Ugh, vm_instance_i_ds is certainly ugly.

return sdk_no_wait(no_wait, client.virtual_machine_scale_sets.begin_reimage_all, resource_group_name,
vm_scale_set_name, instance_ids)

return sdk_no_wait(no_wait, client.virtual_machine_scale_sets.begin_reimage, resource_group_name, vm_scale_set_name)


Expand Down
Loading