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

SqlAG/SqlAGReplica: Logic #517

Closed
Zuldan opened this issue Apr 20, 2017 · 11 comments
Closed

SqlAG/SqlAGReplica: Logic #517

Zuldan opened this issue Apr 20, 2017 · 11 comments
Labels
enhancement The issue is an enhancement request.

Comments

@Zuldan
Copy link

Zuldan commented Apr 20, 2017

Details of the scenario you try and problem that is occurring:

I hope this comes across without sounding too confusing....

At the moment xSQLServerAlwaysOnAvailabilityGroup and xSQLServerAlwaysOnAvailabilityGroupReplica are trying to enforce defaults even when a property is not set but I think it should not check the state of a property if it's not defined in the DSC resource. I would like my DBA's to have control over certain properties and not DSC.

Here is an example.

If I don't set the FailoverMode in xSQLServerAlwaysOnAvailabilityGroup then the resource will enforce 'Manual' mode. So if a DBA gets a call from the network guy to say there will be some network maintenance, the DBA might want to change FailoverMode to Automatic so he doesn't get a phone call in the middle of the night. The problem is now DSC will revert the FailoverMode to Manual even though I haven't specified it in the config.

The "Desired State" should only be what I define in the config. If the properties are not defined then they shouldn't be enforced. @johlju what are your thoughts?

The DSC configuration that is using the resource (as detailed as possible):

N/A

Version of the Operating System, SQL Server and PowerShell the DSC Target Node is running:

Server: Windows 2012 R2 (Update)
SQL Server: 2016 SP1 CU2
Powershell: 5.1

What module (SqlServer or SQLPS) and which version of the module the DSC Target Node is running:

SqlServer v20.0

Version of the DSC module you're using, or 'dev' if you're using current dev branch:

7.0.0.0

@johlju
Copy link
Member

johlju commented Apr 20, 2017

@Zuldan I agree with this. If you haven't set a property it shouldn't enforce it. The problem you are having is that the line 659 is seeing the default values as well and are trying to enforce those default values. It should not do that. Default values should only be used during setup.

@randomnote1 What are your thoughts on this?

@johlju johlju added enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community. labels Apr 20, 2017
@randomnote1
Copy link
Contributor

@Zuldan, this makes complete sense to me. When I wrote the resource I had to use default values in order to build the AG, but it didn't occur to me that somebody might not want the default enforced. This will take some work to change, particularly on the unit testing side.

Thanks for using & testing these! Glad to know there's some interest in the resources.

@Zuldan
Copy link
Author

Zuldan commented Jun 8, 2017

@randomnote1 any chance you would have time to make this change before the next release? We're currently getting through the issue by temporarily disabling DSC on the SQL clusters when we need to manage settings not configured by DSC. Not an ideal situation lol. Any improvements you could make to this would be greatly appreciated.

@randomnote1
Copy link
Contributor

I don't think it's likely in the short term. I have a few weeks of vacation coming up plus some other projects for my day job that need attended to.

@Zuldan
Copy link
Author

Zuldan commented Jun 8, 2017

@randomnote1 that's ok I understand. Appreciate all your work so far forgetting these modules together.

@randomnote1
Copy link
Contributor

"Accidentally" started fixing this for xSQLServerAlwaysOnAvailabilityGroup while working issue #468.

@johlju
Copy link
Member

johlju commented Sep 27, 2017

Great @randomnote1! Labeling this as in progress.

@johlju johlju added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Sep 27, 2017
@Zuldan
Copy link
Author

Zuldan commented Sep 27, 2017

@randomnote1 fantastic news! :-)

johlju pushed a commit that referenced this issue Oct 9, 2017
- Changes to xSQLServerAlwaysOnAvailabilityGroup
  - Refactored the unit tests to allow them to be more user friendly and to test
    additional SQLServer variations.
    - Each test will utilize the Import-SQLModuleStub to ensure the correct
      module is loaded (issue #784).
  - Fixed an issue when setting the SQLServer parameter to a Fully Qualified
    Domain Name (FQDN) (issue #468).
  - Fixed the logic so that if a parameter is not supplied to the resource, the
    resource will not attempt to apply the defaults on subsequent checks
    (issue #517).
- Added the CommonTestHelper.psm1 to store common testing functions.
  - Added the Import-SQLModuleStub function to ensure the correct version of the
    module stubs are loaded (issue #784).
@johlju johlju changed the title xSQLServerAlwaysOnAvailabilityGroup / Replica - Logic SqlAG/SqlAGReplica: Logic Dec 23, 2017
@johlju
Copy link
Member

johlju commented Jan 5, 2018

@randomnote1 Can you confirm that this was fixed in PR #853 (or any another PR)? If so let's close this issue, if not, then I change the label back to 'Help Wanted'.

@randomnote1
Copy link
Contributor

Yes, this was fixed in PR #853. I must have missed this issue when I created the resolved issues list!

@johlju johlju removed the in progress The issue is being actively worked on by someone. label Jan 5, 2018
@johlju
Copy link
Member

johlju commented Jan 5, 2018

No worries. Thanks for confirming! Closing,

@johlju johlju closed this as completed Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is an enhancement request.
Projects
None yet
Development

No branches or pull requests

3 participants