SqlServiceAccount: Updated Get-ServiceObject to find SSIS service

[twerthi]

Pull Request (PR) description

Get-ServiceObject calls the Get-SqlServiceName function which searches the registry for the specified service type. For SSIS, it correctly finds the service and returns MsDtsServer. Get-ServiceObject then searches the $managedComputer.Services collection comparing the Name property with the returned value and fails as the collection contains MsDtsServer130. Changed the operator from -eq to -like with an * after $serviceNameFilter. This should still function correctly with instances as well.

This Pull Request (PR) fixes the following issues

• Fixes SqlServerLogin: fails test for Disabled sql logins everytime #915

Task list

• Added an entry under the Unreleased section of the change log in the CHANGELOG.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-io]

Codecov Report

Merging #1246 into dev will increase coverage by <1%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##            dev   #1246    +/-   ##
=====================================
+ Coverage    98%     98%   +<1%     
=====================================
  Files        34      34            
  Lines      4171    4197    +26     
=====================================
+ Hits       4111    4137    +26     
  Misses       60      60

[twerthi]

What is the protocol when the build fails, but it's not related to a change I made? Am I to fix it?

[twerthi]

Please let me know if there are any outstanding tasks I need to do.

[johlju]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @twerthi)

a discussion (no related file):
Please add a unit test that test this new functionality(ies) in SqlServiceAccount.

---

a discussion (no related file):
Please also add a separate unit test that tests the new helper function. See other comment about making it a generic helper function (so the unit test should be added to the correct .Tests.ps1 file).

---

---

CHANGELOG.md, line 58 at r2 (raw file):

    unit test ([issue #983](https://github.com/PowerShell/SqlServerDsc/issues/983)).
- Changes to SqlServiceAccount
  - Fixed Get-ServiceObject when searching for IntegrationServices service

Could we end the sentence with a full stop (.)?

---

CHANGELOG.md, line 60 at r2 (raw file):

  - Fixed Get-ServiceObject when searching for IntegrationServices service
- Changes to SqlServiceAccount
  - Added code to allow for using Managed Service Accounts

Could we end the sentence with a full stop (.)?

---

DSCResources/MSFT_SqlServiceAccount/MSFT_SqlServiceAccount.psm1, line 287 at r2 (raw file):

$_.Name -like "$serviceNameFilter*"

I think this will return multiple objects if there are two instances that are named almost the same, i.e. MSSQL$TEST1, MSSQL$TEST11?

---

DSCResources/MSFT_SqlServiceAccount/MSFT_SqlServiceAccount.psm1, line 456 at r2 (raw file):

        Type of service account.
#>
function Get-ServiceAccount

This helper function have similarities to the Get-ServiceAccountParameters helper function. Could we possibly make this a generic helper function instead of duplicating the regex logic? 🤔
https://github.com/PowerShell/SqlServerDsc/blob/3714325ef731719da3e7e949ebaa278588e05ab9/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1#L2237-L2290

Generic helper functions (that are used by more than one resource) exist in https://github.com/PowerShell/SqlServerDsc/blob/dev/SqlServerDscHelper.psm1

[twerthi]

Let me look into the helper function.

[johlju]

@twerthi Great work on this! When you have time, can you look at the lint errors here https://ci.appveyor.com/project/PowerShell/sqlserverdsc/builds/19832432?fullLog=true#L2172

[twerthi]

@johlju The build is failing stating that there were commands missed in code coverage. I'm not familiar with what it's trying to tell me, would you be able to provide some explanation? I'll gladly fix it, just not sure what it's saying :)

[twerthi]

That last commit isn't right, not validating password on disabled accounts. I see a need where someone would want to change a password of a disabled account such as sa, which is often disabled. I will fix that.

[twerthi]

AppVeyor has been unable to build this project. I have it hooked up to my repository and each failure is something different. Mostly it's out of memory exceptions, but it will sometimes not be able to install SQL so the tests themselves fail. Is there a way to have AppVeyor give the containers more RAM?

[johlju]

Default it seems the containers use as much memory as they need. https://docs.docker.com/engine/reference/run/#user-memory-constraints

Maybe the constraint is the build worker. The build worker do install several instances in parallel with the containers. The integration tests is running when the containers are running unit tests. The instances (except one I think) is stopped by the integration test as a final test. Maybe before That they ett to much memory of the build worker (or the build worker does not dynamically allocate more memory fast enough).
If removing all integration tests does the containers finish successfully?

If they do, maybe we have to look how to optimize or remove integration tests.

[twerthi]

Removed integration tests and trying on my repository ... which I think is tied to this PR so that commit is in there too. Please don't merge until we are able to resolve. Should I close the PR?

[twerthi]

Hmm, looks like it still fails. I'm a bit out of my element here, any advice?

[twerthi]

Going to close PR and run a build just on my repository to see if the two running at the same time is causing issues.

[twerthi]

It's hit or miss as to whether it works. I'll reopen.

[johlju]

@twerthi I rebased this against dev to remove the 'merge...' commits. You probably did git pull or git merge. But wanted to make sure the commit history was in the right order so did git rebase, and to make sure there was no changes I missed that the merge might have done.

[johlju]

@twerthi I will make sure this pass the tests and then I will merge. Awesome work on this PR! 😄

[johlju]

Reviewed 10 of 35 files at r20.
Reviewable status: complete! all files reviewed, all discussions resolved

[twerthi]

Oh! That is most likely the case, I used the UI and did merges, I didn't know there was a difference between merge an rebase. Looks like I'll have get more familiar with the command line of Git instead of utilizing the GUI of GitHub :)

[johlju]

@twerthi Maybe there is a way of doing rebase through GUI as well? If you didn't know, rebase takes the dev branch and add that to you branch, then it replays all the commits you did. While 'git merge' sees the difference between dev and you branch and add a commit with the difference. Basically.

[johlju]

@twerthi I have finally merge this! Again, awesome work!

[twerthi]

@johlju I appreciate the help you gave me. I'll see if I can practice my Git command line :)