Skip to content
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

Windows support #3

Merged
merged 9 commits into from
Jan 20, 2016
Merged

Windows support #3

merged 9 commits into from
Jan 20, 2016

Conversation

axier
Copy link
Contributor

@axier axier commented Dec 29, 2015

I want to add windows support to your module. This module is tested on windows 2012 R2 servers.

@pcfens
Copy link
Owner

pcfens commented Dec 29, 2015

This is awesome! Thanks so much for adding windows support - since I'm not a windows user, I probably never would have done this.

Can you make the download URL a parameter so it can be modified for systems that aren't connected to the internet? Does downloading the zip file with lwf/remote_file work in place of powershell? If we're able to use something that isn't OS specific then it'll be easier to add non-package install support for Linux, though we'll probably need another class like filebeat::download. install_dir, and tmp_dir should probably be made parameters too so that everything is covered too.

Does it make sense to rename filebeat::install to something like filebeat::install::windows, just to keep things separated, as well as give us a spot to add a similar installer for Linux?

@pcfens
Copy link
Owner

pcfens commented Jan 11, 2016

I've merged this into a new branch, windows, as a working place while some of the few outstanding issues are handled before merging into master.

@pcfens
Copy link
Owner

pcfens commented Jan 11, 2016

I've done some pretty heavy re-factoring to set things up in case we ever need to add the ability to download and install from tarball on Linux. I also replaced the powershell download with lwf/remote_file.

Tests seem happy so far, but since I'm not a windows person I'd appreciate it if someone could test the latest branch before I look at merging it in.

@pcfens
Copy link
Owner

pcfens commented Jan 14, 2016

Hi @axier - I made a few tweaks after my own testing, and now things seem to work without any problems in a vagrant 2012R2 box.

@axier
Copy link
Contributor Author

axier commented Jan 14, 2016

Sorry for the late response but I have been a little busy those days. I see that you have done lot of improvements. I will test the windows branch. Let me know if I can help in something.

@pcfens
Copy link
Owner

pcfens commented Jan 15, 2016

Sorry if it felt like I was bugging you - I just wanted to make sure I documented what was going on a bit so it'd be easy to catch up. This will be an awesome addition. Thanks again for all your help.

@pcfens pcfens merged commit 3fd12e0 into pcfens:master Jan 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants