New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow using rsync for uploading files to the target machine. #359
Conversation
This is much faster than Net::SCP.
We can't guarantee that RSync exists on the target system... |
But when it does, it significantly improves the test turnaround time. In my pull request, I made it revert to Net::SCP if rsync fails. I can make it only use rsync if specified in |
upload_done = false | ||
if !@rsync_failed && | ||
File.directory?(local) && options[:recursive] && | ||
File.exists?('/usr/bin/rsync') && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe being able to load homebrew rsync for newer features might be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion. However, I would like to hear about what it would take to integrate this into test-kitchen
(as a plugin or otherwise) before spending more time on this pull request.
So, it seems like this would be better served as a plugin. I don't like the idea of having rsync as the default, and there are no tests around this functionality. We chose scp because of the limited places it can fail. We are also working on Windows support, and using rsync seems like a step in the wrong direction. On the other hand, I completely agree that rsync is much much faster. Let me think about this a bit more and wait for @fnichol to weigh in as well. |
@damm like I said, I need to think on this a bit more. |
@sethvargo understood. I think also moving this to Plugins would allow any custom syncers to integrate seamlessly so it's rather forward thinking :) |
Just weighing in with a +1 on this idea, file transfers eat a lot of time when testing rapidly. |
A LOT |
@sethvargo @fnichol this is temporarily a huge win and it falls back to default behavior if rsync doesn't exist. Can we approve in the meantime so those of us who are relying on test-kitchen with lots of cookbooks don't waste a ridiculous amount of time when uploading? It's SERIOUSLY affecting productivity and I think in this case, we can always just re-factor it later. |
@mahmoudimus there's nothing stopping you from using @mbautin's branch. But we aren't just going to merge changes because it has a lot of 👍s - that's how software projects become unmanageable. |
@mbautin thx for your work here.. makes test kitchen w/ ec2 driver a 🍹 |
sethvargo: we have some very complex cookbooks with quite some dependencies, most of the time is before actual work is done, is waiting for scp's. It is an so unpleasant experience, that i spent like hours just to find out what actually causes this. ok, the code is not very pretty and could need some cleanup, but why would you not accept something that makes lives much more pleasant for many by adding an optional tool that is actually designed for transfer of data ? |
@poelzi have you checked out the kitchen-sync plugin ? |
Oh yeah, I probably should have mentioned kitchen-sync on this thread a while ago :) |
This is much faster than Net::SCP.
Some test results: uploading cookbooks took 36 seconds with Net::SCP, 0.42 sec with rsync in one case.