test-kitchen / winrm-transport

WinRM transport logic for re-using remote shells and uploading files
Other
8 stars 9 forks source link

#642 unzip cookbooks #9

Closed xmik closed 9 years ago

xmik commented 9 years ago

First commit makes working with powershell scripts outputs easier, does not change any logic, only shows error messages prettier and, in some cases, shows at all (in relation to previous behavior). I really think it should go to master.

Second commit fixes the https://github.com/test-kitchen/test-kitchen/issues/642 issue. Without it, the directory cookbooks was not unzipped from tmpzip-{hash}.zip file. I tested on Windows 7 only. It may need further testing on other Windows versions.

There is still another bug I see, but did not fix: once dna.json and solo.rb are transferred to Windows vm on kitchen converge, they are not updated on next kitchen converge. I did not fix it, because it does not bother me and I actually did not go deep through the MD5 functions here.

xmik commented 9 years ago

I am aware of https://github.com/test-kitchen/winrm-transport/pull/4 and changing a way of how decode_files.ps1 is invoked in file_transporter.rb#L254, because, as @smurawski said:

when we ran the Decode-Files line in the script it makes testing a bit tougher

However, that single change fails for me with errors (parsed from CLIXML powershell script output):

>>>>>> ------Exception-------
>>>>>> Class: Kitchen::ActionFailed
>>>>>> Message: Failed to complete #converge action: [[WinRM::Transport::FileTransporter] Upload failed (exitcode: 0), but stderr present
The term 'Function' is not recognized as the name of a cmdlet, function, script
 file, or operable program. Check the spelling of the name, or if a path was in
cluded, verify that the path is correct and try again.
At line:3 char:2
+ . <<<<  Function Cleanup($o) { if (($o) -and ($o.GetType().GetMethod("Dispose
") -ne $null)) { $o.Dispose() } }
    + CategoryInfo          : ObjectNotFound: (Function:String) [], CommandNot 
   FoundException
    + FullyQualifiedErrorId : CommandNotFoundException

The term 'Cleanup' is not recognized as the name of a cmdlet, function, script 
file, or operable program. Check the spelling of the name, or if a path was inc
luded, verify that the path is correct and try again.
At line:32 char:50
+   } catch [exception]{throw $_} finally { Cleanup <<<<  $c; Cleanup $in }
    + CategoryInfo          : ObjectNotFound: (Cleanup:String) [], CommandNotF 
   oundException
    + FullyQualifiedErrorId : CommandNotFoundException

The term 'Cleanup' is not recognized as the name of a cmdlet, function, script 
file, or operable program. Check the spelling of the name, or if a path was inc
luded, verify that the path is correct and try again.
At line:32 char:62
+   } catch [exception]{throw $_} finally { Cleanup $c; Cleanup <<<<  $in }
    + CategoryInfo          : ObjectNotFound: (Cleanup:String) [], CommandNotF 
   oundException
    + FullyQualifiedErrorId : CommandNotFoundException

Remove-Item : Cannot remove item C:\Users\Administrator\AppData\Local\Temp\tmpz
ip-16fce6ee41b3c10b6f4364883253b1a8.zip: The process cannot access the file 'C:
\Users\ADMINI~1\AppData\Local\Temp\tmpzip-16fce6ee41b3c10b6f4364883253b1a8.zip'
 because it is being used by another process.
At line:22 char:50
+     if ($tzip) {Unzip-File $tzip $dst;Remove-Item <<<<  $tzip -Force}
    + CategoryInfo          : WriteError: (C:\Users\ADMINI...4883253b1a8.zip:F 
   ileInfo) [Remove-Item], IOException
    + FullyQualifiedErrorId : RemoveFileSystemItemIOError,Microsoft.PowerShell 
   .Commands.RemoveItemCommand
]
>>>>>> ----------------------
>>>>>> Please see .kitchen/logs/kitchen.log for more details
>>>>>> Also try running `kitchen diagnose --all` for configuration

The PowerShell version I tested is:

$PSVersionTable.PSVersion
Major  Minor  Build  Revision
-----  -----  -----  -----
2      0      -1     -1
smurawski commented 9 years ago

It's not moving the Decode-Files change that breaks V2. What breaks V2 is when unpacking with the COM object, the destination folder has to exist beforehand. I've got a pull request in to fix that. I've also started to add more test coverage to the powershell commands themselves, testing both COM unpacking and IO.Compression unpacking.

xmik commented 9 years ago

@smurawski, I just tested your branch and am very glad you added tests. I used commit https://github.com/test-kitchen/winrm-transport/commit/41dc8ffc306803597fb5356b5ba6222ae8ee464a.

I ran kitchen converge and got:

...
D      [FileTransporter] Failed to upload 
D      Cleaning up local sandbox in /tmp/default-windows-vagrant-sandbox-20150616-10504-2aeyq7
>>>>>> ------Exception-------
>>>>>> Class: Kitchen::ActionFailed
>>>>>> Message: Failed to complete #converge action: [Failed to upload all files.]
>>>>>> ----------------------
>>>>>> Please see .kitchen/logs/kitchen.log for more details
>>>>>> Also try running `kitchen diagnose --all` for configuration

D      ------Exception-------
D      Class: Kitchen::ActionFailed
D      Message: Failed to complete #converge action: [Failed to upload all files.]
D      ---Nested Exception---
D      Class: RuntimeError
D      Message: Failed to upload all files.
D      ------Backtrace-------
D      /home/ewa/.chefdk/gem/ruby/2.1.0/gems/winrm-transport-1.0.1/lib/winrm/transport/file_transporter.rb:265:in `decode_files'
D      /home/ewa/.chefdk/gem/ruby/2.1.0/gems/winrm-transport-1.0.1/lib/winrm/transport/file_transporter.rb:96:in `block in upload'
...

I ran kitchen converge second time, it raised no such error and chef provisioning started. When I removed C:\Users\Administrator\AppData\Local\Temp\kitchen directory on Windows vm and ran kitchen converge again, the same FileTransporter error happened.

smurawski commented 9 years ago

Yeah, I literally just changed that and reverted. I was trying to catch failed copies, but that's going to take more work than I care to for this case.

I'm still trying to troubleshoot the failed single file copies.

xmik commented 9 years ago

commit 2b249f2d5c2fbf84478d9e491bb8ec195d71cdd7 works (no errors for the whole kitchen converge). Still dna.json and solo.rb are not updated between converges (they are never removed), but that is minor issue.

Since you added tests, it seems more reasonable to use your branch, but I'd still suggest merging the first commit from my pull request (23a92d9318593f31ec3b2c661c9ee6281bfb1e87) to handle powershell scripts errors better.

smurawski commented 9 years ago

@xmik Awesome. I think I found the problem with the single file uploads (like client.rb and dna.json) - testing now and should be pushing to my branch shortly.. I'll try to merge in your changes from that commit this afternoon.

smurawski commented 9 years ago

@xmik I've incorporated your commit. Thanks! And thanks for testing and confirming things! You rock.

xmik commented 9 years ago

Thank you too for cooperation :) Those 2 files are however still not updated (commit 518be5ce82cca9665b55a6bfc5e123b11d9d63a6) (e.g. when I change the run_list in .kitchen.yml, the dna.json does not change), but as I said, it does not bother me.

smurawski commented 9 years ago

Hmm.. Looks like it updates against 2012 R2, but not against 2008 R2. It's just a set-content call, so that should be working.

smurawski commented 9 years ago

I'll have to dig into that one in a bit. Thanks again.

fnichol commented 9 years ago

Does #8 pull the spirit of this PR in? If so, we can close, and if not…onward!

smurawski commented 9 years ago

@fnichol yeah, I grabbed one of @xmik 's commits, but #8 fixes the issue addressed here.

xmik commented 9 years ago

I'm closing this because https://github.com/test-kitchen/winrm-transport/pull/8 fixes the problem well (and is already in master).