-
Notifications
You must be signed in to change notification settings - Fork 90
Add dockerfile for windows server container build #64
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
Conversation
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.
Thanks for making the changes generic, will test it and get back to you.
FYI - Currently hitting some snags with the net-http-persistent dependency of this module - drbrain/net-http-persistent#90. Also, we're expermenting with using |
So we've moved to using net-http-persistent 2.9.4 since the PR to fix that drbrain/net-http-persistent#90 isn't going to be merged anytime soon. The changes from the jq plugin to the record_transformer plugin will be reflected when the PR is open with the configMap. |
# plugin hardfails when using net-http-persistent 3.0.1 on windows | ||
# https://github.com/drbrain/net-http-persistent/issues/79#issuecomment-347541046 | ||
# make an inline replacement to net-http-persistent to resolve | ||
RUN powershell -Command " \ |
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.
@chaitanyaphalak
Would it be possible for the splunk folks to host a fork of this with the patch applied, and publish the gem? Alternatively, the use of net-http-persistent could be changed in this repo so that splunk-hec gem still works with older versions of net-http-persistent.
@rhockenbury we recently merged a PR with a refactor removing net http persistent. You might need to validate this again. |
Can you clarify? I see that library used here - https://github.com/splunk/fluent-plugin-splunk-hec/blob/develop/lib/fluent/plugin/out_splunk_hec.rb#L10 |
The library is still there. |
Good news. The patch for net-http-persistent was merged. Once they put a new release out, and the version is bumped in fluent-plugin-splunk-hec, it should work on windows. |
@chaitanyaphalak Would you be able to bump the version of net-http-persistent so that the min version pulled in is 3.1.0? 3.1.0 contains the fix for the windows bug - https://github.com/drbrain/net-http-persistent/releases |
Also, haven't worked with circleci before, but we'll need to update the config to build the windows image. I'm happy to take a stab at it, but might be quicker if someone with experience with the build pipeline for this project did it - https://circleci.com/docs/2.0/hello-world-windows/. @chaitanyaphalak Any thoughts on this? |
@rhockenbury for the net-http-persistent plugin we can bump the version at release or with your PR. I am not sure when this PR will be merged as we currently don't have access to windows nodes. Similarly, I havent worked with windows images on circleCI, but looking at the link you shared it looks it's easy enough to get some builds working. Again, adding the images to the CI dosen't make sense until this PR is validated, so I would update the CI for building windows images as another PR after we merge this PR. |
Opened #104 for the dependency bump. Has the situation changed at all with regards to the ability to run windows nodes? Both AKS and GKE have preview support for running windows nodes - do the test clusters happen to run on either of those two platforms? |
As of now, there is no plan to support windows nodes. Closing the PR |
Proposed changes
This PR introduces a Dockerfile for windows server containers to support hybrid clusters for splunk-connect for kubernetes - splunk/splunk-connect-for-kubernetes#163
I would need assistance with getting the .circleci config right to build the windows container.
Types of changes
What types of changes does your code introduce?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply.