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

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

Merged
merged 5 commits into from
Oct 16, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4ed3192
use tempdir within data
michalpristas Oct 16, 2020
f8a5ad7
lazy mkdir
michalpristas Oct 16, 2020
ce9554c
fmt
michalpristas Oct 16, 2020
6865dda
changelog
michalpristas Oct 16, 2020
f9549de
Update CHANGELOG.next.asciidoc
michalpristas Oct 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/elastic-agent/CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- Include inputs in action store actions {pull}21298[21298]
- Fix issue where inputs without processors defined would panic {pull}21628[21628]
- Partial extracted beat result in failure to spawn beat {issue}21718[21718]
- Use local temp instead of system one {pull}21883[21883]

==== New features

Expand Down
16 changes: 16 additions & 0 deletions x-pack/elastic-agent/pkg/agent/application/paths/paths.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,20 @@ import (
"os"
"path/filepath"
"strings"
"sync"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/release"
)

const (
tempSubdir = "tmp"
)

var (
topPath string
configPath string
logsPath string
tmpCreator sync.Once
)

func init() {
Expand All @@ -37,6 +43,16 @@ func Top() string {
return topPath
}

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

// create tempdir as it probably don't exists
os.MkdirAll(tmpDir, 0750)
})
return tmpDir
}

// Home returns a directory where binary lives
func Home() string {
return versionedHome(topPath)
Expand Down
4 changes: 3 additions & 1 deletion x-pack/elastic-agent/pkg/artifact/install/atomic/atomic_installer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"io/ioutil"
"os"
"path/filepath"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application/paths"
)

type embeddedInstaller interface {
Expand All @@ -31,7 +33,7 @@ func NewInstaller(i embeddedInstaller) (*Installer, error) {
// Install performs installation of program in a specific version.
func (i *Installer) Install(ctx context.Context, programName, version, installDir string) error {
// tar installer uses Dir of installDir to determine location of unpack
tempDir, err := ioutil.TempDir(os.TempDir(), "elastic-agent-install")
tempDir, err := ioutil.TempDir(paths.TempDir(), "elastic-agent-install")
if err != nil {
return err
}
Expand Down
6 changes: 4 additions & 2 deletions x-pack/elastic-agent/pkg/artifact/install/atomic/atomic_installer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/application/paths"
)

func TestOKInstall(t *testing.T) {
Expand All @@ -25,7 +27,7 @@ func TestOKInstall(t *testing.T) {
assert.NoError(t, err)

ctx := context.Background()
installDir := filepath.Join(os.TempDir(), "install_dir")
installDir := filepath.Join(paths.TempDir(), "install_dir")

wg.Add(1)
go func() {
Expand Down Expand Up @@ -59,7 +61,7 @@ func TestContextCancelledInstall(t *testing.T) {
assert.NoError(t, err)

ctx, cancel := context.WithCancel(context.Background())
installDir := filepath.Join(os.TempDir(), "install_dir")
installDir := filepath.Join(paths.TempDir(), "install_dir")

wg.Add(1)
go func() {
Expand Down