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

Install-DbaMaintenanceSolution Ignores -WhatIf #4901

Closed
4 tasks done
koglerk opened this issue Jan 3, 2019 · 4 comments · Fixed by #4924
Closed
4 tasks done

Install-DbaMaintenanceSolution Ignores -WhatIf #4901

koglerk opened this issue Jan 3, 2019 · 4 comments · Fixed by #4924

Comments

@koglerk
Copy link
Contributor

koglerk commented Jan 3, 2019

Before submitting a bug report:

  • Ensure you are able to reproduce it on the latest released version (0.9.734)
  • Verified this bug is not already reported in an issue
  • Verified errors are not related to permissions
  • Can reproduce in a clean PowerShell session (clean = powershell -NoProfile)

Steps to Reproduce

Running this code against the database where I have installed Ola Hallengren's Maintenance Solution:

Find-DbaStoredProcedure -SqlInstance devsql01 -Database dbatoolbox -Pattern hallengren | select computername, database, name, createdate | ft -a

Shows that the Ola stored procedures associated were created at 2:38PM:

ComputerName Database   Name                   CreateDate
------------ --------   ----                   ----------
DEVSQL01     dbatoolbox CommandExecute         1/3/2019 2:38:41 PM
DEVSQL01     dbatoolbox DatabaseBackup         1/3/2019 2:38:42 PM
DEVSQL01     dbatoolbox DatabaseIntegrityCheck 1/3/2019 2:38:42 PM
DEVSQL01     dbatoolbox IndexOptimize          1/3/2019 2:38:42 PM

Then I run this to reinstall the sprocs (but not the agent jobs), but I specify -WhatIf to see what'll happen:

Install-DbaMaintenanceSolution -SqlInstance devsql01 -Database dbatoolbox -ReplaceExisting -InstallJobs:$false -WhatIf

And the output sure looks like it made changes.

[14:42:49][Install-DbaMaintenanceSolution] Ola Hallengren's solution will be installed on database dbatoolbox.
[14:42:49][Install-DbaMaintenanceSolution] Dropping objects created by Ola's Maintenance Solution
[14:42:49][Install-DbaMaintenanceSolution] Installing on server devsql01, database dbatoolbox.
[14:42:49][Install-DbaMaintenanceSolution] Installing CommandExecute.sql.
[14:42:50][Install-DbaMaintenanceSolution] Installing MaintenanceSolution.sql.
[14:42:50][Install-DbaMaintenanceSolution] Installation complete.

Let's confirm that changes were made by running the same Find-DbaStoredProcedure again:

Find-DbaStoredProcedure -SqlInstance devsql01 -Database dbatoolbox -Pattern hallengren | select computername, database, name, createdate | ft -a
ComputerName Database   Name                   CreateDate
------------ --------   ----                   ----------
DEVSQL01     dbatoolbox CommandExecute         1/3/2019 2:42:49 PM
DEVSQL01     dbatoolbox DatabaseBackup         1/3/2019 2:42:49 PM
DEVSQL01     dbatoolbox DatabaseIntegrityCheck 1/3/2019 2:42:50 PM
DEVSQL01     dbatoolbox IndexOptimize          1/3/2019 2:42:50 PM

Sure enough, the create date has changed from 2:38PM to 2:42PM. It dropped and recreated these sprocs even though -WhatIf was used.

Expected Behavior

Using -WhatIf should not make any permanent changes.

Actual Behavior

Stored procedures are dropped and recreated.

Environmental data

PowerShell:

Name                           Value                                                                                                                                                                                                                                          
----                           -----                                                                                                                                                                                                                                          
PSVersion                      5.1.16299.820
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.16299.820
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

SQL Server:

Microsoft SQL Server 2016 (SP2-CU2) (KB4340355) - 13.0.5153.0 (X64)
Standard Edition (64-bit) on Windows Server 2012 R2 Standard 6.3 (Build 9600: ) (Hypervisor)

@wsmelton
Copy link
Member

wsmelton commented Jan 4, 2019

This is because this line is added to the function but the logic itself was not written to handle it.

https://github.com/sqlcollaborative/dbatools/blob/8373a3b6047933f267581a12b2b036c034c55e91/functions/Install-DbaMaintenanceSolution.ps1#L106

@wsmelton
Copy link
Member

wsmelton commented Jan 4, 2019

We should fix the parameter usage as well. This combination should be required -ReplaceExisting -InstallJobs:$false just to update the objects.

We have an example that shows ReplaceExisting will drop everything, but I don't think this is common practice with Ola's objects and jobs. The deployment script for the jobs is a basic example, and not one I use with any client or system out of the box. I always go and modify that command to what I need it to handle. The main deployment script does not allow for advanced configurations (e.g. you can't deploy using Backup To URL without modifying the backup command in the jobs created).

Shoot when I want to go update to the latest version of the code I just go to GitHub and pull the procedures and just run those scripts to update the objects.

So with all that, I'd say -ReplaceExisting should only do the procedures by default; then tie Force to also replacing the jobs. // @potatoqualitee

@ijeb ijeb self-assigned this Jan 9, 2019
ijeb pushed a commit that referenced this issue Jan 9, 2019
@ijeb
Copy link

ijeb commented Jan 9, 2019

@wsmelton I looked into your comments. I am not sure how to implement that without making it overly complex.

InstallJobs already has a default of false.

This is already implemented, but with the parameter -InstallJobs instead of -Force.
So, if -ReplaceExisting is used, only the database objects (stored procedures and a table) are dropped and created. If -ReplaceExisting and -InstallJobs are used in combination, the jobs are also dropped and created again.

-Force has already a different meaning and changing that would be complicated.

An issue that I would have a problem with is that it is not possible to just update the stored procedures without dropping/creating the table:

  • Dropping the table will probably most of the time be unnecessary, because the definition is unlikely to change.
  • The system should at least give a warning when the table is not empty and data would be lost.
  • The user might want to use a conversion script to move the data.

@wsmelton
Copy link
Member

wsmelton commented Jan 9, 2019

I can address it at a later time if @potatoqualitee would like the adjustment to be implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants