-
Notifications
You must be signed in to change notification settings - Fork 344
Added Windows variants for v5 #151
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
Signed-off-by: Elton Stoneman <[email protected]>
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.
Really nice work!
|
||
# REMARKS - DNS and mount tweaks needed for Windows | ||
RUN Set-ItemProperty -path 'HKLM:\SYSTEM\CurrentControlSet\Services\Dnscache\Parameters' -Name ServerPriorityTimeLimit -Value 0 -Type DWord; ` | ||
Set-ItemProperty -path 'HKLM:\SYSTEM\CurrentControlSet\Control\Session Manager\DOS Devices' -Name 'G:' -Value '\??\C:\data' -Type String |
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.
Any particular reason why you have chosen G:
? :-)
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.
Well, D
is for the CD-ROM drive... Just chosen to distance it from any other mappings the user may want. Could be E
or Z
I suppose?
5/windows/nanoserver/Dockerfile
Outdated
WORKDIR c:/elasticsearch | ||
COPY config ./config | ||
|
||
CMD ".\bin\elasticsearch.bat" |
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.
Should we better switch back to SHELL cmd to avoid running a powershell.exe that calls a cmd.exe to call the elasticsearch.bat?
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.
Yes, will fix that.
@@ -0,0 +1,38 @@ | |||
# escape=` | |||
FROM microsoft/windowsservercore |
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.
Once docker-library/openjdk#88 is merged and Windows images are available it should depend on these ones.
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.
Yep.
RUN Invoke-WebRequest -outfile elasticsearch.zip "https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-$($env:ES_VERSION).zip" -UseBasicParsing; ` | ||
if ((Get-FileHash elasticsearch.zip -Algorithm sha1).Hash -ne $env:ES_SHA1) {exit 1} ; ` | ||
Expand-Archive elasticsearch.zip -DestinationPath C:\ ; ` | ||
Move-Item "c:/elasticsearch-$($env:ES_VERSION)" 'c:\elasticsearch'; ` |
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.
Can you use a backslash here?
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.
Yes, because it's not in double-quotes it doesn't need to be escaped. The rules are a bit too intricate.
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.
Oh, I now see the double quotes. I had lots of trouble in the past with them. They "got lost" and I think they just disappear here without any errors.
So I suggest to remove them here.
Yes, just tried a
RUN Move-Item c:/elasticsearch-$($env:ELASTICSEARCH_VERSION) 'c:\elasticsearch' ; \
without quotes also work.
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.
Agreed.
Signed-off-by: Elton Stoneman <[email protected]>
@StefanScherer - thanks for your review. Updated now with your suggestions. |
@StefanScherer - can I ask what the next steps are with this? Thanks! |
@sixeyed Well I'm not a maintainer. Probably still some missing CI server for Windows? Or should the PR for the openjdk Windows image be merged first. |
I think we are close on docker-library/openjdk#88; so I'd like to see this based on that once we have the other merged and images pushed to the Docker Hub. |
Signed-off-by: Elton Stoneman <[email protected]>
@yosifkit updated PR to use the new Windows OpenJDK base image, and bumped ES to 5.2.0 to match Linux versions. |
Given that this image repo is officially deprecated in favor of the elasticsearch images provided by elastic.co and will receive no further updates after 2017-06-20 (June 20, 2017), we'd rather not add any new variants. See also discussion in #160. You'll probably want to open a new issue with elastic.co in their docker repo: https://github.com/elastic/elasticsearch-docker. 😮 And it seems there is one open to start the discussion: elastic/elasticsearch-docker#61. Closing since the repo is being deprecated. |
This commit was created by the elastic-dockerfiles-publisher.
Signed-off-by: Elton Stoneman [email protected]
Proposed Dockerfiles for building Windows variants of Elasticsearch 5 - based on Windows Server Core and Nano Server images.
There is currently no official OpenJDK image for Windows, so these images download the JDK as part of the setup, following the approach in OpenJDK PR 88. When we have an official image, those steps can be removed from these Dockerfiles.
Two limitations in Windows are addressed with tweaks to the registry:
Both images build and run correctly on Windows 10 and Windows Server 2016.