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!