Skip to content

Conversation

@qwordy
Copy link
Member

@qwordy qwordy commented Jul 21, 2020

Description

Resolve #14056

  1. New parameter --automatic-placement in vm host group create
    --automatic-placement            : Specify whether virtual machines or virtual machine scale
                                       sets can be placed automatically on the dedicated host group.
                                       Automatic placement means resources are allocated on
                                       dedicated hosts, that are chosen by Azure, under the
                                       dedicated host group. The value is defaulted to true when not
                                       provided.  Allowed values: false, true.
  1. New parameter --host-group in vmss create
    --host-group                        : Name or ID of dedicated host group that the virtual
                                          machine scale set resides in.
  1. New command vm host group get-instance-view

I will make a breaking change. These are parameters from vm create. However, --host-group is not a parameter in REST API, it is used to complete the info of host if user only tells host name. Now, in the new API version, host group is supported in service.
I plan to abandon old --host-group, let --host only support ID, and the new --host-group represents the dedicated host group that the virtual machine resides in.

--host

Name or ID of the dedicated host this VM will reside in. If a name is specified, a host group must be specified via --host-group.

--host-group

Name of the dedicated host group containing the dedicated host this VM will reside in.

New spec:

        "host": {
          "$ref": "#/definitions/SubResource",
          "description": "Specifies information about the dedicated host that the virtual machine resides in. <br><br>Minimum api-version: 2018-10-01."
        },
        "hostGroup": {
          "$ref": "#/definitions/SubResource",
          "description": "Specifies information about the dedicated host group that the virtual machine resides in. <br><br>Minimum api-version: 2020-06-01. <br><br>NOTE: User cannot specify both host and hostGroup properties."
        },

@yungezz @arrownj What do you think about this change? Do you agree?

In the past 30 days, number of invocations:
vm create, 474384
vm create --host-group, 39
vm create --host, 46

"supportAutomaticPlacement" is a new property introduced under DedicatedHostGroup.properties. Customers can specify either true or false. It defaults to true, if no input is provided. If a dedicated host group has automatic placement enabled, VMs or VMScaleSets can be placed on the dedicated host group using automatic placement.
GET dedicated host group now supports instance view query parameter. Dedicated host group instance view will return instance view of the dedicated hosts under the dedicated host group.
"hostGroup" is a new property introduced under VM.properties and VMScaleSet.properties. The input should be the resource id of the dedicated host group, on which the customer wants his VM/VMScaleSet placed using automatic placement.
"assignedHost" is a new property introduced under VMInstanceView and VMScaleSetVMInstanceView. It is the resource id of the dedicated host, on which the queried VM/VMScaleSetVM is placed using atuomatic placement.

Testing Guide

az vm host group create -n hg -c 1 -g fy --automatic-placement -l centraluseuap
az vm host create -n h --host-group hg -d 0 -g fy --sku DSv3-Type1 -l centraluseuap
az vm create -g fy -n vm1 --image centos --size Standard_D4s_v3 --host <host_id> -l centraluseuap
az vm create -g fy -n vm2 --image centos --size Standard_D4s_v3 --host-group hg -l centraluseuap

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.

@qwordy qwordy requested a review from arrownj as a code owner July 21, 2020 05:55
@qwordy qwordy requested a review from yungezz July 22, 2020 03:33
@yungezz yungezz self-assigned this Jul 22, 2020
@yungezz yungezz added the Compute az vm/vmss/image/disk/snapshot label Jul 22, 2020
@Azure Azure deleted a comment from yonzhan Jul 22, 2020
Copy link
Contributor

@arrownj arrownj left a comment

Choose a reason for hiding this comment

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

Code is good. Seems no test for --host-group. And why this is a breaking change ? All the new parameters are optional.

self.assertTrue(instance_view["assetId"])
self.assertTrue(instance_view["availableCapacity"])

self.cmd('vm host group get-instance-view -g {rg} -n {host-group}')
Copy link
Contributor

Choose a reason for hiding this comment

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

add check ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Testing is not finished yet

@yungezz
Copy link
Member

yungezz commented Jul 22, 2020

thanks @qwordy , excellent PR description to detail problems and changes!

after learning host, host group and service change, I propose below change. It's not necessary to be a breaking change, since meaning of host-group param not change: it means the host group this VM would like to go into. Pls have a look and check does it make sense to you, we can have discussion offline also.

My proposal:

  • keep host, host-group params.
  • change the logic to below
    • if --host is name
      -- host-group is name, then it's existing usge: host-group+ host is used to specific host
      -- host-group is id, throw error.

    • if --host is id
      -- host-group is name, throw error. host is enough to specific host, no need of host-group.
      -- host-group is id, throw error. same as above

    • if --host is None and --automatic-placement true
      -- host-group is name, it will be VM.host
      -- host-group is id, it will be VM.host

    • if --host is None and --automatic-placement false
      -- host-group is name, error
      -- host-group is id, throw error.

@qwordy qwordy marked this pull request as draft July 22, 2020 10:52
@qwordy
Copy link
Member Author

qwordy commented Jul 22, 2020

Very clear! --host-group has multiple meanings now. Will it be a burden for user to understand? I need to think about how to write help for --host-group.

@yungezz
Copy link
Member

yungezz commented Jul 22, 2020

Very clear! --host-group has multiple meanings now. Will it be a burden for user to understand? I need to think about how to write help for --host-group.

valid concern :) let's work together to see any better way

@qwordy qwordy marked this pull request as ready for review July 30, 2020 04:46
@qwordy qwordy merged commit b3d5d96 into Azure:dev Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compute az vm/vmss/image/disk/snapshot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changes related to dedicated host group automatic placement

3 participants