xWindowsUpdateAgent: Added Retry Logic for Known Transient Errors (#24)

[pyrostew]

Pull Request (PR) description

Within the Get-WuaWrapper which was already handling some errors, added logic to perform retries on some of the errors that are likely to be transient.

This Pull Request (PR) fixes the following issues

Fixes #24

Task list

• Added an entry under the Unreleased section of the change log in the README.md.
  Entry should say what was changed, and how that affects users (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 Resource Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[codecov]

Codecov Report

Merging #92 into dev will increase coverage by 1%.
The diff coverage is 91%.

@@        Coverage Diff        @@
##           dev   #92   +/-   ##
=================================
+ Coverage   62%   63%   +1%     
=================================
  Files        2     2           
  Lines      307   338   +31     
=================================
+ Hits       191   215   +24     
- Misses     116   123    +7

[pyrostew]

@TravisEz13 As requested, here is the PR!

[TravisEz13]

Not blocking, but it might be good to refactor this into a function that takes an structure of the errorcode, if it's retryable and warning and iterates over them and does the right thing.

It would reduce code duplication and hopefully make the code easier to read.

[pyrostew]

I've made this change you suggested.

[TravisEz13]

I don't see anything blocking.

I recommend submitting a PR to update formatting first. It makes things much easier to review.

[codecov-io]

Codecov Report

Merging #92 into dev will increase coverage by 1%.
The diff coverage is 92%.

@@        Coverage Diff        @@
##           dev   #92   +/-   ##
=================================
+ Coverage   62%   64%   +1%     
=================================
  Files        2     2           
  Lines      307   342   +35     
=================================
+ Hits       191   219   +28     
- Misses     116   123    +7     

[pyrostew]

While I don't disagree that having a separate PR for the formatting changes would be sensible, at this point I would have to faff to separate the changes to do that. I will bare it in mind for future changes however.
Who needs prodding to merge this PR and get a Nuget package released?

[TravisEz13]

I'm not a maintainer in the repo anymore....

[TravisEz13]

cc @johlju

[johlju]

@pyrostew a few comments. This repo must be converted to the continuous delivery pipeline for us to be able to release a new version (see https://dsccommunity.org/blog/convert-a-module-for-continuous-delivery/). While you look at the review comments I can help convert this repo, but it will take me a week or so due to bandwidth.

Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @pyrostew and @TravisEz13)

---

DscResources/MSFT_xWindowsUpdateAgent/MSFT_xWindowsUpdateAgent.psm1, line 128 at r2 (raw file):

function Check-Retry

We are not testing if this function actually retries? 🤔

---

DscResources/MSFT_xWindowsUpdateAgent/MSFT_xWindowsUpdateAgent.psm1, line 240 at r2 (raw file):

Quoted 4 lines of code…

            if(-not (Check-Retry $errorObj))
            {
                return $ExceptionReturnValue
            }

Not sure we actually unti test this function so that it is actually retrying?

---

DscResources/MSFT_xWindowsUpdateAgent/MSFT_xWindowsUpdateAgent.psm1, line 723 at r2 (raw file):

Quoted 8 lines of code…

    if ($RetryAttempts -ge 0)
    {
        $script:retryAttempts = $RetryAttempts
    }
    if ($RetryDelay -ge 0)
    {
        $script:retryDelay = $RetryAttempts
    }

Not seeing this code is needed since it does not call any function that uses them from this function? 🤔

[johlju]

@pyrostew btw thank you for fixing style issues (formatting). There were still issues, but did not comment on that this time since whole repo still need fixing. 🙂

[pyrostew]

Right sorry the delay in replying.
@johlju I can't seem to login to reviewable to respond directly to your comments, so I will respond to them here.

Relating to the first 2 comments about not testing, I have added a some test code in the first revision between lines 1395 and 1420, that I think suitably tests the retry logic, it certainly helped with creating the logic in revision 1 then refactoring it for revision 2.

Your 3rd comment is quite right, there is no need for that code in the Test-TargetResource, I will remove that and push a new revision.

[johlju]

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @TravisEz13)

[johlju]

@pyrostew thank you for this contribution!

[johlju]

I took longer than expected to convert another repo, but finished with that so I will look into converting this one next, unless someone beat me to it.