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

Introduce proper init system (/bin/tini) to Docker container #22137

Merged
merged 6 commits into from
Dec 8, 2020

Conversation

mieciu
Copy link
Contributor

@mieciu mieciu commented Oct 26, 2020

What does this PR do?

🚧 Work in progress, there's probably more to come

This PR introduces init system (tini - ref: https://github.com/krallin/tini) for reaping zombie processes and proper signal forwarding. It also enables running processes with non-default UID/GID.

Why is it important?

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 26, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 26, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #22137 updated

  • Start Time: 2020-12-07T15:35:58.815+0000

  • Duration: 79 min 22 sec

Test stats 🧪

Test Results
Failed 0
Passed 17337
Skipped 1373
Total 18710

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 17337
Skipped 1373
Total 18710

@andresrc andresrc added the Team:Integrations Label for the Integrations team label Oct 26, 2020
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 26, 2020
@kuisathaverat kuisathaverat added needs_backport PR is waiting to be backported to other branches. v7.10.0 v7.11.0 v7.9.3 labels Oct 27, 2020
@kuisathaverat
Copy link
Contributor

kuisathaverat commented Oct 27, 2020

I can backport the change when it is merged, we use backport

backport --pr 22137 --branch 7.x --branch 7.10 --branch 7.9

@mieciu mieciu closed this Dec 4, 2020
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
@jsoriano
Copy link
Member

jsoriano commented Dec 7, 2020

/package

@jsoriano
Copy link
Member

jsoriano commented Dec 7, 2020

/package

@jsoriano
Copy link
Member

jsoriano commented Dec 7, 2020

@mieciu package tests are failing, with this error:

[2020-12-07T10:21:50.602Z] >> Testing package contents
[2020-12-07T10:22:02.839Z] --- FAIL: TestDocker (3.70s)
[2020-12-07T10:22:02.839Z]     --- FAIL: TestDocker/auditbeat-oss-8.0.0-SNAPSHOT-linux-amd64.docker.tar.gz_entrypoint (0.00s)
[2020-12-07T10:22:02.839Z]         package_test.go:412: bin/tini entrypoint not found in docker
[2020-12-07T10:22:02.839Z] FAIL
[2020-12-07T10:22:02.839Z] FAIL	command-line-arguments	9.065s
[2020-12-07T10:22:02.839Z] FAIL

I think this is happening because /bin is a link to /usr/bin, so tini is actually in /usr/bin/tini in the docker layer.

Could you please replace appearances of /bin/tini path with /usr/bin/tini?

Thanks!

(Another solution could be to make tests aware of symlinks, but not sure how hard this would be as we are reading the layers as tar files).

@mieciu
Copy link
Contributor Author

mieciu commented Dec 7, 2020

/package

@mieciu mieciu marked this pull request as ready for review December 7, 2020 14:41
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
@jsoriano
Copy link
Member

jsoriano commented Dec 7, 2020

/package

@jsoriano
Copy link
Member

jsoriano commented Dec 7, 2020

LGTM, but lets wait for a last CI run. Thanks @mieciu!

@jsoriano
Copy link
Member

jsoriano commented Dec 8, 2020

Ignoring failure in E2E tests by now, they are passing in master, but they are failing in other PRs too, like in this one: #22879.

@mdelapenya could be something wrong with E2E tests for PRs? Master and nightly builds seem to be working fine.

@jsoriano jsoriano merged commit ee50020 into elastic:master Dec 8, 2020
@jsoriano jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Dec 8, 2020
@jsoriano
Copy link
Member

jsoriano commented Dec 8, 2020

I am backporting this to 7.x so it will be in 7.11 (#22982).

@kuisathaverat I see you also added labels for 7.10 and 7.9. Is this change needed in these versions? I would prefer not to make this change on versions already released.

jsoriano added a commit to jsoriano/beats that referenced this pull request Dec 8, 2020
jsoriano pushed a commit to jsoriano/beats that referenced this pull request Dec 8, 2020
Introduce tini as init system for Docker containers. Tini is the init system
used in docker images of other Elastic products.
It implements basic init functionality as reaping zombie processes and
properly handle signals.

(cherry picked from commit ee50020)
@jsoriano
Copy link
Member

jsoriano commented Dec 8, 2020

I forgot adding a changelog, adding it here: #22983

jsoriano added a commit that referenced this pull request Dec 8, 2020
jsoriano added a commit that referenced this pull request Dec 8, 2020
Introduce tini as init system for Docker containers. Tini is the init system
used in docker images of other Elastic products.
It implements basic init functionality as reaping zombie processes and
properly handle signals.

(cherry picked from commit ee50020)

Co-authored-by: Przemysław Hejman <przemyslaw.hejman@elastic.co>
@jsoriano jsoriano added the test-plan Add this PR to be manual test plan label Dec 10, 2020
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.9.3 v7.10.0 v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants