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

[Elastic Agent] Fix agent control socket path to always be less than 107 characters #20426

Merged
merged 3 commits into from
Aug 4, 2020
Merged

[Elastic Agent] Fix agent control socket path to always be less than 107 characters #20426

merged 3 commits into from
Aug 4, 2020

Conversation

blakerouse
Copy link
Contributor

What does this PR do?

Instead of placing the control socket inside the paths.Data() it moves it to be under /tmp/elastic-agent-{$ hash_of(paths.Data()) $}.sock. That ensure that the path will always be less than 107 characters, but be unique per Elastic Agent data path.

I was original could to change it to the /run directory but that has permission issues, that would always required Elastic Agent to run as root, and we do not want to make that a requirement.

Why is it important?

So that running Elastic Agent when it is nested deep into multiple directories doesn't cause an issue with the socket path being longer than 107 characters.

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.

@blakerouse blakerouse self-assigned this Aug 4, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest-management (Team:Ingest Management)

@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 4, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 4, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20426 updated]

  • Start Time: 2020-08-04T13:50:25.512+0000

  • Duration: 31 min 1 sec

Steps errors

Expand to view the steps failures

  • Name: Install Go 1.14.4

    • Description: .ci/scripts/install-go.sh

    • Duration: 1 min 32 sec

    • Start Time: 2020-08-04T14:18:48.423+0000

    • log

michalpristas
michalpristas approved these changes Aug 4, 2020
View reviewed changes
Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

i think this should be fine,
tmp should be there but in case it is not maybe it would be better to use os.TempDir

@blakerouse
Copy link
Contributor Author

os.TempDir opens up for the case that the path will be greater than 107 characters. I hit that as soon as I changed it to use os.TempDir as my IDE set the temporary directory to a longer path and broke the unit test.

/tmp is a standard UNIX path and should be there on Linux and Mac.

@michalpristas michalpristas mentioned this pull request Aug 4, 2020
Merged
3 tasks
@blakerouse blakerouse merged commit b1bd7b7 into elastic:master Aug 4, 2020
@blakerouse blakerouse deleted the agent-control-fix-socket-path branch August 4, 2020 14:35
blakerouse added a commit to blakerouse/beats that referenced this pull request Aug 4, 2020
@blakerouse
[Elastic Agent] Fix agent control socket path to always be less than …
7fa2a87 
…107 characters (elastic#20426)

* Fix agent control socket path to always be less than 107 characters.

* Use os.TempDir.

* Don't use os.TempDir.
@blakerouse blakerouse mentioned this pull request Aug 4, 2020
Merged
4 tasks
blakerouse added a commit that referenced this pull request Aug 4, 2020
@blakerouse
Cherry-pick #20359 to 7.x: [Elastic Agent] Improve version, restart, …
9615f41 
…enroll CLI commands (#20431)

* [Elastic Agent] Improve version, restart, enroll CLI commands (#20359)

* Add improve version CLI cmd.

* Add new restart cmd. Perform restart at end of enroll.

* Fix yaml annotations on version struct.

* Fix control.Address on Windows.

* Fix control.Address on Windows.

* Fix windows dialer.

* Fix control.Address on Windows.

* Add to CHANGELOG.

* Review cleanups.

* Fix go vet.

* Update talking to communicating.

(cherry picked from commit 77b3b07)

* [Elastic Agent] Fix agent control socket path to always be less than 107 characters (#20426)

* Fix agent control socket path to always be less than 107 characters.

* Use os.TempDir.

* Don't use os.TempDir.
v1v added a commit to v1v/beats that referenced this pull request Aug 6, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/ci-pipeli…
6fa6d3a 
…ne-2.0

* upstream/master:
  [docs] Promote ingest management to beta (elastic#20295)
  Upgrade elasticsearch client library used in tests (elastic#20405)
  Disable logging when pulling on python integration tests (elastic#20397)
  Remove pillow from testing requirements.txt (elastic#20407)
  [Filebeat][ATP Module]Setting user agent field required by the API (elastic#20440)
  [Ingest Manager] Send datastreams fields (elastic#20402)
  Add event.ingested to all Filebeat modules (elastic#20386)
  [Elastic Agent] Fix agent control socket path to always be less than 107 characters (elastic#20426)
  Improve cgroup_regex docs with examples (elastic#20425)
  Makes `metrics` config option required in app_insights (elastic#20406)
  Ensure install scripts only install if needed (elastic#20349)
  Update container name for the azure filesets (elastic#19899)
  Group same timestamp metrics values in app_insights metricset (elastic#20403)
  add_process_metadata processor adds container id even if process metadata not accessible (elastic#19767)
  Support "cluster" scope in Metricbeat elasticsearch module (elastic#18547)
  [Filebeat][SophosXG Module] Renaming module and fileset (elastic#20396)
  Update Suricata dashboards (elastic#20394)
  [Elastic Agent] Improve version, restart, enroll CLI commands (elastic#20359)
  Prepare home directories for docker images in a different stage (elastic#20356)
v1v added a commit to v1v/beats that referenced this pull request Aug 6, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into fix/ci-tools-inst…
6b99d98 
…allation

* upstream/master: (23 commits)
  [docs] Promote ingest management to beta (elastic#20295)
  Upgrade elasticsearch client library used in tests (elastic#20405)
  Disable logging when pulling on python integration tests (elastic#20397)
  Remove pillow from testing requirements.txt (elastic#20407)
  [Filebeat][ATP Module]Setting user agent field required by the API (elastic#20440)
  [Ingest Manager] Send datastreams fields (elastic#20402)
  Add event.ingested to all Filebeat modules (elastic#20386)
  [Elastic Agent] Fix agent control socket path to always be less than 107 characters (elastic#20426)
  Improve cgroup_regex docs with examples (elastic#20425)
  Makes `metrics` config option required in app_insights (elastic#20406)
  Ensure install scripts only install if needed (elastic#20349)
  Update container name for the azure filesets (elastic#19899)
  Group same timestamp metrics values in app_insights metricset (elastic#20403)
  add_process_metadata processor adds container id even if process metadata not accessible (elastic#19767)
  Support "cluster" scope in Metricbeat elasticsearch module (elastic#18547)
  [Filebeat][SophosXG Module] Renaming module and fileset (elastic#20396)
  Update Suricata dashboards (elastic#20394)
  [Elastic Agent] Improve version, restart, enroll CLI commands (elastic#20359)
  Prepare home directories for docker images in a different stage (elastic#20356)
  New multiline mode in Filebeat: while_pattern (elastic#19662)
  ...
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
@blakerouse @melchiormoulin
[Elastic Agent] Fix agent control socket path to always be less than …
d41587c 
…107 characters (elastic#20426)

* Fix agent control socket path to always be less than 107 characters.

* Use os.TempDir.

* Don't use os.TempDir.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalpristas michalpristas michalpristas approved these changes

Assignees

@blakerouse blakerouse

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@blakerouse @elasticmachine @michalpristas