hashicorp / hashicorp/packer

Windows-Restart Provisioner Should Support Custom Validation

Open
#12,921 0 comments 4 reactions 0 assignees View on GitHub
enhancement provisioner/windows-restart
Dominant language
Go
Stars
15.8k
Forks
3.3k
PR merge metrics
PR metrics pending

Description

#### Community Note

* Please vote on this issue by adding a 👍 [reaction](https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to the original issue to help the community and maintainers prioritize this request
* Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
* If you are interested in working on this issue or have submitted a pull request, please leave a comment

#### Description

The windows-restart provisioner should support running a custom script to confirm the VM has finished restarting, and that the process that required this restart in the first place has completed its tasks.

This could/should work together with the `check_registry` parameter to provide a much better user experience for long-running Windows builds.

This appears to have been partially implemented via the `restart_check_command` parameter, but that parameter's current behavior makes it trip right at the finish line. This may be a bug, or it might be the intended behavior. See "About `restart_check_command`" in the Additional Details section at the bottom.

I know similar requests in the past have been met with a statement to use AutoUnattend / Sysprep... but I feel I've identified enough use cases and a simple enough implementation mechanism to warrant another review. This is especially true considering how bad the user experience is when trying to orchestrate complex builds with sysprep run_once tasks.

#### Use Case(s)

* Wait for installation scripts to complete across multiple restarts, such as:
* Windows updates management
* Testing OS upgrades, such as from Windows 10 XXXX -> Windows 10 YYYY
* Installing agents that deploy filter drivers (Horizon View, AV/AM products, FSLogix, etc)
* Installing applications that don't respect "IgnoreRestart" installation arguments
* Support using configuration managers, such as DSC, without needing to add direct support for the tool itself
* Show progress output to the build console during long-running builds
* This is a borderline **requirement** for creating Windows builds in an enterprise environment

#### Potential configuration
**NOTE** I'm not a GO developer, I may be completely wrong about the changes that need to be implemented.

If updating the `restart_check_command` parameter's behavior is acceptable, this seems fairly simple:

The provisioner requires no changes to configuration:
```
provisioner "windows-restart" {
check_registry = true
restart_check_command = "powershell -file \"C:\\packer\\Block-PackerBuild.ps1\""
registry_keys = [
"HKLM:\\Software\\packer\script-in-progress"
]
}
```

Changes to the provisioner's wait loop:
```go
if runCustomRestartCheck {
// run user-configured restart check
err := cmdRestartCheck.RunWithUi(ctx, p.comm, p.ui)
if err != nil {
log.Printf("Communication connection err: %s", err)
continue
}
log.Printf("Connected to machine")

// Don't disable the restart check users specifically ask to run
// runCustomRestartCheck = false
}
```

Alternative options are below that preserve the original behavior of the provisioner for backwards-compatibility. Note that I don't know how to update the parameters or configs, so I can't provide examples for those changes.

##### A. Add a boolean param `always_rerun_restart_check`
This option would just cause the provisioner to re-run the restart check on each run of the loop.

The user of the provisioner could then author their own script to evaluate the state of the VM in a wait loop, and then continue the build after the condition is met.

Configuring the provisioner:
```
provisioner "windows-restart" {
always_rerun_restart_check = true
check_registry = true
restart_check_command = "powershell -file \"C:\\packer\\Block-PackerBuild.ps1\""
registry_keys = [
"HKLM:\\Software\\packer\\script-in-progress"
]
}
```

Changes to the provisioner's wait loop:
```go
if runCustomRestartCheck {
// run user-configured restart check
err := cmdRestartCheck.RunWithUi(ctx, p.comm, p.ui)
if err != nil {
log.Printf("Communication connection err: %s", err)
continue
}
log.Printf("Connected to machine")

// Previous code, always disables the custom restart check
// runCustomRestartCheck = false

// My update - only disable the custom restart check
// if always_run_restart_check is not set to true
if !p.config.always_run_restart_check {
runCustomRestartCheck = false
}
}
```

##### B. Evaluate the return code from the `restart_check_command` parameter as part of the success criteria
This option would cause the provisioner to check the result of the custom restart check script to determine success.

The user of the provisioner could then author a script that exits with non-zero exit codes if it detects the build isn't ready to continue.

Configuring the provisioner:
```
provisioner "windows-restart" {
check_registry = true
restart_check_command = "powershell -file \"C:\\packer\\Block-PackerBuild.ps1\""
registry_keys = [
"HKLM:\\Software\\packer\\script-in-progress"
]
}
```

Changes to the provisioner's wait loop:
```go
if runCustomRestartCheck {
// run user-configured restart check
err := cmdRestartCheck.RunWithUi(ctx, p.comm, p.ui)
if err != nil {
log.Printf("Communication connection err: %s", err)
continue
}
log.Printf("Connected to machine")

// My update - restart the loop if the script exits without an exit code of 0
if(!cmdRestartCheck.ExitStatus() == 0) {
// We failed, continue the loop until it returns 0
log.Printf("Exited with non-zero code: %s", cmdRestartCheck.ExitStatus())
continue
}
}
```

##### C. Add a string param `registry_check_status_script`

This option would define a script that should run every time the registry key check detects the keys defined in the `registry_keys` parameter.

The user could then author a script that outputs the current status to the log.

Configuring the provisioner:
```
provisioner "windows-restart" {
check_registry = true
registry_check_status_script = "C:\\packer\\Show-BuildProgress.ps1"
registry_keys = [
"HKLM:\\Software\\packer\\script-in-progress"
]
}
```

```go
if p.config.CheckKey {
// Run the specified status check script if we're supposed to check the registry
if !p.config.RegistryCheckStatusScript == "" {
RegistryStatusCheckCommand := winrm.Powershell(p.config.RegistryCheckStatusScript)
cmdRegistryStatusCheck := &packersdk.RemoteCmd{Command: RegistryStatusCheckCommand}
log.Printf("Invoking status check script %s", p.config.RegistryCheckStatusScript)

// Invoke the script here
err := cmdRegistryStatusCheck.RunWithUi(ctx, p.comm, p.ui)
if err != nil {
log.Printf("Status check script errored: %s", err)
continue
}
}

// continue to checking individual registry keys
}
```

#### Potential References
This is the file I think that needs to be changed:
[provisioner.go](https://github.com/hashicorp/packer/blob/main/provisioner/windows-restart/provisioner.go)

#### Additional Details
##### Workflow and Sample Block-PackerBuild script
In more detail, the workflow this would allow is as follows:

1. Powershell/etc provisioner invokes a long-running script that:
1. Sets a registry key to block the `windows-restart` provisioner from continuing
2. Registers itself to re-run on startup
3. Installs software, then initiates 0 or more restarts as needed
4. Unregisters startup task and deletes blocking registry key when all operations are done

2. Windows-Restart provisioner then:
1. Runs custom check to wait for the startup script to complete
2. Re-runs custom check script if the VM restarts and the registry values are still present
3. Only continues when the custom check completes AND the configured registry values are no longer present

Here's an example of a script that could be used in conjunction with the feature:
```powershell
# Block-PackerBuild.ps1 - registered to run at startup or invoked by the windows-restart provisioner as a status check

# Add the registry key to block packer from continuing
New-Item -ItemType Directory -Path "HKLM:\Software\packer\script-in-progress" -Force

# Wait until the DSC agent is compliant
do
{
$LCMState = (Get-DSCLocalConfigurationManager -ErrorAction SilentlyContinue).LCMState
Write-Host "Waiting for lcm state to be idle: [ $LCMState ]"
Start-Sleep -Seconds 5
}
while($LCMState -ne 'Idle')

# Remove the registry key and exit to allow the provisioner to continue
Remove-Item -Path "HKLM:\Software\packer\script-in-progress" -Force
```

##### The Problems With `restart_check_command`
In theory, this should be supported by the `restart_check_command` parameter.
However, there are several major problems with the parameter's current setup:

1. There is no method of signaling a failure to the provisioner using this parameter
1. All output from the script is ignored, including stderr, stdout, Write-Output, Write-Host, throwing terminating exceptions, etc
2. Exit/return codes are similarly ignored
2. The custom check is only run until the first time it succeeds
1. After first success, the provisioner only runs the "default" check on future iterations of the loop
3. The default check used afterwards outputs a message to the log every 5 - 10 seconds "`$vm-name` restarted.", which
1. Provides no useful information to the user
2. Clutters up the log, making troubleshooting more difficult
3. Misleads the user, implying that the VM is being repeatedly restarted every few seconds

##### Regarding The AutoUnattend/Sysprep Option
While I understand the official stance from others in the past has been to use sysprep / autounattend to run workflows like this prior to enabling WinRM connections, this has several issues:

1. Not all workflows are suitable for use during an auto-unattend deployment
2. No status is given to the user during the unattended setup phase, making for a terrible user experience if the process takes more than 5 - 10 minutes
3. Unattended installations are often difficult to configure and troubleshoot, especially when compared to WinRM-based provisioning steps
4. Provisioning steps that require credentials would require embedding application secrets in the sysprep XML

Contributor guide

Open the contributing guide

Research direction

Start with provisioner/windows-restart/provisioner.go and inspect the wait loop around restart_check_command and check_registry. Compare the proposed validation behaviors, select the intended configuration and success semantics, and verify that the provisioner can report progress, rerun validation across restarts, and continue only when the custom check and registry conditions are satisfied.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
devops
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.