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.

Sign up for GitHub

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

Jump to bottom

Add ulimit setting to debian containers in mage build #32580

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Add ulimit setting to debian containers in mage build #32580

merged 2 commits into from
Aug 2, 2022

Conversation

fearful-symmetry
Copy link
Contributor

What does this PR do?

After a considerable amount of googling and strace yesterday, I learned that there's a bug in certain older versions of apt-get in debian, where it'll try to iterate through all possible FDs as set by internal linux system limits. This results in some amusing strace output:

fcntl(337334262, F_SETFD, FD_CLOEXEC)    = -1 EBADF (Bad file descriptor)

This only happens on certain docker installs where the docker daemon won't set the NOFILE ulimit in its containers.

This fix just adds a ulimit flag to the docker invocation when needed.

Why is it important?

On systems that hit this bug, mage package on filebeat can take up to an hour, purely due to the apt-get update operation.

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 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.

@fearful-symmetry fearful-symmetry added bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Aug 2, 2022
@fearful-symmetry fearful-symmetry requested a review from a team as a code owner August 2, 2022 17:31
@fearful-symmetry fearful-symmetry self-assigned this Aug 2, 2022
@fearful-symmetry fearful-symmetry requested review from rdner and cmacknz and removed request for a team August 2, 2022 17:31
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Aug 2, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 2, 2022 •
edited by jenkins-beats-ci bot
Loading

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-08-02T17:32:39.980+0000

  • Duration: 96 min 42 sec

Test stats 🧪

Test Results
Failed 0
Passed 22536
Skipped 1937
Total 24473

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

cmacknz
cmacknz reviewed Aug 2, 2022
View reviewed changes
CHANGELOG.next.asciidoc
@@ -55,6 +55,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff]

- Fix counter for number of events published in `httpjson` input. {pull}31993[31993]
- Fix handling of Checkpoint event for R81. {issue}32380[32380] {pull}32458[32458]
- Fix a hang on `apt-get update` stage in packaging. {pull}32580[32580]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Fix a hang on `apt-get update` stage in packaging. {pull}32580[32580]

I don't think we need a changelog entry for this because it isn't user facing. This could go in the developer changelog though.

@cmacknz
Copy link
Member

cmacknz commented Aug 2, 2022

@AndersonQ this might be the root cause of the issue I remember you experiencing some time ago with apt-get update taking forever.

@ph it is likely worth porting this to the agent repository.

@fearful-symmetry fearful-symmetry merged commit ee6f470 into elastic:main Aug 2, 2022
@fearful-symmetry fearful-symmetry mentioned this pull request Aug 2, 2022
Merged
andrewvc pushed a commit to andrewvc/beats that referenced this pull request Aug 3, 2022
@fearful-symmetry @andrewvc
Add ulimit setting to debian containers in mage build (elastic#32580)
87d3930 
* add ulimit to debian containers

* add changelog
@belimawr
Copy link
Contributor

belimawr commented Aug 3, 2022

I believe we should backport this to 7.17 as well.

@belimawr belimawr added the backport-7.17 Automated backport to the 7.17 branch with mergify label Aug 3, 2022
@mergify mergify bot mentioned this pull request Aug 3, 2022
Closed
mergify bot pushed a commit that referenced this pull request Aug 3, 2022
@fearful-symmetry @mergify
Add ulimit setting to debian containers in mage build (#32580)
4e8e8fc 
* add ulimit to debian containers

* add changelog

(cherry picked from commit ee6f470)
@cmacknz cmacknz added backport-v8.3.0 Automated backport with mergify backport-v8.4.0 Automated backport with mergify labels Aug 3, 2022
@cmacknz
Copy link
Member

cmacknz commented Aug 3, 2022

Also backport to 8.3 and 8.4

mergify bot pushed a commit that referenced this pull request Aug 3, 2022
@fearful-symmetry @mergify
Add ulimit setting to debian containers in mage build (#32580)
eef2fcd 
* add ulimit to debian containers

* add changelog

(cherry picked from commit ee6f470)
@mergify mergify bot mentioned this pull request Aug 3, 2022
Merged
mergify bot pushed a commit that referenced this pull request Aug 3, 2022
@fearful-symmetry @mergify
Add ulimit setting to debian containers in mage build (#32580)
c866bdb 
* add ulimit to debian containers

* add changelog

(cherry picked from commit ee6f470)
@mergify mergify bot mentioned this pull request Aug 3, 2022
Closed
@ph
Copy link
Contributor

ph commented Aug 3, 2022

I will make the same changes on elastic agent, it use the same code but the file aren't shared yet.

@ph ph mentioned this pull request Aug 3, 2022
Closed
5 tasks
@ph
Copy link
Contributor

ph commented Aug 3, 2022

see elastic/elastic-agent#849

@florianl florianl mentioned this pull request Aug 4, 2022
Merged
6 tasks
gpop63 pushed a commit that referenced this pull request Aug 24, 2022
@mergify @fearful-symmetry @rdner
[8.3](backport #32580) Add ulimit setting to debian containers in mag…
2dfa258 
…e build (#32590)

* Add ulimit setting to debian containers in mage build (#32580)

* add ulimit to debian containers

* add changelog

(cherry picked from commit ee6f470)

* Fix the changelog

Co-authored-by: Alex K <[email protected]>
Co-authored-by: Denis Rechkunov <[email protected]>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
@fearful-symmetry @chrisberkhout
Add ulimit setting to debian containers in mage build (#32580)
5d5263d 
* add ulimit to debian containers

* add changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmacknz cmacknz cmacknz approved these changes

@rdner rdner Awaiting requested review from rdner rdner is a code owner automatically assigned from elastic/elastic-agent-data-plane

Assignees

@fearful-symmetry fearful-symmetry

Labels
backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.3.0 Automated backport with mergify backport-v8.4.0 Automated backport with mergify bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fearful-symmetry @elasticmachine @cmacknz @belimawr @ph