Add retention capabilities to SQL Backup Database Step Template

[bcullman]

Background

Allow user of the SQL Backup Database step template to optionally limit the number of backup files on disk

Results

By Default, this script will work exactly the same as before - the retention capability is set to false via the RetentionPolicyEnabled value. It set to true, we clean up Db backup files, leaving only newest N remaining, where N is specified in RetentionPolicyCount

Before

After

Pre-requisites

• [✔] Id should be a GUID that is not 00000000-0000-0000-0000-000000000000
  • NOTE If you are modifying an existing step template, please make sure that you do not modify the Id property (updating the Id will break the Library sync functionality in Octopus).
• [✔] Version should be incremented, otherwise the integration with Octopus won't update the step template correctly
• [✔] Parameter names should not start with $
• [✔] Step template parameter names (the ones declared in the JSON, not the script body) should be prefixed with a namespace so that they are less likely to clash with other user-defined variables in Octopus (see this issue). For example, use an abbreviated name of the step template or the category of the step template).
• [✔] LastModifiedBy field must be present, and (optionally) updated with the correct author
• [✔] The best practices documented here have been applied
• [NA] If a new Category has been created:
• [✔] An image with the name {categoryname}.png must be present under the step-templates/logos folder
• [NA] The switch in the humanize function in gulpfile.babel.js must have a case statement corresponding to it

[CLAassistant]

All committers have signed the CLA.

[bcullman]

@ebalders - I think you were the last person to touch this script. Your review would be appreciated.

[ebalders]

@bcullman I don't have write access, so my review won't allow this to be merged, but I'll add my 2 cents.

Get-ChildItem $BackupDirectory -Exclude $retained | Remove-Item Here, a filter should be applied that only removes files starting with the db name. Even then, I think there is a chance this may delete unintended files:

Take the following databases and the resulting backup names: database1 => database1_timestamp.bak database1_final => database1_final_timestamp.bak

I think a safer approach would be to build a regex based on the $dbname and timestamp format used.

[twerthi]

I concur with @ebalders , good suggestion.

[twerthi]

Thank you for making the update, I think one more refinement on the filter and we should be in good shape. Adding something like

$regexPattern = "$($dbName)_\d+.*\.bak"

$allfiles = (Get-ChildItem $BackupDirectory | Where-Object {$_.Name -Match $regexPattern})

would filter out files that have a similar name. @ebalders , thoughts?

• I'm not terribly good at Regex, so the pattern may be able to be enhanced.

[bcullman]

@twerthi, @ebalders

Appreciate the feedback. I am working on refactoring so i can put some unit tests around this change.

Converting this to a draft until i validate this effort.

[bcullman]

@twerthi, @ebalders

Thank you for your patience. Completed my validation here.

Please note: The existing unit test in the project for windows-scheduled-task-create did not seem work on my machine on the master branch. 😢 I did nothing here to resolve that issue here.

I did alter the Invoke-PesterTests.ps1 to fall back to a global instance of Pester if the local one is not available.

[harrisonmeister]

I did alter the Invoke-PesterTests.ps1 to fall back to a global instance of Pester if the local one is not available.

I had some issues executing the pester tests using Invoke-PesterTests.ps1 as there wasnt a sql-backup-database.ScriptBody.ps1 file in the (root) folder that could be "." sourced. The function file threw exceptions for both

However, after I created one from the step template itself and then ran your tests with 7 passing.

I completely ignored windows scheduled task one too.

[bcullman]

I had some issues executing the pester tests using Invoke-PesterTests.ps1 as there wasnt a sql-backup-database.ScriptBody.ps1 file in the (root) folder that could be "." sourced. The function file threw exceptions for both

However, after I created one from the step template itself and then ran your tests with 7 passing.

🤔 It wouldn't be too hard to alter the script to check for a > 0 count of .ps1 files and throw a warning if none are found. Or, perhaps even check for each .Tests.ps1 file, check for a matching *.ps1 file...

[harrisonmeister]

🤔 It wouldn't be too hard to alter the script to check for a > 0 count of .ps1 files and throw a warning if none are found. Or, perhaps even check for each .Tests.ps1 file, check for a matching *.ps1 file...

We'll have a discussion internally and see if its something we want to update

[bcullman]

@harrisonmeister - Running the unit tests as part of a PR status check via a GitHub action or other means would be something to review as well. Although I understand if you were to conclude it's not worth the effort, given there are only 2 unit tests at the moment, and one of them does not seem to work...

Although

...once Github action were place, you could then automate the Converter.ps1 step. Then, you could make it so that folks check in the actual .ps1 files, rather than the .json files for step templates.

That would be a big win, because then PRs would be code reviews not of the .json files, where the script under review script has been compressed into a single json property, but instead of the actual .ps1 files themselves, where commentary can happen line-by-line, and git history becomes more useful.