dsccommunity / SqlServerDsc

This module contains DSC resources for deployment and configuration of Microsoft SQL Server.
MIT License
358 stars 225 forks source link

SqlScript: complains about duplicates when reusing scripts #596

Closed DarenDaigle closed 3 weeks ago

DarenDaigle commented 7 years ago

Details of the scenario you try and problem that is occurring: I am using an xSQLServerScript to restore four databases. After perfecting the first one, i am unable to reuse the scripts in their entirety.

Now, i am trying to run this four different times, changing the name of each section and the variable i am using to pass the parameters. I see no reason that i cannot reuse the scripts as i have written them to be generic enough to pass unique information each time i change the variables.

But right now, the only way I can make this work is to copy and paste the test file script and rename it for each new database I restore.

Here is the TSQL script I am using:

RESTORE DATABASE [$(TSQL_DB)] FROM DISK = '$(TSQL_SRC)' WITH MOVE '$(TSQL_DATA)' TO '$(TSQL_DATA_FILE)', MOVE '$(TSQL_LOG)' TO '$(TSQL_LOG_FILE)', RECOVERY, REPLACE, STATS = 10;

If these keys are required, then I propose a fifth key to allow the uniqueness to be defined.

The DSC configuration that is using the resource:

    xSQLServerScript RestoreDB {
        ServerInstance       = "localhost"
        SetFilePath          = "C:\source\sqlscripts\xScript_CreateDBFromBackup.sql"
        TestFilePath         = "c:\source\sqlscripts\xScript_TestForBackup.sql"
        GetFilePath          = "c:\source\sqlscripts\xScript_TestForBackup.sql"
        Variable             = $Node.RestoreDBParms
        PsDSCRunasCredential = $Node.LocalAdminAccount
        DependsOn            = "[File]RestoreBackupFile"
    }

Version of the Operating System and PowerShell the DSC Target Node is running: Windows 2012 R2 with WMF 5.1

Version of the DSC module you're using: xSQLServer 7.1.0.0

johlju commented 7 years ago

Thanks for sending in this issue!

So the problem is that you can not specify xSQLServerScript four times in a row using the same filenames, right? I guess it was a reason for that, so that the same files should not be used twice, potentially making two xSQLServerScript working against each other.

But, I guess if we set the Variable parameter to be unique also (making it a key) that would solve your problem. But that would make every configuration require Variable parameter to be set (a breaking change). Would users question that decision? Same if we add another parameter. If we add another parameter, it must be something that would feel like an enhancement to the resource for many users. Maybe adding the Variable parameter as Key would be just that. I feel we need more input on this issue for making a decision here. Do you have a suggestion for a fifth key?

To work around this you could of course make a script for each database you should restore, or make the one script restore all databases in the same script (passing all variables). I understand that this would not be ideal in your particular use case.

DarenDaigle commented 7 years ago

Well, honestly, i cannot see why they need to be keys at all. With DependsOn, you can cause them to order if needed. Either that or you could set IsSingleton to be a default of true which could be overridden to allow for this situation.

But unnecessarily creating the same file over and over seems a lot of work. And as you can see by the SQL statement, great pains where taken to be capable of reusing the code.

johlju commented 7 years ago

They need to be keys to be able to use the xSQLServerScriot resource more than once in a configuration.

A Boolean parameter IsSingleton would need to be a key, which would only allow two resources in the same configuration, one that is set to true and another that is set to false.

For you scenario to work you need to have a key parameter that is either an integer or a string that can be set to a unique value. If that unique value parameter exist then there would be no need for the script parameters to be key.

What would that unique parameter name be?

LukaszBiegus commented 5 years ago

I have similar issue to Daren. Used SQLScriptQuery resource to ensure a specific SQL Login with known SID value exists. Now would like to reuse the code for a second login and an hitting the 'duplicate' issue.

The new unique parameter could be called 'Name' (Key). One of the values in the Variables is likely to be a good candidate for the value of 'Name'. For me it would be the login name. For Daren - database name. Even if Variables are not used, it should be easy to come up with a unique name in your configuration.

Can a new key parameter have default value, so that this change is non-breaking?

yeghisheh commented 1 year ago

Any solution or workaround to this issue? We really need to be able to reuse SQL script files.

Borgquite commented 1 month ago

Also coming across the same issue - I like @johlju original suggestion - the Variable parameter to be made into a key, so that if you're running one script with many variables, it works. Since I can't think of a scenario where Variable wouldn't be different if you're running the script more than once, isn't this the best way ahead - rather than making another key?

Also I don't see how this can be done without a breaking change?

johlju commented 1 month ago

The problem with making Variable a key is that is has to be provided every time even if a variable is not wanted, so I don't think that is preferable solution either.

The main problem is trying to reuse a generic script (to avoid duplication?) instead of it being a script specifically written for that specific configuration instance. If the generic script would be copied to another name then this would not be a problem. 🤔

Borgquite commented 1 month ago

@johlju If someone doesn't need Variable, can't they just pass an empty array?

It's more for the scenario where you want to create multiple records, but don't want to specify the script each time. For example, imagine you need to add, and monitor drift for, 10 separate records within a given SQL table (This is something I've personally encountered in two different DSC setup scenarios (for example below here's something that checks if a particular subnet has been added to a network scanning solution's internal database):

You can write a foreach loop which creates 10x different resources like the one below, changing $tmpBaseSubnetIpStart, $tmpBaseSubnetIpend and $tmpBaseSubnetDescription and $IPLocation for each (see below).

However this creates 10 large, but almost identical resources (including all of the code) with the only difference between them being the variables included. If you're using Azure Automation there's a hard limit on 23Kb on the mof file which you can easily blow like this. When using SQLScript, you need to autogenerate 10 almost identical TSQL files (probably using the File resource) to achieve this, again unnecessarily bloating the MOF file. But this is exactly what variables were made for. If you can use a single TSQL file with different Variables, you can do this much much more efficiently using something like the second example.

_foreach subnet you need to create_
SqlScriptQuery $("$tmpBaseSubnetIpstart-$tmpBaseSubnetIpend\$tmpBaseSubnetDescription" -replace "['\(\)]" -replace "/", "\")
{
    ServerName = $dbinstance
    InstanceName = $sqlinstance
    Credential = $dbuserconfig
    GetQuery = @'
SELECT IPLocation, Realstart, Realend FROM dbo.tsysIPLocations' FOR JSON AUTO
'@
    TestQuery = @"
IF (SELECT COUNT(IPLocation) FROM dbo.tsysIPLocations WHERE IPLocation = '$IPLocation' AND Realstart = '$tmpBaseSubnetIpstart' AND Realend = '$tmpBaseSubnetIpend') = 0
BEGIN
RAISERROR ('Could not find IP Address Range location for $tmpBaseSubnetIpstart - $tmpBaseSubnetIpend', 16, 1)
END
ELSE
BEGIN
PRINT 'Found IP Address Range location for $tmpBaseSubnetIpstart - $tmpBaseSubnetIpend'
END
"@
    SetQuery = @'
RETURN
'@
}
_create .sql files containing above code using TSQL vairables
_foreach subnet you need to create_
  SqlScript 'RunAsSqlCredential'
  {
      ServerName   = 'localhost'
      InstanceName = 'SQL2016'
      Credential   = $SqlCredential

      SetFilePath  = 'C:\DSCTemp\SQLScripts\Set-RunSQLScript.sql'
      TestFilePath = 'C:\DSCTemp\SQLScripts\Test-RunSQLScript.sql'
      GetFilePath  = 'C:\DSCTemp\SQLScripts\Get-RunSQLScript.sql'
      Variable     = @('tmpBaseSubnetIpStart={0}' -f ($tmpBaseSubnetIpStart -replace "[']", "''"))
  }
Borgquite commented 1 month ago

@johlju Any thoughts as it seems quite a lot of people have this issue? I think I know how to implement this with Variable as another key (and update the doco).

Borgquite commented 1 month ago

After an attempt to implement Variable as a Key in #2042 it was discovered that it's not supported to use a StringArray[] as a Key (ends up with MOF compilation errors).

After some discussion in #DSC it was decided that setting up a new Key which can be set by the end-user to a unique value was the best way ahead. The easiest thing to use is simply the name of the resource (Id is not used for any configuration at all) and this allows a script to be reused, only updating the Id and Variable parameters i.e.:

SQLScript RestoreDB {
    Id                   = 'RestoreDB'
    ServerInstance       = "localhost"
    SetFilePath          = "C:\source\sqlscripts\xScript_CreateDBFromBackup.sql"
    TestFilePath         = "c:\source\sqlscripts\xScript_TestForBackup.sql"
    GetFilePath          = "c:\source\sqlscripts\xScript_TestForBackup.sql"
    Variable             = @("Database=DB")
    PsDSCRunasCredential = $Node.LocalAdminAccount
    DependsOn            = "[File]RestoreBackupFile"
}

SQLScript RestoreDB2 {
    Id                   = 'RestoreDB2'
    ServerInstance       = "localhost"
    SetFilePath          = "C:\source\sqlscripts\xScript_CreateDBFromBackup.sql"
    TestFilePath         = "c:\source\sqlscripts\xScript_TestForBackup.sql"
    GetFilePath          = "c:\source\sqlscripts\xScript_TestForBackup.sql"
    Variable             = @("Database=DB2")
    PsDSCRunasCredential = $Node.LocalAdminAccount
    DependsOn            = "[File]RestoreBackupFile"
}

PR #2043 implements this