Skip to content

MORPH-9106: Deploy instances to host selected by user#147

Open
trevor-jordy wants to merge 3 commits into
mainfrom
host-selection-fix
Open

MORPH-9106: Deploy instances to host selected by user#147
trevor-jordy wants to merge 3 commits into
mainfrom
host-selection-fix

Conversation

@trevor-jordy

Copy link
Copy Markdown
Contributor

Fixes issue where the user's selected vmhost is ignored by the provisioning process. Now if a host is selected, the instance will be provisioned to said host, and if one is not, it will fall back to the SCVMM's decision.

@trevor-jordy trevor-jordy marked this pull request as ready for review June 15, 2026 21:00
if (hostExternalId) {
// Host explicitly selected — use VMHost parameter set regardless of cloud scope
commands << "\$vmHost = Get-SCVMHost -ID \"$hostExternalId\""
commands << "\$ignore = Set-SCVMConfiguration -VMConfiguration \$virtualMachineConfiguration -VMHost \$vmHost"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is -CapabilityProfile $CapabilityProfile on line 2587 but not here. Intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This line is instead using VMHost to explicitly provision to a host.

commands << "\$vmHost = Get-SCVMHost -ID \"$hostExternalId\""
commands << "\$ignore = Set-SCVMConfiguration -VMConfiguration \$virtualMachineConfiguration -VMHost \$vmHost"
commands << "\$ignore = Update-SCVMConfiguration -VMConfiguration \$virtualMachineConfiguration"
def newVMString = "\$createdVm = New-SCVirtualMachine -Name \"$vmId\" -VMConfiguration \$virtualMachineConfiguration ${isSysprep ? "-AnswerFile \$AnswerFile" : ""} ${isTemplate ? "-HardwareProfile \$HardwareProfile" : ""} -JobGroup \"$diskJobGuid\" -StartAction \"TurnOnVMIfRunningWhenVSStopped\" -RunAsynchronously -StopAction \"SaveVM\""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seeing conditional use of isTemplate here but not line 2589. What is the reason?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is an existing behavior that I did not touch. Looking into it, the "host" path if it is using an existing template there is no earlier step to bind the hardwareprofile to the vm config. If it is creating a newtemplate, the hardwareprofile is added during that creation. I don't see a reason why this was done only for host path, as just always passing hardwareprofile like the cloud path does should be fine. I can get rid of this to make it cleaner, but I'd be removing it without being sure why this was done originally

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's a valid response that you want to keep existing logic to minimize risk. I would just like to understand why the if/else ordering switch fixes the issue (see other comment). Thank you.

@NimbleMike

Copy link
Copy Markdown
Contributor

Both the previous code, as well as the embedded SCVMM plugin, did this:

            if (deployingToCloud) {
            } else {
                if(hostExternalId) {
                } else {
                }
            }

Your code changes the ordering to:

           if (hostExternalId) {
            } else if (deployingToCloud) {
            } else {
            }

The deployToCloud is defined as:

def deployingToCloud = opts.zone.regionCode ? true : false

Couple of questions:

  • How does deployToCloud become true? Is it a user input setting?
  • Why does switching the ordering resolve the issue?
  • Is this defect present in the embedded SCVMM as well? It looks like it would.

Just looking for more context to better understand the fix. Thank you.

@trevor-jordy

Copy link
Copy Markdown
Contributor Author

Both the previous code, as well as the embedded SCVMM plugin, did this:

            if (deployingToCloud) {
            } else {
                if(hostExternalId) {
                } else {
                }
            }

Your code changes the ordering to:

           if (hostExternalId) {
            } else if (deployingToCloud) {
            } else {
            }

The deployToCloud is defined as:

def deployingToCloud = opts.zone.regionCode ? true : false

Couple of questions:

  • How does deployToCloud become true? Is it a user input setting?
  • Why does switching the ordering resolve the issue?
  • Is this defect present in the embedded SCVMM as well? It looks like it would.

Just looking for more context to better understand the fix. Thank you.

deployToCloud becomes true whenever a cloud is selected during provisioning. Since a cloud is automatically selected on the provisioning menus, this means with the original code, your selected host would be ignored if you are provisioning to a cloud. If this is intentional, I'm not sure why, because it is confusing to be allowed to select a host only for it to be ignored.

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.

2 participants