-
Notifications
You must be signed in to change notification settings - Fork 39.9k
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
Bump elasticsearch and kibana to 5.4.0 #45589
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Hi @it-svit. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Please recheck CLA |
@k8s-bot ok to test |
@it-svit Thanks a lot for you PR! I'll build the image and test it manually, but overall LGTM |
It might be useful for testing: Also kubernetes manifests should be updated a bit too. kibana-controller.yaml
|
@it-svit I haven't noticed one detail. Could you please update the image version in the Makefile to respect the ES version? In both images |
@it-svit While testing, I've encountered a problem:
Have you seen it? |
I've noticed that Elasticsearch base image was changed to alpine and it caused conflict. Regarding vm.max_map_count issue. I'm using CoreOS based kubernetes deployment via kube-aws (https://github.com/kubernetes-incubator/kube-aws) I've added the following options to the cloud-init files:
|
@it-svit Sorry for the late response Yeah, I understand the problem, but I think it's a bad experience for users to have to change something on their host machines to make this deployment work. Do you think it's possible to configure a container in such a way so that it works on the host without this change? |
/cc @coffeepac |
@crassirostris @it-svit this can be solved if you launch a privileged initialization container that can up the limit on the host machine by mounting /proc and writing to the limits file manually. I'm not sure that's an appropriate solution to propagate in the kubernetes repo though, it seems like a bad actor moment. This general problem is being talked about here I am unsure what the appropriate path forward is but re-launching nodes to pick up a new cloud config seems like a non-starter. I'm open to ideas about how to make this workable. |
@crassirostris Yes, I think that generally containers should depend on OS specific configuration. I've found possible solution but have not tested it yet. I'll try it and share result with you. |
@it-svit Thanks a lot, appreciate your efforts!
Should or should not? |
@crassirostris Should not, of course. |
@it-svit What's up? |
@crassirostris I'm sorry, but the Pull Request was closed accidentally by some github's internal logic. I was making force push in my repository at that moment. |
@crassirostris @coffeepac It seems, Elasticsearch 5.4.0 doesn't have option to set vm.max_map_count requirement less than 262144. I've applied changes recommended by @coffeepac. I've added Here is elasticsearch kubernetes manifest which I've used.
|
@it-svit I was unable to get sysctl calls to function from inside a running container when the host OS is containerOS, which is why I resorted to editing the file in /proc directly. What host OS did you try this on? @crassirostris is running as a privileged pod and reconfiguring the host machine an okay thing? Its what I do but I'm not sure we want to share that generally. also, @it-svit needs a release note ("Upgrade Elasticsearch Addon to v5.4.0" is sufficient) and your CLA check is still failing. |
@coffeepac I'm using CoreOS as host OS. I wonder, why is my CLA failing. |
@coffeepac I've noticed, that my email address kb@itsvit.org was attached to the "kb-itsvit" account instead of "it-svit". I fixed that. Please recheck CLA. |
@it-svit awesome! if it works on the minimal containerOS it should work everywhere. great. Thanks for the work and off it goes. |
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.
/lgtm
@coffeepac I have this problem on my list /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: crassirostris, it-svit
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
@crassirostris I can approve but not lgtm. thanks for looking into it. |
Automatic merge from submit-queue (batch tested with PRs 46124, 46434, 46089, 45589, 46045) |
What this PR does / why we need it: Updates elasticsearch and kibana docker image assets to 5.4.0 version
Release note: