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

xService - Add recovery options properties #679

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

RandomNoun7
Copy link

@RandomNoun7 RandomNoun7 commented Mar 27, 2020

Pull Request (PR) description

Implements the ability to manage service Failure Actions via the registry.

This change will add additional properties to the xService resource to specify reboot message, failure command, and all three of the Failure Actions.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@RandomNoun7
Copy link
Author

This PR is a draft at the moment to give anyone interested a chance to see where the code is going so far and a chance to comment.

Currently only Get-TargetResource and Test-TargetResource are working.

Still to do:

  • Set-TargetResource
  • Unit Testing
  • Acceptance Testing
  • Evaluate byte conversion methods to see if something cleaner can be found
  • Refactor the code into seperate files possibly to aid in readability and maintainability
  • Update MOF Files
  • Update documentation and examples

@RandomNoun7
Copy link
Author

RandomNoun7 commented Mar 31, 2020

Refactored the byte parsing to use [System.BitConverter]::ToInt32(byte[]) instead of a bunch of string conversion.

I also discovered that while the UI will only represent up to three failure actions, you can specify as many as you want if you create a service in C# or if you set the failure actions using sc.exe. I also found that if the last action in the actions array is to take no action at all, the output of sc.exe qfailure will not show you that action in the output, but it is still present and will be honored in the binary data in the registry key.

The behavior is that for whichever failure you are currently experiencing N, the action N-1 will be taken, unless that is out of bounds of the array, in wich case the last action present in the array will be selected and executed. In the case of an array with a final action of taking no action, it seems that the service will attempt corrective action N-1 times, but thereafter, for the final action, will simply give up and not bother to restart the computer, or even the service itself.

As an example this appears to be the default behavior for the WSearch service, which has 6 actions specified, 5 of which restart the service, and the sixth which does nothing at all.

To account for fact that the number of failure actions cannot be known ahead of time, I have had to implement the collection of failure actions to manage as a collection of objects called FailureActionsCollection. The idea is that the user can specify the list of failure actions as an array of hash tables with the keys type and delaySeconds, and the code in the Test-TargetResource and Set-TargetResource commands should be able to iterate over the collection and ensure each action is properly reflected in the registry key data.

@RandomNoun7 RandomNoun7 force-pushed the add-recovery-options-properties branch from 18bca55 to a91133c Compare March 31, 2020 06:12
@Outek
Copy link

Outek commented Mar 31, 2020

Actualy we are also interested in this configuration(in our case for a SQL service). If you like to have some assistance, just ping me.

@PlagueHO PlagueHO added the needs review The pull request needs a code review. label Apr 2, 2020
@RandomNoun7
Copy link
Author

The Set-Target functionality is nearly complete. However, in the course of testing I found another registry key that I need to manage.

In the Recovery tab of a services properties dialog, there is a check box labeled Enable Actions For Stops With Errors. It turns out that this check box corresponds to a registry key called FailureActionsOnNonCrashFailures. The key stores the boolean value represented by the struct documented here.

This is a very important key to manage because the value of that key makes a critical difference to the recovery behavior of a service.

By default this key does not exist. It is only created when the check box is ticked.

I will have to add a parameter for this value to all of the functions.

@RandomNoun7 RandomNoun7 force-pushed the add-recovery-options-properties branch 2 times, most recently from 04c3a55 to d5f7cf6 Compare April 7, 2020 20:50
@RandomNoun7 RandomNoun7 force-pushed the add-recovery-options-properties branch 2 times, most recently from fcac90f to f11d379 Compare April 25, 2020 14:22
@RandomNoun7 RandomNoun7 force-pushed the add-recovery-options-properties branch 2 times, most recently from a7c31e9 to c5dbd5e Compare May 6, 2020 15:14
@PlagueHO
Copy link
Member

PlagueHO commented Jul 5, 2020

Hi @RandomNoun7 - is this one still progressing? Would be great to get it in.

@RandomNoun7
Copy link
Author

@PlagueHO I would like to get started making progress again in the next week or so. Are you targeting a release date to get this in?

@PlagueHO
Copy link
Member

PlagueHO commented Jul 7, 2020

Hi @RandomNoun7 - as soon as this goes in I can release it. We've adopted a continuous delivery model so we can immediately release any significant new features or bugs. For breaking changes or high risk/impact changes we might leave the module in preview for a bit and release it a month after.

@RandomNoun7 RandomNoun7 force-pushed the add-recovery-options-properties branch from c5cf1b5 to 1ad978c Compare July 8, 2020 13:47
@RandomNoun7 RandomNoun7 force-pushed the add-recovery-options-properties branch from c798abb to d2b27db Compare July 8, 2020 15:27
@PlagueHO PlagueHO changed the base branch from master to main December 30, 2020 18:55
@Borgquite
Copy link

If the ‘todo’ file is correct, it looks like the only issue remaining here is that sometimes a ‘single item ’ array does not have the Count value.

This is a known PS 5.1 issue with a quick fix if you wrap the relevant object into an array:

Can this be implemented?

https://learn.microsoft.com/en-us/powershell/scripting/learn/deep-dives/everything-about-arrays?view=powershell-7.4

There is one more trap to watch out for here. You can use the count even if you have a single object, unless that object is a PSCustomObject. This is a bug that is fixed in PowerShell 6.1. That's good news, but a lot of people are still on 5.1 and need to watch out for it.
PowerShell

Copy
PS> $object = [PSCustomObject]@{Name='TestObject'}
PS> $object.count
$null
If you're still on PowerShell 5.1, you can wrap the object in an array before checking the count to get an accurate count.
PowerShell

Copy
if ( @($array).count -gt 0 )
{
"Array isn't empty"
}
To fully play it safe, check for $null, then check the count.
PowerShell

Copy
if ( $null -ne $array -and @($array).count -gt 0 )
{
"Array isn't empty"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review The pull request needs a code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xService Cannot Manage Failure Actions
4 participants