ComputerManagementDsc icon indicating copy to clipboard operation
ComputerManagementDsc copied to clipboard

Computer: add Restart parameter when joining a computer to a domain.

Open jlongo62 opened this issue 5 years ago • 4 comments

ISSUE TITLE: Computer: add Restart parameter when joining a computer to a domain.

ISSUE DESCRIPTION: Computer does not support the Restart parameter of the underlying Add-Computer PS command. Add Restart parameter implemented as documented in Add-Computer.

jlongo62 avatar Aug 07 '19 03:08 jlongo62

Hi @jlongo62 - generally, we don't want resources to restart the node during the application of a configuration. Instead they would typically set one of the registry keys that Windows uses to indicate that a reboot is required. You'd then add the xPendingReboot resource where you wanted a restart to occur which causes the config to stop when it reaches that point, if a reboot is required. The LCM would then take care of the restart.

Is this what you're after? If so, then what we probably need to do is check if the Add-Computer correctly sets one of the flags indicating a reboot is required. If it isn't then we'll need to fix that.

PlagueHO avatar Aug 07 '19 06:08 PlagueHO

The xPendingReboot you have described sounds useful, however (for me conceptually) DSC is just a wrapper around Powershell . When I determine the PS needed, I find a DSC that does the PS. Therefore having support for the Restart parameter would be desirable.

jlongo62 avatar Aug 07 '19 15:08 jlongo62

Hi @jlongo62 - sure, I have a similar approach when doing this myself. But the best practice for resources is that they don't restart the machine themselves - they ask the LCM to do it on their behalf. This allows the LCM to do it gracefully.

If the resource itself issues a restart without asking the LCM then the LCM will not have an opportunity to clean up temporary files and log information to Pull Servers etc about the result of the run - in fact it will appear the run never completed at all.

This might be an acceptable trade-off for many users (yourself included), but the risk would be that other users might not realize about this trade-off and use the function, thinking it is a best practice.

tl;dr; adding the Restart parameter would go against the best practice of not allowing resources to restart the machine themselves - instead requesting the LCM does it by calling xPendingReboot.

This is the recommended approach for what you're wanting to do:

        Computer JoinDomain
        {
            Name       = 'Server01'
            DomainName = 'Contoso'
            Credential = $Credential
        }

       xPendingReboot RebootAfterDomainJoin
       {
            Name = 'RebootAfterDomainJoin'
            DependsOn = '[Computer]JoinDomain
       }

FWIW, xPendingReboot is going to be moved across to become part of this module shortly (most likely before next release). So this will simplify things by reducing the need to include the xPendingReboot module, instead just doing:

        Computer JoinDomain
        {
            Name       = 'Server01'
            DomainName = 'Contoso'
            Credential = $Credential
        }

       PendingReboot RebootAfterDomainJoin
       {
            Name = 'RebootAfterDomainJoin'
            DependsOn = '[Computer]JoinDomain
       }

PlagueHO avatar Aug 08 '19 07:08 PlagueHO

I am using this from Azure DevOps for deploymnet of VM's. Does supporting a reboot in this manner will maintain control of the flow from the VSTS build side? i.e. I deploy the PendingReboot extension to reboot, but the Build Server waits for the VM to be back up before deploying the next extension ?

jlongo62 avatar Aug 08 '19 14:08 jlongo62