Skip to content

5.0.2 debian package not able to build a docker image #21877

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

Closed
neoecos opened this issue Nov 30, 2016 · 15 comments · Fixed by #21899
Closed

5.0.2 debian package not able to build a docker image #21877

neoecos opened this issue Nov 30, 2016 · 15 comments · Fixed by #21899
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts discuss Team:Delivery Meta label for Delivery team

Comments

@neoecos
Copy link

neoecos commented Nov 30, 2016

Elasticsearch version: 5.0.2

Plugins installed: []

JVM version:

OS version: debian jessie

Description of the problem including expected versus actual behavior:
The modification of the /proc inside the configuration of the debian package, makes docker unable to create the image.
Steps to reproduce:
Check:
docker-library/elasticsearch#144

Provide logs (if relevant):

@clintongormley clintongormley added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts discuss labels Nov 30, 2016
@eht16
Copy link

eht16 commented Nov 30, 2016

A normal package upgrade in a LXC container is broken as well:

Setting up elasticsearch (5.0.2) ...
Couldn't write '262144' to 'vm/max_map_count', ignoring: Read-only file system
dpkg: error processing package elasticsearch (--configure):
 subprocess installed post-installation script returned error exit status 1
Errors were encountered while processing:
 elasticsearch

@jasontedor
Copy link
Member

@neoecos @eht16 I opened #21899; please let me know if this would suit your needs.

Also, please note the official Elasticsearch Docker image (the one built and supported by Elastic, not the one in Docker Hub that claims to be official but is not affiliated with nor supported by Elastic) does not have this problem. If you're interested, we have some docs and a GitHub repository.

@eht16
Copy link

eht16 commented Nov 30, 2016

@jasontedor thanks. Setting SKIP_SET_KERNEL_PARAMETERS would fit for me.
Even more convenient would be: /usr/lib/systemd/systemd-sysctl || true
Then setting the value would fail, the error message is shown but the installation will continue.
Maybe an additional hint would be useful in the case systemd-sysctl failed to explain to the user that the shown error message is harmless (as long as the user is aware to set vm.max_map_count herself on the host).

But I'm not really sure what is the better solution, just thinking.

@jasontedor
Copy link
Member

Thanks for the feedback @eht16. I considered that approach, and my concern with it is a lot of users do not read console messages when nothing goes wrong (so if the install does not exit with an error status, such users will just not see that setting the kernel parameters did not take). The upside to that approach is that it is less likely to get in the way of the end-user.

That said, I'm glad to know that we have two viable approaches that would be acceptable to at least one of you and @neoecos. I will solicit feedback from others (e.g., @clintongormley) but either way we will get a fix in for you very soon.

@neoecos
Copy link
Author

neoecos commented Dec 1, 2016

@jasontedor The setting solves the issue, thank you.

The difference between the two builds, is that the dockerhub image is build using the official debian package, meanwhile the elasticsearch image just downloads the binary .tar.gz.

For me, using a deb package is definitely more elegant way to do that.

RUN wget ${ES_DOWNLOAD_URL}/elasticsearch-${ELASTIC_VERSION}.tar.gz &&
EXPECTED_SHA=$(wget -O - ${ES_DOWNLOAD_URL}/elasticsearch-${ELASTIC_VERSION}.tar.gz.sha1) &&
test $EXPECTED_SHA == $(sha1sum elasticsearch-${ELASTIC_VERSION}.tar.gz | awk '{print $1}') &&
tar zxf elasticsearch-${ELASTIC_VERSION}.tar.gz &&
chown -R elasticsearch:elasticsearch elasticsearch-${ELASTIC_VERSION} &&
mv elasticsearch-${ELASTIC_VERSION}/* . &&
rmdir elasticsearch-${ELASTIC_VERSION} &&
rm elasticsearch-${ELASTIC_VERSION}.tar.gz

@jasontedor
Copy link
Member

jasontedor commented Dec 1, 2016

The difference between the two builds, is that the dockerhub image is build using the official debian package, meanwhile the elasticsearch image just downloads the binary .tar.gz.

Yes, I'm aware of what accounts for the difference. The points that I'm alluding to and making are the following:

  • the official Elastic images will work when new releases are made, rather than having to adjust to upstream changes such as the one that led to the problem here for downstream maintainers
  • the official Elastic images are supported by Elastic, the downstream images are not

@C0rn3j
Copy link

C0rn3j commented Jan 5, 2017

For those stumbling upon this and being confused like me- the setting merged ended up being ES_SKIP_SET_KERNEL_PARAMETERS, so set that to true if you want to ignore setting kernel parameters.

@jasontedor
Copy link
Member

@C0rn3j Thanks. This is also documented on the deb and RPM installation pages.

@wwentland
Copy link

@jasontedor I do not quite understand why the issue was fixed in such a way that sysctl is called at all even if /proc isn't writable. It looks as if this is a common issue (cf. #8793) and should be easy to fix. For now it is rather cumbersome to use ES in containers. Requiring every user to ensure that ES_SKIP_SET_KERNEL_PARAMETERS is set in the environment is quite suboptimal and might not be easy to achieve if the containers are provisioned automatically.

I would recommend to not call sysctl if /proc isn't writable. One might want to document that users can, for example, run their docker containers in privileged mode.

@jasontedor
Copy link
Member

I do not quite understand why the issue was fixed in such a way that sysctl is called at all even if /proc isn't writable.

We abhor silent failures. Either it succeeds, or it fails and you know about it and you take steps to correct it.

Requiring every user to ensure that ES_SKIP_SET_KERNEL_PARAMETERS is set in the environment is quite suboptimal and might not be easy to achieve if the containers are provisioned automatically.

I don't understand this, just bake it into the provisioning.

@wwentland
Copy link

wwentland commented Jan 9, 2017

We abhor silent failures. Either it succeeds, or it fails and you know about it and you take steps to correct it.

Sure, but the service start should (and would) fail rather than the package installation itself.

I don't understand this, just bake it into the provisioning.

Not all provisioning tools allow you to easily define environment variables for package installations and it would be better to rely on the debconf database for ES_SKIP_SET_KERNEL_PARAMETERS if you really have to do this.

Using debconf would be in line with Debian policy [0] and is easy to preseed in every provisioning system that I'm familiar with.

[0] https://www.debian.org/doc/debian-policy/ch-binary.html#s-maintscriptprompt

@jasontedor
Copy link
Member

Sure, but the service start should (and would) fail rather than the package installation itself.

We also abhor late failures.

Not all provisioning tools allow you to easily define environment variables for package installations

Such as?

@wwentland
Copy link

wwentland commented Jan 11, 2017

Sure, but the service start should (and would) fail rather than the package installation itself.

We also abhor late failures.

Failing during the postinst phase of a package installation is, to put it mildly, a rather unfortunate thing to do. I totally understand your motivation of making the ES installation as easy for people as possible, but I would recommend to solve these issues in a standard manner as it allows for these packages to integrate well with the rest of the ecosystem.

I mentioned the standard solution that is being advocated/mandated in the Debian policy in cases like this, namely debconf and would still recommend to adopt it.

Not all provisioning tools allow you to easily define environment variables for package installations

Such as?

I was thinking of Saltstack here (despite environ.setenv), but does it really matter? Having to define an environment variable before a package installation is a rather unusual thing to do and should, in my humble opinion, be avoided in favour of more standard approaches.

I'd also like to mention #8793 again which seems to have eluded your attention.

@jasontedor
Copy link
Member

I was thinking of Saltstack here (despite environ.setenv), but does it really matter?

It absolutely matters. You are trying to convince us to take another point of view, and in support of that you put out a statement:

Not all provisioning tools allow you to easily define environment variables for package installations

I'm asking for additional context. If, for example, the tool that you had in mind was an obscure tool then I would be less likely to consider your argument as we do not design for special cases.

I'd also like to mention #8793 again which seems to have eluded your attention.

It did not elude my attention. I've already read it and considered it. It's a two-year old pull request. The philosophy of the maintainers of this project has evolved considerably in that period (for example, to become less lenient).

Having to define an environment variable before a package installation is a rather unusual thing to do and should, in my humble opinion, be avoided in favour of more standard approaches.

Maybe it is unusual (although it is done, cf. SUDO_FORCE_REMOVE on sudo-ldap), but it sure is easy to understand and it's consistent with what we already do.

@wwentland
Copy link

Having to define an environment variable before a package installation is a rather unusual thing to do and should, in my humble opinion, be avoided in favour of more standard approaches.

Maybe it is unusual (although it is done, cf. SUDO_FORCE_REMOVE on sudo-ldap), but it sure is easy to understand and it's consistent with what we already do.

What exactly is consistent with what you already do? I don't quite understand what the problem is with adopting debconf for this. It would be in line with Debian policy, user expectations and it is well supported in all provisioning systems I have worked with.

@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts discuss Team:Delivery Meta label for Delivery team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants