dasuchin / grunt-ssh-deploy

Grunt SSH Deployment
MIT License
66 stars 45 forks source link

Options for zipping the upload and keeping a specified number of releases. #17

Closed hurricane766 closed 9 years ago

hurricane766 commented 9 years ago

scp was being extra slow on production deploys to remote servers so I started zipping the build directory before the scp call and unzipping after. I made this an option which defaults to true.

There has been talk in issues about having an option for cleaning up the release directory by only keeping a certain number of build directories around. I made an option for this.

dasuchin commented 9 years ago

I'd like some discussion on the topic of the zip before I merge this in.

I think it'd be better off being set to false by default rather than true. It seems like this has a pretty niche use and most people would prefer to just rely on scp of all the files rather than the zip. But that's just my thought. Anyone else care to chime in?

lochstar commented 9 years ago

Hi @hurricane766, might I suggest the function I referenced at issue https://github.com/dasuchin/grunt-ssh-deploy/issues/16 to streamline this, it tars and transfer on the fly. I've been using it as a local mod, I was considering suggesting it for the main branch.

hurricane766 commented 9 years ago

@dasuchin I set it to true by default because the amount of time that was saved was significant, although you're right to question this as it would be changing the default functionality most users have come to expect. I think most of the speed gains are from the overhead that comes with transferring ~2000 different files vs 1 file, rather than the actual compression. At any rate, I figured they'd enjoy the speed increase.

@lochstar I like your method as it is a lot cleaner and done in one function rather than 3. It does create a second ssh connection to do the transfer instead of using the existing connection - although I don't see a way around this?

hurricane766 commented 9 years ago

Oh and if anyone can see someways to make remoteCleanup more bulletproof please let me know. rm -rfcan nuke your releases if anything less than 1 gets through (including undefined - which I found out the hard way ;)

Of note: ls -t sorts by the last modified time for the folder (or anything in the folder). So if a user is messing in around in old releases it can change the sorting and subsequently what releases are deleted. It just occurred to me that since the releases are named with a timestamp this should probably just sort by name ls -r to avoid that case. Thoughts?

lochstar commented 9 years ago

@hurricane766 I'm fine with the idea of sorting the folder by name to determine the oldest release, since they're timestamped anyway.

I haven't had an issue with the 2nd ssh connection, I guess it'd only be a problem if a server had connection limitations.

hurricane766 commented 9 years ago

@lochstar @dasuchin I was going to use this instead of my current zip implementation:

var tarBuild = function(callback) {
                var remote_string = 'ssh ' + options.username + '@' + options.host +
                    ' -p' + options.port + ' "tar -C ' + options.deploy_path +
                    '/releases/' + timestamp + '/' + ' -jxf -"';
                var command = 'tar -C ' + options.local_path + ' -jcf - ./ | ' + remote_string;
                grunt.log.subhead('UPLOADING NEW BUILD');
                grunt.log.writeln('--- ' + command);
                execLocal(command, callback);
            };

But this needs to work for all 3 options the users have for ssh authentication. (username/password, privatekey, agent). agent will work as is, privatekey can work with a -i [path to key], but I'm not sure about the username/password as the user would have to enter their password at the prompt. I've only been able to find a library called sshpass that users would have to install for this to work without a prompt.

The above function is nicer but makes username/password authentication annoying. Should I keep the current method?

hurricane766 commented 9 years ago

Ok, I changed the default for zip deploy to false. I left the zipping the way it is as it will use the existing ssh connection and it will work for all 3 authentication scenerios.

hurricane766 commented 9 years ago

I've found some issues with the proposed changes, I will resubmit a pull request when I've fixed the issues.

hurricane766 commented 9 years ago

I'm having problems getting client.scp to work with agent, the handshake always times out.

hurricane766 commented 9 years ago

Ok sorry for the close/reopen, I didn't want it to get pulled before I fixed that bug. I've tried it a few times now and it has worked for me.