search

packer-community / packer-windows-plugins

A suite of Packer plugins for provisioning Windows machines
113 stars 20 forks source link

Elevated scripts attempt to upload the elevated runner without any retry logic #65

Open chrissimon-au opened 8 years ago

chrissimon-au commented 8 years ago

In https://github.com/packer-community/packer-windows-plugins/blob/master/provisioner/powershell/provisioner.go, line 460, the generateElevatedRunner function uploads the script to create the scheduled task immediately.

However, it does so without any retry logic (e.g. the way non-elevated scripts are wrapped in p.retryable, line 277). This means that if an elevated script is being run after a reboot, there is a risk that it will attempt to upload before the winrm connectivity is available again and will fail. We can extend pause_before massively (for safety) but we have found mixed results with this, with intermittent failures. It would be nice if this upload step could be wrapped in p.retryable as well to give it a better chance of succeeding.

This is the first time I've looked at the provisioner.go source, so feel free to ignore if I've misunderstood how it's working :)

An example of the error we get:

--> amazon-windows-ebs: Error processing command: Error preparing elevated shell script: 
 Error uploading file to $env:TEMP\winrmfs-5639a93e-af78-be60-787c-4bf1fe44f7f0.tmp: 
 Couldn't create shell: unknown error
 Post http://ec2-<ipaddress>.ap-southeast-2.compute.amazonaws.com:5985/wsman: dial tcp <ip address>:5985: 
 ConnectEx tcp: A connection attempt failed because the connected party did not properly 
respond after a period of time, or established connection failed because connected host 
has failed to respond.
mefellows commented 8 years ago

Thanks @chrissimon-au, good spot. First up, it's probably worth mentioning that this project has been moved to https://github.com/mitchellh/packer/, so any fix should be submitted there (e.g. https://github.com/mitchellh/packer/blob/master/provisioner/powershell/provisioner.go#L454).

We would certainly accept a PR and help getting it accepted in upstream Packer should it work - would you be interested in patching this for us?

chrissimon-au commented 8 years ago

Great, thanks for the heads up on the move. I'll add a bug on the upstream packer github and will see if I get a chance to put together a PR. Would love to help, but time constraints may prevent. Also, I've never done anything in Go before and don't have an environment setup - but it would be fun to give it a try!

Since lodging the bug have found a workaround (which also lowers the fix priority) - include an all-but-empty non-elevated provisioner just prior to the elevated provisioner to take advantage of the non-elevated retry handling, and only after that succeeds move onto the elevated provisioner. Testing that as we speak and will confirm on the main packer bug if successful.

mefellows commented 8 years ago

No worries - this is how I got into go (creating this project with Dylan) and I've never looked back. If I get some spare time I'll take a look and submit a patch myself.

Thanks for documenting the workaround.