Update-DbaInstance - Allow callers to retain extracted update files for later cleanup #10777

Activity

1. shaded0

   added

   feature

   new commandFeature request that would need a new commandFeature request that would need a new command

   triage requiredNew issue that has not been reviewed by maintainersNew issue that has not been reviewed by maintainers

   on Oct 6, 2026
2. andreasjordan commented on Oct 11, 2026

   

   andreasjordan

   on Oct 11, 2026

   Collaborator

   More actions

   Thanks for the careful write-up and the measurements. Below is some technical input on the interface so a PR can be scoped well. Whether the parameter goes in is Chrissy's decision.

   Current code on development

   The cleanup logic has not changed since 2.7.22; only the help text has. It runs in the finally block of Invoke-DbaAdvancedUpdate, once per action, before the pending-reboot check and before Restart-Computer. With -Restart it delays the reboot. Without -Restart it delays the return to the caller, exactly as you describe.

   Related history

   #8244 (2022) reported a hang after setup had finished and before the temporary files were gone. I saw the same symptom at a customer on a two-node cluster. We closed that issue without a repro, and your numbers may explain it.

   Interface suggestions

   • Name: -KeepExtractedFiles fits the existing Keep* switches. The closest one is Start-DbaAzMigration -KeepBacpac, which works the same way: an intermediate file is removed by default, and its path is reported in an output property.
   • Forwarding: add the switch to Invoke-DbaAdvancedUpdate (it is exported) and to Update-DbaInstance, and add it to $updateSplat. That also covers two or more computers, because Invoke-Parallel runs the same script block.
   • Path, normal case: ExtractPath is already on every output object. The default view hides it, and the existing tests in Update-DbaInstance.Tests.ps1 already assert it. It only needs an entry in the .OUTPUTS help of Update-DbaInstance, which does not list it today.
   • Path with -EnableException: under -EnableException, the inner Stop-Function calls throw into the catch block, which calls Stop-Function again. Passing -Target $output to that call puts the object into the error record, so the caller's catch can read $_.TargetObject.ExtractPath. This is worth a test for a single computer. With two or more computers, Invoke-Parallel handles the errors from its runspaces, so I would not promise it there.
   • The contract I would document instead of relying on either of these: the folder is always <ExtractPath>\dbatools_KB<KB>_Extract_<32 hex characters>. With -KeepExtractedFiles, pass a dedicated -ExtractPath and, after recovery, remove the dbatools_KB*_Extract_* folders below it. This also covers runs that never return an object, for example a WinRM session lost during the reboot or an orchestrator step that was killed. Without -ExtractPath, each action picks the drive with the most free space. Kept folders use up that space, so a second update in the same run can land on a different drive.
   • Failures: with the switch, the folder stays on every path. That includes a failed extraction, which leaves a partial folder, and a failed setup. The help text should say so when it states that the caller owns the cleanup.
   • -WhatIf: Update-DbaInstance calls ShouldProcess before it hands any action to Invoke-DbaAdvancedUpdate, so nothing is extracted and nothing can be kept. One test showing that the switch does not change this is enough.

   Tests

   tests/Invoke-DbaAdvancedUpdate.Tests.ps1 mocks Invoke-Program but not the cleanup, so the real Remove-Item runs. Let the Invoke-Program mock create the extraction folder below a test-owned -ExtractPath, and the tests can check the file system directly:

   • by default the folder is gone
   • with the switch it still exists and matches ExtractPath on the output
   • the same holds for the failing-setup case and for a run with two actions

   Heads-up

   My PR #10773 (approved, not merged yet) wraps the body of Invoke-DbaAdvancedUpdate in an outer try/finally to complete the progress bar, which re-indents the whole function. The two changes do not overlap in substance, but a PR based on today's development will need a rebase once #10773 is in.

   A focused PR along these lines sounds good to me.

   ---

   This text was created by Claude and reviewed by Andreas Jordan.
3. shaded0 commented on Oct 12, 2026

   

   shaded0

   on Oct 12, 2026

   Last edited by shaded0

   Author

   More actions

   Thanks for the detailed feedback, Andreas! I opened #10784, scoped to your suggestions and to what I had already tested in my environment.

   It adds -KeepExtractedFiles to both commands, passes -Target $output so the result is reachable under -EnableException for a single computer, and documents the folder naming contract. Tests cover the default cleanup, retention, a failed setup, -EnableException and -WhatIf.

   If anything needs changing, please let me know!

   • John Wild (jwild91@gmail.com)