xSQLServerAlwaysOnAvailabilityGroup: Add Database Level Health Detection, per database DTC support and improve BasicAvailabilityGroup

[mdaniou]

Pull Request (PR) description
This PR is to implement the two new features in SQL Server 2016 :

DatabaseHealthTrigger : It is a boolean parameter. The resource will enable or disable it depending on its value
DtcSupportEnabled : It is a boolean parameter. The resource will enable it only as this feature can be configured at the creation of the Availability Group.

I have also figured that there was an issue with the feature :

BasicAvailabilityGroup :
The resource didn't allow us to disable the feature if we wanted because the check didn't take in account the fact that the parameter is a boolean.

This Pull Request (PR) fixes the following issues:
Fixes #721

Task list:

• Change details added to Unreleased section of CHANGELOG.md?
• Added/updated documentation, comment-based help and descriptions in .schema.mof files where appropriate?
• Examples appropriately updated?
• New/changed code adheres to Style Guidelines?
• Unit and (optional) Integration tests created/updated where possible?

---

This change is 

[msftclas]

@mdaniou,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

[codecov-io]

Codecov Report

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

@@         Coverage Diff          @@
##            dev   #730    +/-   ##
====================================
+ Coverage    97%    97%   +<1%     
====================================
  Files        30     30            
  Lines      3245   3255    +10     
====================================
+ Hits       3160   3170    +10     
  Misses       85     85

[johlju]

Are you working on the failing tests? Let me know if you need help.
I saw that you had a problem in the PR you closed, I guess you got past that problem?

[mdaniou]

Hi, yes it should be fine now. I need to fix the red lights. I didn't have much time to work on that the past days.
Thanks

[johlju]

@mdaniou No worries. Hope you get some time to get back to it soon. Love to get this in. :)

[johlju]

@mdaniou Please let me know if you need any help with the tests. Happy to help.

[mdaniou]

Sorry I am back into it. :)

[johlju]

@mdaniou Awesome! Let me know if you need any help 😄

[johlju]

Review status: 3 of 8 files reviewed at latest revision, 23 unresolved discussions, some commit checks failed.

---

CHANGELOG.md, line 6 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done.

There is still a typo in 'parameter'. :)

---

CHANGELOG.md, line 9 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done.

Typo in 'enable'. Missing 'e'.

---

Comments from Reviewable

[johlju]

Reviewed 4 of 7 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 72 at r1 (raw file):

sqlMajorVersion = $sqlMajorVersion

I think the easiest for this PR is to change back the property name to the original Version, and then we can submit a new issue to fix a read-only property in the schema.mof in another PR.

Version = $sqlMajorVersion

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 129 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done.

You got a single quote after 'Group'.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 667 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done.

This should me changed back to

$getTargetResourceResult.Version

See previous comment.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 701 at r3 (raw file):

# Add properties compatible with SQL Server 2016 or later versions
# DtcSupportEnabled is enabled at the creation of the Availability Group only, hence it will not be checked in this block

Please change to a comment block

<#
    Add properties compatible with SQL Server 2016 or later versions.
    DtcSupportEnabled is enabled at the creation of the Availability Group only, hence it will not be checked in this block.
#>

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.schema.mof, line 11 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done.

At first I wanted to implement the check of the edition, then I thought that it was not part of this fix.
Now I am wondering ... do we assume that the admin using this resource will handle this fact himself/herself or do we want this resource to fully do it.
At the current state the resource will fail if we want to enable BasicAvailabilityGroup in an Enterprise/Developper edition.

Should I create a bug for this ?

Yes, please submit an issue for this.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.schema.mof, line 13 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done.

You got a single quote after 'Group'.

---

Comments from Reviewable

[mdaniou]

Review status: 6 of 8 files reviewed at latest revision, 8 unresolved discussions.

---

CHANGELOG.md, line 6 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

There is still a typo in 'parameter'. :)

Done.

---

CHANGELOG.md, line 9 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Typo in 'enable'. Missing 'e'.

Done.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 72 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

sqlMajorVersion = $sqlMajorVersion

I think the easiest for this PR is to change back the property name to the original Version, and then we can submit a new issue to fix a read-only property in the schema.mof in another PR.

Version = $sqlMajorVersion

Done.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 129 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

You got a single quote after 'Group'.

Done.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 667 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

This should me changed back to

$getTargetResourceResult.Version

See previous comment.

Done.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 701 at r3 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

# Add properties compatible with SQL Server 2016 or later versions
# DtcSupportEnabled is enabled at the creation of the Availability Group only, hence it will not be checked in this block

Please change to a comment block

<#
    Add properties compatible with SQL Server 2016 or later versions.
    DtcSupportEnabled is enabled at the creation of the Availability Group only, hence it will not be checked in this block.
#>

Done.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.schema.mof, line 11 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Yes, please submit an issue for this.

Done.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.schema.mof, line 13 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

You got a single quote after 'Group'.

Done.

---

Comments from Reviewable

[johlju]

Reviewed 3 of 3 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.schema.mof, line 13 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done.

Seems the single quote is still there :)

---

Comments from Reviewable

[mdaniou]

Review status: 7 of 8 files reviewed at latest revision, 1 unresolved discussion.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.schema.mof, line 13 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Seems the single quote is still there :)

Done. (for good !)

---

Comments from Reviewable

[mdaniou]

Review status: 7 of 8 files reviewed at latest revision, 1 unresolved discussion.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.schema.mof, line 13 at r1 (raw file):

Previously, mdaniou (Maxime Daniou) wrote…

Done. (for good !)

Done.

---

Comments from Reviewable

[johlju]

Only a tiny thing left! After this I think we are good to go. 😄

---

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 666 at r5 (raw file):

and edition

Ah, I missed this one. Can you remove the reference to edition here?

---

Comments from Reviewable

[mdaniou]

Review status: 7 of 8 files reviewed at latest revision, 1 unresolved discussion.

---

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1, line 666 at r5 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

and edition

Ah, I missed this one. Can you remove the reference to edition here?

Done.

---

Comments from Reviewable

[johlju]

---

Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[johlju]

@mdaniou Thanks for this! Awesome work!

[mdaniou]

thanks :)
It was interesting to go through all the best practice and test unit, good learning !
Thanks for your help