Monday, May 13, 2013

Powershell Scripting Games Event 1, lesson learned

The 2013 Powershell Scripting Games have started for real. I am competing this is for the firs time in the Advanced category. For the moment two tasks have been published. In the first one we had to develop a function aimed at moving old logfiles to a location off the system drive. Easy right? Well, not really. The judges (as well as the other competitors) are very nitty-picky and I have soon discovered that any small mistake can bring very low scores.

Even tough I paid careful attention to what I was doing, I overlooked a few things which I want to share here.

Here's the function I wrote for the assignment:
  1. #Requires -Version 3.0  
  2.   
  3. set strict-mode latest  
  4.   
  5. function Archive-Logs {  
  6. <#  
  7.  .SYNOPSIS  
  8.    Archives application logs older than a certain amount of days to a specific location.  
  9.   
  10.  .DESCRIPTION  
  11.    Moves all the application logs older than a certain amount of days to a specific location.  
  12.    The subfolder structure is maintained.  
  13.   
  14.  .PARAMETER SourcePath  
  15.    Specifies the current path of the folder containing the logs to be moved.  
  16.   
  17.  .PARAMETER DestinationPath  
  18.    Specifies the destination path where to recreate the subfolders structure and move the logs.  
  19.   
  20.  .PARAMETER Days  
  21.    Specifies the minimum age in days of the logs that have to be archived.  
  22.    The default is 90 days.  
  23.   
  24.  .PARAMETER Log  
  25.    Switch parameter to enable error logging.  
  26.    The default is False.  
  27.   
  28.  .PARAMETER FailedLogFile  
  29.    Specifies the path of the file where the errors are logged if the Log switch is set to True.  
  30.    The default is $env:temp\failed-log-move.log  
  31.   
  32.  .PARAMETER ViewErrorLog  
  33.    Opens the error log for viewing using the default associated program (i.e. Notepad).  
  34.    The default is False.  
  35.    
  36.  .EXAMPLE  
  37.    Archives logs older than 100 days from C:\Application\Log to \\nasserver\Archives  
  38.   
  39.    Archive-Logs C:\Application\Log \\nasserver\Archives 100  
  40.    
  41.  .EXAMPLE  
  42.    Archives logs older than 100 days from C:\Application\Log to \\nasserver\Archives. It also logs errors to $env:temp\failed-log-move.log and opens this log for reading.  
  43.   
  44.    Archive-Logs C:\Application\Log \\nasserver\Archives 100 -Log -ViewErrorLog  
  45.   
  46.  .EXAMPLE  
  47.    Archives logs older than 2 years from C:\Application\Log to \\nasserver\Archives and shows verbose output  
  48.      
  49.    Archive-Logs C:\Application\Log \\nasserver\Archives 730 -Verbose  
  50.    
  51.  .EXAMPLE  
  52.    Simulates archival of logs older than 3 months from C:\Application\Log to d:\backup  
  53.      
  54.    Archive-Logs C:\Application\Log d:\backup 90 -Whatif  
  55.   
  56.  .NOTES  
  57.    Entry for 2013SG Advanced Event 1. To use, dot source script and run 'Archive-Logs'.  
  58.    Use 'get-help Archive-Logs -full' to see how to run complete help.  
  59.      
  60.    This function leverages Powershell V3 cmdlets and parameters, such as OlderThan in Test-Path or $PSItem instead of $_.  
  61.    I am also taking advantage of Get-ChildItem -File and -Directory.  
  62.      
  63. #>  
  64.     
  65.   [CmdletBinding(SupportsShouldProcess=$True)]  
  66.     
  67.   param(  
  68.     [Parameter(Mandatory, Position=0)]  
  69.     [ValidateScript({Test-Path $_ -PathType Container})]  
  70.     [String]$SourcePath,  
  71.   
  72.     [Parameter(Mandatory, Position=1)]  
  73.     [ValidateScript({Test-Path $_ -PathType Container})]  
  74.     [String]$DestinationPath,  
  75.   
  76.     [Parameter(Mandatory=$False, Position=2)]  
  77.     [Int]$Days = 90,  
  78.   
  79.     [Parameter(Mandatory=$False, Position=3, ParameterSetName = 'Logging')]  
  80.     [Switch]$Log = $False,  
  81.   
  82.     [Parameter(Mandatory=$False, Position=4, ParameterSetName = 'Logging')]  
  83.     [ValidateScript(  
  84.       {  
  85.       Try {  
  86.         Add-Content -LiteralPath $_ -Value $null -ErrorAction Stop  
  87.         $true  
  88.         }  
  89.       Catch { $false }  
  90.      })]  
  91.     [String]$FailedLogFile=(Join-Path $env:Temp 'failed-log-move.log'),  
  92.   
  93.     [Parameter(Mandatory=$false, Position=5, ParameterSetName = 'Logging')]  
  94.     [switch]$ViewErrorLog = $false  
  95.     )  
  96.   
  97.     Write-Verbose ("Script started processing at {0:hh}:{0:mm}:{0:ss}" -f (Get-Date))  
  98.     $ErrorTracker = 'without errors'  
  99.   
  100.     ForEach($SubFolder in (Get-ChildItem $SourcePath -Directory))   
  101.       {  
  102.       $DestSubFolder = Join-Path -Path $DestinationPath -ChildPath $SubFolder  
  103.       if(!(Test-Path $DestSubFolder))  
  104.         {  
  105.         Try {  
  106.           [void] (New-Item -path $DestinationPath -name $SubFolder -ItemType directory -Force -ErrorAction Stop)  
  107.           }  
  108.         Catch  
  109.           {  
  110.           Write-Warning "Failed creating $SubFolder in $DestinationPath"  
  111.           $ErrorTracker = 'with errors'  
  112.           if ($Log)  
  113.             {  
  114.             Add-Content -LiteralPath $FailedLogFile -Value "$(Get-Date) - Failed creating $SubFolder in $DestinationPath"  
  115.             }  
  116.           } # end Try/Catch  
  117.         }  
  118.       Try {  
  119.           $OldLogs = Get-ChildItem -Path $SubFolder.FullName -Recurse -File -Include *.log -ErrorAction Stop |  
  120.             Where-Object { Test-Path -Path $PSItem.FullName -OlderThan (Get-Date).AddDays(-$Days) }  
  121.           }  
  122.       Catch  
  123.           {  
  124.           Write-Warning "An error occurred retrieving logs from $($SubFolder.fullname)"  
  125.           $ErrorTracker = 'with errors'  
  126.           if ($Log)  
  127.             {  
  128.             Add-Content -LiteralPath $FailedLogFile -Value "$(Get-Date) - An error occurred retrieving logs from $($SubFolder.fullname)"  
  129.             }  
  130.           } # end Try/Catch  
  131.       if($OldLogs)  
  132.         {  
  133.         $OldLogs | Foreach-Object {  
  134.           Try {  
  135.             $CurrentLog = $PSItem  
  136.             Write-Verbose "Archiving $($CurrentLog.name) [$($CurrentLog.lastWriteTime)]"  
  137.             $CurrentLog | Move-Item -Destination $DestSubFolder -Force -ErrorAction Stop  
  138.             }  
  139.           Catch {  
  140.             Write-Warning "An error occurred moving $($CurrentLog.fullname)"  
  141.             $ErrorTracker = 'with errors'  
  142.           if ($Log)  
  143.             {  
  144.             Add-Content -LiteralPath $FailedLogFile -Value "$(Get-Date) - An error occurred moving $($CurrentLog.fullname)"  
  145.             }  
  146.             } # end Try/Catch  
  147.           }  
  148.         }  
  149.       } # end ForEach-Object  
  150.     Write-Verbose ("Script finished processing $ErrorTracker at {0:hh}:{0:mm}:{0:ss}" -f (Get-Date))  
  151.     if($ViewErrorLog)  
  152.       {  
  153.       & $FailedLogFile  
  154.       }  
  155.   } # end Function Archive-Logs  
Apparently everything is well. Unfortunately there are a few mistakes that make me score 2.8 out of 5 possible points.

Let's start with the biggest mistake, which is: 
  1. Get-ChildItem -Path $SubFolder.FullName -Recurse -File -Include *.log -ErrorAction Stop  
The problem here is that I have misinterpreted the use of Get-ChildItem. When I tell this cmdlet to Recurse, I am going to return also files which are stored in any subdirectory. At the same time if I omitted this switch, no files where returned.

The solution to this, I give credit to Mike for this, was to include the desired file extension into the Path parameter:
  1. Get-ChildItem -Path (Join-Path -Path $SourcePath -ChildPath '*\*.log') -ErrorAction Stop  
The lesson I get from this is always to read the cmdlet full help, even if I am pretty sure of how it works.

Another big error of mine was not to comment enough. Many people stated that my script was hard to follow. The fact is that I deemed unnecessary to explain how my code worked to people who already have a proficient knowledge of Powershell. I was wrong.

The lesson I get from this is always to put comments before complicated lines of code as well as the end of each scriptblock. Here's two examples:
  1.  foreach ($Computer in $ComputerName) {  
  2. ... any action ...  
  3. # end of foreach $Computer
or
  1. if($pingworks){  
  2. get-wmiobject...  
  3. # end of If block  
This makes my script much more readable.

The final thing that I learned from Event 1 and that I want to share with you is never to use superflous code. An example could be specifying that a parameters is not mandatory:
  1. [Parameter(Mandatory=$False]  
This is unneded in Powershell. So better to omit it.

That's all for the for Event 1. If you want to see it on the ScriptingGames website click on the image. Unfortunately is too late to vote for my entry on this assignment.

In the next post I will talk about the Second Event of the Games. Stay tuned!

No comments:

Post a Comment

Related Posts Plugin for WordPress, Blogger...