[Ingest Manager] Use local temp instead of system one #21883

michalpristas · 2020-10-16T07:54:20Z

What does this PR do?

Using system temp dir might result sometimes in cross-device linking. This should make it ok.

Why is it important?

Fixes: #21860

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.

elasticmachine · 2020-10-16T07:54:22Z

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

ruflin

Change LGTM. I stumbled over the part that it inside the data directory but not sure having it at the top level is better. My assumption is that this temp dir is not needed across restart, so it should not be a problem if we change the location in the future.

ruflin · 2020-10-16T07:57:14Z

x-pack/elastic-agent/pkg/agent/application/paths/paths.go

@@ -29,6 +33,9 @@ func init() {
 	fs.StringVar(&topPath, "path.home", topPath, "Agent root path")
 	fs.StringVar(&configPath, "path.config", configPath, "Config path is the directory Agent looks for its config file")
 	fs.StringVar(&logsPath, "path.logs", logsPath, "Logs path contains Agent log output")
+
+	// create tempdir as it probably don't exists
+	os.MkdirAll(TempDir(), 0750)


Should this TempDir be wiped on startup to make sure it is always "clean"?

we can wipe it but ideally whoever uses it cleans after work is done so we dont have to wait until restart and we free resources

elasticmachine · 2020-10-16T08:02:42Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: [Pull request #21883 updated]
Start Time: 2020-10-16T08:49:23.790+0000
Duration: 40 min 19 sec

Test stats 🧪

Test	Results
Failed	0
Passed	1394
Skipped	4
Total	1398

ruflin · 2020-10-16T10:00:53Z

x-pack/elastic-agent/pkg/agent/application/paths/paths.go

+// TempDir returns agent temp dir located within data dir.
+func TempDir() string {
+	tmpDir := filepath.Join(Data(), tempSubdir)
+	tmpCreator.Do(func() {


Not sure in which case this is executed multiple times? Tests?

tempDir is called on each install, i want to avoid Mkdir to be executed multiple times

The two things I'm trying to understand:

What is the problem of executing it multiple times

Do we have a case where this might happen

My goal is to understand what the driver for this is. I don't see any issue with the code itself.

Actually I think it would be better to call MkdirAll every time. If it already exists it will do nothing, but it also ensures that before going further in this code path it does exist.

The user might have gone in and deleted or something while Agent is already running, so its better to just make sure its there before going any further.

@michalpristas you might have to follow up on that as you just merged ^

[Ingest Manager] Use local temp instead of system one (elastic#21883)

* upstream/master: (23 commits) [Ingest Manager] Prevent reporting ecs version twice (elastic#21616) [CI] Use google storage to keep artifacts (elastic#21910) Update docs.asciidoc (elastic#21849) Kubernetes leaderelection improvements (elastic#21896) Apply name changes to elastic agent docs (elastic#21549) Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941) [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930) Refactor docker watcher to fix flaky test and other small issues (elastic#21851) [CI] Add stage name in the step (elastic#21887) [docs] Remove extra word in autodiscover docs (elastic#21871) [CI] lint stage doesn't produce test reports (elastic#21888) Add tests of reader of filestream input (elastic#21814) [Ingest Manager] Use local temp instead of system one (elastic#21883) chore: delegate variant pushes to the right method (elastic#21861) [CI] kind setup fails sometimes (elastic#21857) Fix panic on add_docker_metadata close (elastic#21882) Add tests for fileProspector in filestream input (elastic#21712) [Filebeat][okta] Fix okta pagination (elastic#21797) Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776) Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854) ...

…laky-test-analyser * upstream/master: (22 commits) [Ingest Manager] Prevent reporting ecs version twice (elastic#21616) [CI] Use google storage to keep artifacts (elastic#21910) Update docs.asciidoc (elastic#21849) Kubernetes leaderelection improvements (elastic#21896) Apply name changes to elastic agent docs (elastic#21549) Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941) [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930) Refactor docker watcher to fix flaky test and other small issues (elastic#21851) [CI] Add stage name in the step (elastic#21887) [docs] Remove extra word in autodiscover docs (elastic#21871) [CI] lint stage doesn't produce test reports (elastic#21888) Add tests of reader of filestream input (elastic#21814) [Ingest Manager] Use local temp instead of system one (elastic#21883) chore: delegate variant pushes to the right method (elastic#21861) [CI] kind setup fails sometimes (elastic#21857) Fix panic on add_docker_metadata close (elastic#21882) Add tests for fileProspector in filestream input (elastic#21712) [Filebeat][okta] Fix okta pagination (elastic#21797) Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776) Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854) ...

…-store-in-another-location-too * upstream/master: [Ingest Manager] Prevent reporting ecs version twice (elastic#21616) [CI] Use google storage to keep artifacts (elastic#21910) Update docs.asciidoc (elastic#21849) Kubernetes leaderelection improvements (elastic#21896) Apply name changes to elastic agent docs (elastic#21549) Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941) [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930) Refactor docker watcher to fix flaky test and other small issues (elastic#21851) [CI] Add stage name in the step (elastic#21887) [docs] Remove extra word in autodiscover docs (elastic#21871) [CI] lint stage doesn't produce test reports (elastic#21888) Add tests of reader of filestream input (elastic#21814) [Ingest Manager] Use local temp instead of system one (elastic#21883)

[Ingest Manager] Use local temp instead of system one (#21883)

use tempdir within data

4ed3192

michalpristas added bug Team:Ingest Management Ingest Management:beta2 Group issues for ingest management beta2 labels Oct 16, 2020

michalpristas self-assigned this Oct 16, 2020

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 Oct 16, 2020

ruflin approved these changes Oct 16, 2020

View reviewed changes

lazy mkdir

f8a5ad7

michalpristas added 3 commits October 16, 2020 10:44

fmt

ce9554c

changelog

6865dda

Update CHANGELOG.next.asciidoc

f9549de

ruflin reviewed Oct 16, 2020

View reviewed changes

ruflin approved these changes Oct 16, 2020

View reviewed changes

michalpristas merged commit bb79569 into elastic:master Oct 16, 2020

michalpristas added a commit to michalpristas/beats that referenced this pull request Oct 16, 2020

[Ingest Manager] Use local temp instead of system one (elastic#21883)

81c63c4

[Ingest Manager] Use local temp instead of system one (elastic#21883)

michalpristas added a commit to michalpristas/beats that referenced this pull request Oct 16, 2020

[Ingest Manager] Use local temp instead of system one (elastic#21883)

17a44b0

[Ingest Manager] Use local temp instead of system one (elastic#21883)

michalpristas added the needs_backport PR is waiting to be backported to other branches. label Oct 16, 2020

This was referenced Oct 16, 2020

Cherry-pick #21883 to 7.x: Use local temp instead of system one #21905

Merged

Cherry-pick #21883 to 7.10: Use local temp instead of system one #21906

Merged

michalpristas added a commit that referenced this pull request Oct 21, 2020

[Ingest Manager] Use local temp instead of system one (#21883) (#21906)

0bd97c9

[Ingest Manager] Use local temp instead of system one (#21883)

michalpristas added a commit that referenced this pull request Oct 21, 2020

[Ingest Manager] Use local temp instead of system one (#21883) (#21905)

2869ca6

[Ingest Manager] Use local temp instead of system one (#21883)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest Manager] Use local temp instead of system one #21883

[Ingest Manager] Use local temp instead of system one #21883

michalpristas commented Oct 16, 2020 •

edited

Loading

elasticmachine commented Oct 16, 2020

ruflin left a comment

ruflin Oct 16, 2020

michalpristas Oct 16, 2020

elasticmachine commented Oct 16, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

ruflin Oct 16, 2020

michalpristas Oct 16, 2020

ruflin Oct 16, 2020

blakerouse Oct 16, 2020

ruflin Oct 16, 2020

[Ingest Manager] Use local temp instead of system one #21883

[Ingest Manager] Use local temp instead of system one #21883

Conversation

michalpristas commented Oct 16, 2020 • edited Loading

What does this PR do?

Why is it important?

Checklist

elasticmachine commented Oct 16, 2020

ruflin left a comment

Choose a reason for hiding this comment

ruflin Oct 16, 2020

Choose a reason for hiding this comment

michalpristas Oct 16, 2020

Choose a reason for hiding this comment

elasticmachine commented Oct 16, 2020 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

ruflin Oct 16, 2020

Choose a reason for hiding this comment

michalpristas Oct 16, 2020

Choose a reason for hiding this comment

ruflin Oct 16, 2020

Choose a reason for hiding this comment

blakerouse Oct 16, 2020

Choose a reason for hiding this comment

ruflin Oct 16, 2020

Choose a reason for hiding this comment

michalpristas commented Oct 16, 2020 •

edited

Loading

elasticmachine commented Oct 16, 2020 •

edited by jenkins-beats-ci bot

Loading