Skip to content
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

azurerm_orchestrated_virtual_machine_scale_set - add sku_profile to support instance mix feature #27599

Merged
merged 3 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,35 @@ func resourceOrchestratedVirtualMachineScaleSet() *pluginsdk.Resource {
ValidateFunc: computeValidate.OrchestratedVirtualMachineScaleSetSku,
},

"sku_profile": {
Type: pluginsdk.TypeList,
Optional: true,
ForceNew: true,
Copy link
Member

Choose a reason for hiding this comment

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

Given this is ForceNew what is the default value here? Are we going to trigger replacements for existing resources?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sku_profile can be set only when sku_name="Mix" and it cannot be added/removed through update. However, its sub-properties allocation_strategy and vm_sizes can be changed through Update.

MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"allocation_strategy": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice(
virtualmachinescalesets.PossibleValuesForAllocationStrategy(),
false,
),
},

"vm_sizes": {
Type: pluginsdk.TypeSet,
Required: true,
MinItems: 1,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
},
},
},
},
},

"os_profile": OrchestratedVirtualMachineScaleSetOSProfileSchema(),

// Optional
Expand Down Expand Up @@ -351,6 +380,14 @@ func resourceOrchestratedVirtualMachineScaleSetCreate(d *pluginsdk.ResourceData,
props.Sku = sku
}

if v, ok := d.GetOk("sku_profile"); ok {
if props.Sku == nil || pointer.From(props.Sku.Name) != "Mix" {
Copy link
Member

Choose a reason for hiding this comment

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

Could this be implemented as a customiseDiff to catch this configuration issue at plan time rather than erroring out here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved the logic into customiseDiff . Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Also, can we have this ("Mix") defined as a const rather than using magic string everywhere?
maybe:

Suggested change
if props.Sku == nil || pointer.From(props.Sku.Name) != "Mix" {
if props.Sku == nil || pointer.From(props.Sku.Name) != skuNameMix {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

defined it in a const variable. Thanks.

return fmt.Errorf("`sku_profile` can only be set when `sku_name` is set to `Mix`")
}

props.Properties.SkuProfile = expandOrchestratedVirtualMachineScaleSetSkuProfile(v.([]interface{}))
}

if v, ok := d.GetOk("capacity_reservation_group_id"); ok {
if d.Get("single_placement_group").(bool) {
return fmt.Errorf("`single_placement_group` must be set to `false` when `capacity_reservation_group_id` is specified")
Expand Down Expand Up @@ -732,6 +769,11 @@ func resourceOrchestratedVirtualMachineScaleSetUpdate(d *pluginsdk.ResourceData,
}
}

if d.HasChange("sku_profile") {
Copy link
Member

Choose a reason for hiding this comment

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

This block is ForceNew in the schema, why are we checking for updates?

Copy link
Contributor Author

@ms-zhenhua ms-zhenhua Oct 28, 2024

Choose a reason for hiding this comment

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

answered in the above comment. Though it's defined ForceNew, its sub-properties can be changed through Update as long as sku_profile is not removed.

updateInstances = true
updateProps.SkuProfile = expandOrchestratedVirtualMachineScaleSetSkuProfile(d.Get("sku_profile").([]interface{}))
}

if d.HasChange("max_bid_price") {
if priority != virtualmachinescalesets.VirtualMachinePriorityTypesSpot {
return fmt.Errorf("`max_bid_price` can only be configured when `priority` is set to `Spot`")
Expand Down Expand Up @@ -1194,6 +1236,11 @@ func resourceOrchestratedVirtualMachineScaleSetRead(d *pluginsdk.ResourceData, m
if props.SinglePlacementGroup != nil {
d.Set("single_placement_group", props.SinglePlacementGroup)
}

if err := d.Set("sku_profile", flattenOrchestratedVirtualMachineScaleSetSkuProfile(props.SkuProfile)); err != nil {
return fmt.Errorf("setting `sku_profile`: %+v", err)
}

d.Set("unique_id", props.UniqueId)
d.Set("zone_balance", props.ZoneBalance)

Expand Down Expand Up @@ -1361,17 +1408,60 @@ func resourceOrchestratedVirtualMachineScaleSetDelete(d *pluginsdk.ResourceData,
return nil
}

func expandOrchestratedVirtualMachineScaleSetSkuProfile(input []interface{}) *virtualmachinescalesets.SkuProfile {
if len(input) == 0 || input[0] == nil {
return nil
}

v := input[0].(map[string]interface{})
vmSizesRaw := v["vm_sizes"].(*pluginsdk.Set).List()
vmSizes := make([]virtualmachinescalesets.SkuProfileVMSize, 0)
for _, vmSize := range vmSizesRaw {
vmSizes = append(vmSizes, virtualmachinescalesets.SkuProfileVMSize{
Name: pointer.To(vmSize.(string)),
})
}

return &virtualmachinescalesets.SkuProfile{
AllocationStrategy: pointer.To((virtualmachinescalesets.AllocationStrategy)(v["allocation_strategy"].(string))),
VMSizes: pointer.To(vmSizes),
}
}

func flattenOrchestratedVirtualMachineScaleSetSkuProfile(input *virtualmachinescalesets.SkuProfile) []interface{} {
if input == nil {
return make([]interface{}, 0)
}

vmSizes := make([]string, 0)
if input.VMSizes != nil {
for _, vmSize := range *input.VMSizes {
vmSizes = append(vmSizes, *vmSize.Name)
}
}

return []interface{}{
map[string]interface{}{
"allocation_strategy": string(pointer.From(input.AllocationStrategy)),
"vm_sizes": vmSizes,
},
}
}

func expandOrchestratedVirtualMachineScaleSetSku(input string, capacity int) (*virtualmachinescalesets.Sku, error) {
skuParts := strings.Split(input, "_")

if len(skuParts) < 2 || strings.Contains(input, "__") || strings.Contains(input, " ") {
if (input != "Mix" && len(skuParts) < 2) || strings.Contains(input, "__") || strings.Contains(input, " ") {
return nil, fmt.Errorf("'sku_name'(%q) is not formatted properly", input)
}

sku := &virtualmachinescalesets.Sku{
Name: pointer.To(input),
Capacity: utils.Int64(int64(capacity)),
Tier: pointer.To("Standard"),
}

if input != "Mix" {
sku.Tier = pointer.To("Standard")
}

return sku, nil
Expand All @@ -1380,7 +1470,7 @@ func expandOrchestratedVirtualMachineScaleSetSku(input string, capacity int) (*v
func flattenOrchestratedVirtualMachineScaleSetSku(input *virtualmachinescalesets.Sku) (*string, error) {
var skuName string
if input != nil && input.Name != nil {
if strings.HasPrefix(strings.ToLower(*input.Name), "standard") {
if strings.HasPrefix(strings.ToLower(*input.Name), "standard") || *input.Name == "Mix" {
skuName = *input.Name
} else {
skuName = fmt.Sprintf("Standard_%s", *input.Name)
Expand Down
Loading
Loading