From e7a63f10c8d9f9e44b40754bfe9d44f11ca6cabc Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 25 May 2021 12:47:11 +0200 Subject: [PATCH 1/6] log level small optim --- .../pkg/agent/application/application.go | 3 +- .../gateway/fleet/fleet_gateway_test.go | 13 +- .../pkg/agent/application/info/agent_id.go | 12 +- .../pkg/agent/application/info/agent_info.go | 13 +- .../agent/application/info/agent_metadata.go | 2 +- .../handlers/handler_action_settings.go | 2 +- .../emitter/modifiers/fleet_decorator.go | 8 +- .../elastic-agent/pkg/agent/cmd/enroll_cmd.go | 4 +- x-pack/elastic-agent/pkg/agent/cmd/run.go | 5 +- .../pkg/agent/storage/disk_store.go | 95 ++++++++ .../pkg/agent/storage/handler_store.go | 24 +++ .../pkg/agent/storage/null_store.go | 15 ++ .../pkg/agent/storage/replace_store.go | 106 +++++++++ .../pkg/agent/storage/storage.go | 202 ------------------ .../pkg/agent/storage/sync_on_save_store.go | 77 +++++++ .../elastic-agent/pkg/core/logger/logger.go | 5 +- 16 files changed, 360 insertions(+), 226 deletions(-) create mode 100644 x-pack/elastic-agent/pkg/agent/storage/disk_store.go create mode 100644 x-pack/elastic-agent/pkg/agent/storage/handler_store.go create mode 100644 x-pack/elastic-agent/pkg/agent/storage/null_store.go create mode 100644 x-pack/elastic-agent/pkg/agent/storage/replace_store.go create mode 100644 x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go diff --git a/x-pack/elastic-agent/pkg/agent/application/application.go b/x-pack/elastic-agent/pkg/agent/application/application.go index 53c74afabe50..c590cad670f0 100644 --- a/x-pack/elastic-agent/pkg/agent/application/application.go +++ b/x-pack/elastic-agent/pkg/agent/application/application.go @@ -96,7 +96,8 @@ func createApplication( func mergeFleetConfig(rawConfig *config.Config) (storage.Store, *configuration.Configuration, error) { path := paths.AgentConfigFile() - store := storage.NewDiskStore(path) + diskStore := storage.NewDiskStore(path) + store := storage.NewWindowsSyncOnSaveStore(diskStore, path) reader, err := store.Load() if err != nil { return store, nil, errors.New(err, "could not initialize config store", diff --git a/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go b/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go index 28ffa9dd76eb..c711a1acbb9e 100644 --- a/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go +++ b/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go @@ -124,7 +124,9 @@ func withGateway(agentInfo agentInfo, settings *fleetGatewaySettings, fn withGat ctx, cancel := context.WithCancel(context.Background()) defer cancel() - stateStore, err := store.NewStateStore(log, storage.NewDiskStore(paths.AgentStateStoreFile())) + diskStore := storage.NewDiskStore(paths.AgentStateStoreFile()) + syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, paths.AgentStateStoreFile()) + stateStore, err := store.NewStateStore(log, syncStore) require.NoError(t, err) gateway, err := newFleetGatewayWithScheduler( @@ -253,7 +255,10 @@ func TestFleetGateway(t *testing.T) { defer cancel() log, _ := logger.New("tst", false) - stateStore, err := store.NewStateStore(log, storage.NewDiskStore(paths.AgentStateStoreFile())) + + diskStore := storage.NewDiskStore(paths.AgentStateStoreFile()) + syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, paths.AgentStateStoreFile()) + stateStore, err := store.NewStateStore(log, syncStore) require.NoError(t, err) gateway, err := newFleetGatewayWithScheduler( @@ -344,7 +349,9 @@ func TestFleetGateway(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) log, _ := logger.New("tst", false) - stateStore, err := store.NewStateStore(log, storage.NewDiskStore(paths.AgentStateStoreFile())) + diskStore := storage.NewDiskStore(paths.AgentStateStoreFile()) + syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, paths.AgentStateStoreFile()) + stateStore, err := store.NewStateStore(log, syncStore) require.NoError(t, err) gateway, err := newFleetGatewayWithScheduler( diff --git a/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go b/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go index b0011bd7352a..6c65afac38d1 100644 --- a/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go +++ b/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go @@ -51,10 +51,11 @@ func updateLogLevel(level string) error { } agentConfigFile := paths.AgentConfigFile() - s := storage.NewDiskStore(agentConfigFile) + diskStore := storage.NewDiskStore(agentConfigFile) + syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, agentConfigFile) ai.LogLevel = level - return updateAgentInfo(s, ai) + return updateAgentInfo(syncStore, ai) } func generateAgentID() (string, error) { @@ -191,9 +192,10 @@ func loadAgentInfo(forceUpdate bool, logLevel string, createAgentID bool) (*pers defer idLock.Unlock() agentConfigFile := paths.AgentConfigFile() - s := storage.NewDiskStore(agentConfigFile) + diskStore := storage.NewDiskStore(agentConfigFile) + syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, agentConfigFile) - agentinfo, err := getInfoFromStore(s, logLevel) + agentinfo, err := getInfoFromStore(syncStore, logLevel) if err != nil { return nil, err } @@ -202,7 +204,7 @@ func loadAgentInfo(forceUpdate bool, logLevel string, createAgentID bool) (*pers return agentinfo, nil } - if err := updateID(agentinfo, s); err != nil { + if err := updateID(agentinfo, syncStore); err != nil { return nil, err } diff --git a/x-pack/elastic-agent/pkg/agent/application/info/agent_info.go b/x-pack/elastic-agent/pkg/agent/application/info/agent_info.go index ff0b1d8263bf..8ae09c2efc35 100644 --- a/x-pack/elastic-agent/pkg/agent/application/info/agent_info.go +++ b/x-pack/elastic-agent/pkg/agent/application/info/agent_info.go @@ -5,6 +5,7 @@ package info import ( + "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/core/logger" "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/release" ) @@ -41,8 +42,16 @@ func NewAgentInfo(createAgentID bool) (*AgentInfo, error) { return NewAgentInfoWithLog(defaultLogLevel, createAgentID) } -// LogLevel updates log level of agent. -func (i *AgentInfo) LogLevel(level string) error { +// LogLevel retrieves a log level. +func (i *AgentInfo) LogLevel() string { + if i.logLevel == "" { + return logger.DefaultLogLevel.String() + } + return i.logLevel +} + +// SetLogLevel updates log level of agent. +func (i *AgentInfo) SetLogLevel(level string) error { if err := updateLogLevel(level); err != nil { return err } diff --git a/x-pack/elastic-agent/pkg/agent/application/info/agent_metadata.go b/x-pack/elastic-agent/pkg/agent/application/info/agent_metadata.go index c1b4f4055ee2..11c784d59238 100644 --- a/x-pack/elastic-agent/pkg/agent/application/info/agent_metadata.go +++ b/x-pack/elastic-agent/pkg/agent/application/info/agent_metadata.go @@ -158,7 +158,7 @@ func (i *AgentInfo) ECSMetadata() (*ECSMeta, error) { // only upgradeable if running from Agent installer and running under the // control of the system supervisor (or built specifically with upgrading enabled) Upgradeable: release.Upgradeable() || (RunningInstalled() && RunningUnderSupervisor()), - LogLevel: i.logLevel, + LogLevel: i.LogLevel(), }, }, Host: &HostECSMeta{ diff --git a/x-pack/elastic-agent/pkg/agent/application/pipeline/actions/handlers/handler_action_settings.go b/x-pack/elastic-agent/pkg/agent/application/pipeline/actions/handlers/handler_action_settings.go index 17b0b8ac4a0d..e45ef26724fd 100644 --- a/x-pack/elastic-agent/pkg/agent/application/pipeline/actions/handlers/handler_action_settings.go +++ b/x-pack/elastic-agent/pkg/agent/application/pipeline/actions/handlers/handler_action_settings.go @@ -51,7 +51,7 @@ func (h *Settings) Handle(ctx context.Context, a fleetapi.Action, acker store.Fl return fmt.Errorf("invalid log level, expected debug|info|warning|error and received '%s'", action.LogLevel) } - if err := h.agentInfo.LogLevel(action.LogLevel); err != nil { + if err := h.agentInfo.SetLogLevel(action.LogLevel); err != nil { return errors.New("failed to update log level", err) } diff --git a/x-pack/elastic-agent/pkg/agent/application/pipeline/emitter/modifiers/fleet_decorator.go b/x-pack/elastic-agent/pkg/agent/application/pipeline/emitter/modifiers/fleet_decorator.go index f4850a345af9..08cf8e673334 100644 --- a/x-pack/elastic-agent/pkg/agent/application/pipeline/emitter/modifiers/fleet_decorator.go +++ b/x-pack/elastic-agent/pkg/agent/application/pipeline/emitter/modifiers/fleet_decorator.go @@ -17,12 +17,6 @@ import ( // InjectFleet injects fleet metadata into a configuration. func InjectFleet(cfg *config.Config, hostInfo types.HostInfo, agentInfo *info.AgentInfo) func(*logger.Logger, *transpiler.AST) error { return func(logger *logger.Logger, rootAst *transpiler.AST) error { - ecsMeta, err := agentInfo.ECSMetadata() - if err != nil { - return err - } - logLevel := ecsMeta.Elastic.Agent.LogLevel - config, err := cfg.ToMapStr() if err != nil { return err @@ -46,7 +40,7 @@ func InjectFleet(cfg *config.Config, hostInfo types.HostInfo, agentInfo *info.Ag // ensure that the agent.logging.level is present if _, found := transpiler.Lookup(ast, "agent.logging.level"); !found { - transpiler.Insert(ast, transpiler.NewKey("level", transpiler.NewStrVal(logLevel)), "agent.logging") + transpiler.Insert(ast, transpiler.NewKey("level", transpiler.NewStrVal(agentInfo.LogLevel())), "agent.logging") } // fleet.host to Agent can be the host to connect to Fleet Server, but to Applications it should diff --git a/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go b/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go index c7453f326f9c..e1e0df6a8f65 100644 --- a/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go +++ b/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go @@ -133,11 +133,13 @@ func newEnrollCmd( storage.NewDiskStore(paths.AgentConfigFile()), ) + syncStore := storage.NewWindowsSyncOnSaveStore(store, paths.AgentStateStoreFile()) + return newEnrollCmdWithStore( log, options, configPath, - store, + syncStore, ) } diff --git a/x-pack/elastic-agent/pkg/agent/cmd/run.go b/x-pack/elastic-agent/pkg/agent/cmd/run.go index 5fa7ad112216..06296b4c6556 100644 --- a/x-pack/elastic-agent/pkg/agent/cmd/run.go +++ b/x-pack/elastic-agent/pkg/agent/cmd/run.go @@ -225,8 +225,9 @@ func getOverwrites(rawConfig *config.Config) error { return nil } path := paths.AgentConfigFile() + diskStore := storage.NewDiskStore(path) + store := storage.NewWindowsSyncOnSaveStore(diskStore, path) - store := storage.NewDiskStore(path) reader, err := store.Load() if err != nil && errors.Is(err, os.ErrNotExist) { // no fleet file ignore @@ -262,7 +263,7 @@ func defaultLogLevel(cfg *configuration.Configuration) string { return "" } - defaultLogLevel := logger.DefaultLoggingConfig().Level.String() + defaultLogLevel := logger.DefaultLogLevel.String() if configuredLevel := cfg.Settings.LoggingConfig.Level.String(); configuredLevel != "" && configuredLevel != defaultLogLevel { // predefined log level return configuredLevel diff --git a/x-pack/elastic-agent/pkg/agent/storage/disk_store.go b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go new file mode 100644 index 000000000000..b7153af039e5 --- /dev/null +++ b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go @@ -0,0 +1,95 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package storage + +import ( + "fmt" + "io" + "os" + + "github.com/elastic/beats/v7/libbeat/common/file" + "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors" + "github.com/hectane/go-acl" +) + +// NewDiskStore creates an unencrypted disk store. +func NewDiskStore(target string) *DiskStore { + return &DiskStore{target: target} +} + +// Exists check if the store file exists on the disk +func (d *DiskStore) Exists() (bool, error) { + _, err := os.Stat(d.target) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + return true, nil +} + +// Delete deletes the store file on the disk +func (d *DiskStore) Delete() error { + return os.Remove(d.target) +} + +// Save accepts a persistedConfig and saved it to a target file, to do so we will +// make a temporary files if the write is successful we are replacing the target file with the +// original content. +func (d *DiskStore) Save(in io.Reader) error { + tmpFile := d.target + ".tmp" + + fd, err := os.OpenFile(tmpFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, perms) + if err != nil { + return errors.New(err, + fmt.Sprintf("could not save to %s", tmpFile), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, tmpFile)) + } + + // Always clean up the temporary file and ignore errors. + defer os.Remove(tmpFile) + + if _, err := io.Copy(fd, in); err != nil { + return errors.New(err, "could not save content on disk", + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, tmpFile)) + } + + if err := fd.Close(); err != nil { + return errors.New(err, "could not close temporary file", + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, tmpFile)) + } + + if err := file.SafeFileRotate(d.target, tmpFile); err != nil { + return errors.New(err, + fmt.Sprintf("could not replace target file %s", d.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, d.target)) + } + + if err := acl.Chmod(d.target, perms); err != nil { + return errors.New(err, + fmt.Sprintf("could not set permissions target file %s", d.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, d.target)) + } + + return nil +} + +// Load return a io.ReadCloser for the target file. +func (d *DiskStore) Load() (io.ReadCloser, error) { + fd, err := os.OpenFile(d.target, os.O_RDONLY|os.O_CREATE, perms) + if err != nil { + return nil, errors.New(err, + fmt.Sprintf("could not open %s", d.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, d.target)) + } + return fd, nil +} diff --git a/x-pack/elastic-agent/pkg/agent/storage/handler_store.go b/x-pack/elastic-agent/pkg/agent/storage/handler_store.go new file mode 100644 index 000000000000..247d34581f38 --- /dev/null +++ b/x-pack/elastic-agent/pkg/agent/storage/handler_store.go @@ -0,0 +1,24 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package storage + +import "io" + +type handlerFunc func(io.Reader) error + +// HandlerStore take a function handler and wrap it into the store interface. +type HandlerStore struct { + fn handlerFunc +} + +// NewHandlerStore takes a function and wrap it into an handlerStore. +func NewHandlerStore(fn handlerFunc) *HandlerStore { + return &HandlerStore{fn: fn} +} + +// Save calls the handler. +func (h *HandlerStore) Save(in io.Reader) error { + return h.fn(in) +} diff --git a/x-pack/elastic-agent/pkg/agent/storage/null_store.go b/x-pack/elastic-agent/pkg/agent/storage/null_store.go new file mode 100644 index 000000000000..79b648c3816a --- /dev/null +++ b/x-pack/elastic-agent/pkg/agent/storage/null_store.go @@ -0,0 +1,15 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package storage + +import "io" + +// NullStore this is only use to split the work into multiples PRs. +type NullStore struct{} + +// Save takes the fleetConfig and persist it, will return an errors on failure. +func (m *NullStore) Save(_ io.Reader) error { + return nil +} diff --git a/x-pack/elastic-agent/pkg/agent/storage/replace_store.go b/x-pack/elastic-agent/pkg/agent/storage/replace_store.go new file mode 100644 index 000000000000..498aad4a1499 --- /dev/null +++ b/x-pack/elastic-agent/pkg/agent/storage/replace_store.go @@ -0,0 +1,106 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package storage + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + "os" + "time" + + "github.com/elastic/beats/v7/libbeat/common/file" + "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors" + "github.com/hectane/go-acl" +) + +// ReplaceOnSuccessStore takes a target file, a replacement content and a wrapped store. This +// store is useful if you want to trigger an action to replace another file when the wrapped store save method +// is successful. This store will take care of making a backup copy of the target file and will not +// override the content of the target if the target has already the same content. If an error happen, +// we will not replace the file. +type ReplaceOnSuccessStore struct { + target string + replaceWith []byte + + wrapped Store +} + +// NewReplaceOnSuccessStore takes a target file and a replacement content and will replace the target +// file content if the wrapped store execution is done without any error. +func NewReplaceOnSuccessStore(target string, replaceWith []byte, wrapped Store) *ReplaceOnSuccessStore { + return &ReplaceOnSuccessStore{ + target: target, + replaceWith: replaceWith, + wrapped: wrapped, + } +} + +// Save will replace a target file with new content if the wrapped store is successful. +func (r *ReplaceOnSuccessStore) Save(in io.Reader) error { + // Ensure we can read the target files before delegating any call to the wrapped store. + target, err := ioutil.ReadFile(r.target) + if err != nil { + return errors.New(err, + fmt.Sprintf("fail to read content of %s", r.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, r.target)) + } + + err = r.wrapped.Save(in) + if err != nil { + return err + } + + if bytes.Equal(target, r.replaceWith) { + return nil + } + + // Windows is tricky with the characters permitted for the path and filename, so we have + // to remove any colon from the string. We are using nanosec precision here because of automated + // tools. + const fsSafeTs = "2006-01-02T15-04-05.9999" + + ts := time.Now() + backFilename := r.target + "." + ts.Format(fsSafeTs) + ".bak" + if err := file.SafeFileRotate(backFilename, r.target); err != nil { + return errors.New(err, + fmt.Sprintf("could not backup %s", r.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, r.target)) + } + + fd, err := os.OpenFile(r.target, os.O_CREATE|os.O_WRONLY, perms) + if err != nil { + // Rollback on any errors to minimize non working state. + if err := file.SafeFileRotate(r.target, backFilename); err != nil { + return errors.New(err, + fmt.Sprintf("could not rollback %s to %s", backFilename, r.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, r.target), + errors.M("backup_path", backFilename)) + } + } + + if _, err := fd.Write(r.replaceWith); err != nil { + if err := file.SafeFileRotate(r.target, backFilename); err != nil { + return errors.New(err, + fmt.Sprintf("could not rollback %s to %s", backFilename, r.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, r.target), + errors.M("backup_path", backFilename)) + } + } + + if err := acl.Chmod(r.target, perms); err != nil { + return errors.New(err, + fmt.Sprintf("could not set permissions target file %s", r.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, r.target)) + } + + return nil +} diff --git a/x-pack/elastic-agent/pkg/agent/storage/storage.go b/x-pack/elastic-agent/pkg/agent/storage/storage.go index a578edb1dfd1..1945f10c3efe 100644 --- a/x-pack/elastic-agent/pkg/agent/storage/storage.go +++ b/x-pack/elastic-agent/pkg/agent/storage/storage.go @@ -5,17 +5,8 @@ package storage import ( - "bytes" - "fmt" "io" - "io/ioutil" "os" - "time" - - "github.com/hectane/go-acl" - - "github.com/elastic/beats/v7/libbeat/common/file" - "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors" ) const perms os.FileMode = 0600 @@ -26,200 +17,7 @@ type Store interface { Save(io.Reader) error } -// NullStore this is only use to split the work into multiples PRs. -type NullStore struct{} - -// Save takes the fleetConfig and persist it, will return an errors on failure. -func (m *NullStore) Save(_ io.Reader) error { - return nil -} - -type handlerFunc func(io.Reader) error - -// HandlerStore take a function handler and wrap it into the store interface. -type HandlerStore struct { - fn handlerFunc -} - -// NewHandlerStore takes a function and wrap it into an handlerStore. -func NewHandlerStore(fn handlerFunc) *HandlerStore { - return &HandlerStore{fn: fn} -} - -// Save calls the handler. -func (h *HandlerStore) Save(in io.Reader) error { - return h.fn(in) -} - -// ReplaceOnSuccessStore takes a target file, a replacement content and a wrapped store. This -// store is useful if you want to trigger an action to replace another file when the wrapped store save method -// is successful. This store will take care of making a backup copy of the target file and will not -// override the content of the target if the target has already the same content. If an error happen, -// we will not replace the file. -type ReplaceOnSuccessStore struct { - target string - replaceWith []byte - - wrapped Store -} - -// NewReplaceOnSuccessStore takes a target file and a replacement content and will replace the target -// file content if the wrapped store execution is done without any error. -func NewReplaceOnSuccessStore(target string, replaceWith []byte, wrapped Store) *ReplaceOnSuccessStore { - return &ReplaceOnSuccessStore{ - target: target, - replaceWith: replaceWith, - wrapped: wrapped, - } -} - -// Save will replace a target file with new content if the wrapped store is successful. -func (r *ReplaceOnSuccessStore) Save(in io.Reader) error { - // Ensure we can read the target files before delegating any call to the wrapped store. - target, err := ioutil.ReadFile(r.target) - if err != nil { - return errors.New(err, - fmt.Sprintf("fail to read content of %s", r.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, r.target)) - } - - err = r.wrapped.Save(in) - if err != nil { - return err - } - - if bytes.Equal(target, r.replaceWith) { - return nil - } - - // Windows is tricky with the characters permitted for the path and filename, so we have - // to remove any colon from the string. We are using nanosec precision here because of automated - // tools. - const fsSafeTs = "2006-01-02T15-04-05.9999" - - ts := time.Now() - backFilename := r.target + "." + ts.Format(fsSafeTs) + ".bak" - if err := file.SafeFileRotate(backFilename, r.target); err != nil { - return errors.New(err, - fmt.Sprintf("could not backup %s", r.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, r.target)) - } - - fd, err := os.OpenFile(r.target, os.O_CREATE|os.O_WRONLY, perms) - if err != nil { - // Rollback on any errors to minimize non working state. - if err := file.SafeFileRotate(r.target, backFilename); err != nil { - return errors.New(err, - fmt.Sprintf("could not rollback %s to %s", backFilename, r.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, r.target), - errors.M("backup_path", backFilename)) - } - } - - if _, err := fd.Write(r.replaceWith); err != nil { - if err := file.SafeFileRotate(r.target, backFilename); err != nil { - return errors.New(err, - fmt.Sprintf("could not rollback %s to %s", backFilename, r.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, r.target), - errors.M("backup_path", backFilename)) - } - } - - if err := acl.Chmod(r.target, perms); err != nil { - return errors.New(err, - fmt.Sprintf("could not set permissions target file %s", r.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, r.target)) - } - - return nil -} - // DiskStore takes a persistedConfig and save it to a temporary files and replace the target file. type DiskStore struct { target string } - -// NewDiskStore creates an unencrypted disk store. -func NewDiskStore(target string) *DiskStore { - return &DiskStore{target: target} -} - -// Exists check if the store file exists on the disk -func (d *DiskStore) Exists() (bool, error) { - _, err := os.Stat(d.target) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - return true, nil -} - -// Delete deletes the store file on the disk -func (d *DiskStore) Delete() error { - return os.Remove(d.target) -} - -// Save accepts a persistedConfig and saved it to a target file, to do so we will -// make a temporary files if the write is successful we are replacing the target file with the -// original content. -func (d *DiskStore) Save(in io.Reader) error { - tmpFile := d.target + ".tmp" - - fd, err := os.OpenFile(tmpFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, perms) - if err != nil { - return errors.New(err, - fmt.Sprintf("could not save to %s", tmpFile), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, tmpFile)) - } - - // Always clean up the temporary file and ignore errors. - defer os.Remove(tmpFile) - - if _, err := io.Copy(fd, in); err != nil { - return errors.New(err, "could not save content on disk", - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, tmpFile)) - } - - if err := fd.Close(); err != nil { - return errors.New(err, "could not close temporary file", - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, tmpFile)) - } - - if err := file.SafeFileRotate(d.target, tmpFile); err != nil { - return errors.New(err, - fmt.Sprintf("could not replace target file %s", d.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, d.target)) - } - - if err := acl.Chmod(d.target, perms); err != nil { - return errors.New(err, - fmt.Sprintf("could not set permissions target file %s", d.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, d.target)) - } - - return nil -} - -// Load return a io.ReadCloser for the target file. -func (d *DiskStore) Load() (io.ReadCloser, error) { - fd, err := os.OpenFile(d.target, os.O_RDONLY|os.O_CREATE, perms) - if err != nil { - return nil, errors.New(err, - fmt.Sprintf("could not open %s", d.target), - errors.TypeFilesystem, - errors.M(errors.MetaKeyPath, d.target)) - } - return fd, nil -} diff --git a/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go b/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go new file mode 100644 index 000000000000..99f85a99d11e --- /dev/null +++ b/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go @@ -0,0 +1,77 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package storage + +import ( + "io" + "os" + "runtime" + + "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors" +) + +// SyncOnSaveStore syncs paths after successful write. +type SyncOnSaveStore struct { + enabled bool + syncPath string + wrapped Store +} + +// NewSyncStore creates always syncing store. +func NewSyncStore(wrappedStore Store, syncPath string) *SyncOnSaveStore { + return &SyncOnSaveStore{ + enabled: true, + syncPath: syncPath, + wrapped: wrappedStore, + } +} + +// NewWindowsSyncOnSaveStore creates windows syncing store. +func NewWindowsSyncOnSaveStore(wrappedStore Store, syncPath string) *SyncOnSaveStore { + // TODO: think of windows 7 only syncing store as this is the slowest + return &SyncOnSaveStore{ + enabled: runtime.GOOS == "windows", + syncPath: syncPath, + wrapped: wrappedStore, + } +} + +// Save accepts a persistedConfig and saved it to a target file, to do so we will +// make a temporary files if the write is successful we are replacing the target file with the +// original content. +func (s *SyncOnSaveStore) Save(in io.Reader) error { + if err := s.wrapped.Save(in); err != nil { + return err + } + + if !s.enabled { + return nil + } + + f, err := os.OpenFile(s.syncPath, os.O_RDWR, 0777) + if os.IsNotExist(err) { + return nil + } + + if err != nil { + return err + } + + defer f.Close() + return f.Sync() +} + +// Load return a io.ReadCloser for the target file. +func (s *SyncOnSaveStore) Load() (io.ReadCloser, error) { + type loader interface { + Load() (io.ReadCloser, error) + } + + if loader, ok := s.wrapped.(loader); ok { + return loader.Load() + } + + return nil, errors.New("load is not supported for this store") +} diff --git a/x-pack/elastic-agent/pkg/core/logger/logger.go b/x-pack/elastic-agent/pkg/core/logger/logger.go index 7d74bf1f6f80..e32c4b680880 100644 --- a/x-pack/elastic-agent/pkg/core/logger/logger.go +++ b/x-pack/elastic-agent/pkg/core/logger/logger.go @@ -23,6 +23,9 @@ import ( const agentName = "elastic-agent" +// DefaultLogLevel used in agent and its processes. +const DefaultLogLevel = logp.InfoLevel + // Logger alias ecslog.Logger with Logger. type Logger = logp.Logger @@ -93,7 +96,7 @@ func toCommonConfig(cfg *Config) (*common.Config, error) { func DefaultLoggingConfig() *Config { cfg := logp.DefaultConfig(logp.DefaultEnvironment) cfg.Beat = agentName - cfg.Level = logp.InfoLevel + cfg.Level = DefaultLogLevel cfg.ToFiles = true cfg.Files.Path = paths.Logs() cfg.Files.Name = agentName From 8f6dfd3d14382b0a27d82874bbf0ddeae34c474f Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 25 May 2021 17:08:16 +0200 Subject: [PATCH 2/6] comment --- x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go b/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go index 99f85a99d11e..9bb1cc99c4dc 100644 --- a/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go +++ b/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go @@ -13,6 +13,10 @@ import ( ) // SyncOnSaveStore syncs paths after successful write. +// we use this sync store to avoid issues seen on +// low spec windows environment where agent is faster +// than filesystem and what we read is different(stale) +// than what we just wrote. type SyncOnSaveStore struct { enabled bool syncPath string From e34323de16d050cfff5fd0e70cc829fcc59486f1 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 25 May 2021 17:14:54 +0200 Subject: [PATCH 3/6] chl --- x-pack/elastic-agent/CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/elastic-agent/CHANGELOG.next.asciidoc b/x-pack/elastic-agent/CHANGELOG.next.asciidoc index 4cdfd25c6dbd..914d45258bca 100644 --- a/x-pack/elastic-agent/CHANGELOG.next.asciidoc +++ b/x-pack/elastic-agent/CHANGELOG.next.asciidoc @@ -67,6 +67,7 @@ - Add error log entry when listener creation fails {issue}23483[23482] - Handle case where policy doesn't contain Fleet connection information {pull}25707[25707] - Fix fleet-server.yml spec to not overwrite existing keys {pull}25741[25741] +- Agent sends wrong log level to Endpoint {issue}25583[25583] ==== New features From 092c2e6df57a0760dbbac2e9cf0bd980fb3affd7 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 25 May 2021 17:42:25 +0200 Subject: [PATCH 4/6] fmt --- x-pack/elastic-agent/pkg/agent/storage/disk_store.go | 3 ++- x-pack/elastic-agent/pkg/agent/storage/replace_store.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/elastic-agent/pkg/agent/storage/disk_store.go b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go index b7153af039e5..9276b497539b 100644 --- a/x-pack/elastic-agent/pkg/agent/storage/disk_store.go +++ b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go @@ -9,9 +9,10 @@ import ( "io" "os" + "github.com/hectane/go-acl" + "github.com/elastic/beats/v7/libbeat/common/file" "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors" - "github.com/hectane/go-acl" ) // NewDiskStore creates an unencrypted disk store. diff --git a/x-pack/elastic-agent/pkg/agent/storage/replace_store.go b/x-pack/elastic-agent/pkg/agent/storage/replace_store.go index 498aad4a1499..44c076ddc1c2 100644 --- a/x-pack/elastic-agent/pkg/agent/storage/replace_store.go +++ b/x-pack/elastic-agent/pkg/agent/storage/replace_store.go @@ -12,9 +12,10 @@ import ( "os" "time" + "github.com/hectane/go-acl" + "github.com/elastic/beats/v7/libbeat/common/file" "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors" - "github.com/hectane/go-acl" ) // ReplaceOnSuccessStore takes a target file, a replacement content and a wrapped store. This From 13e06049df505882722606a2c15974a4726d3b55 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 26 May 2021 11:16:10 +0200 Subject: [PATCH 5/6] sync before close instead of wrap --- .../pkg/agent/application/application.go | 3 +- .../gateway/fleet/fleet_gateway_test.go | 9 +-- .../pkg/agent/application/info/agent_id.go | 8 +- .../elastic-agent/pkg/agent/cmd/enroll_cmd.go | 4 +- x-pack/elastic-agent/pkg/agent/cmd/run.go | 3 +- .../pkg/agent/storage/disk_store.go | 2 + .../pkg/agent/storage/sync_on_save_store.go | 81 ------------------- 7 files changed, 11 insertions(+), 99 deletions(-) delete mode 100644 x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go diff --git a/x-pack/elastic-agent/pkg/agent/application/application.go b/x-pack/elastic-agent/pkg/agent/application/application.go index c590cad670f0..53c74afabe50 100644 --- a/x-pack/elastic-agent/pkg/agent/application/application.go +++ b/x-pack/elastic-agent/pkg/agent/application/application.go @@ -96,8 +96,7 @@ func createApplication( func mergeFleetConfig(rawConfig *config.Config) (storage.Store, *configuration.Configuration, error) { path := paths.AgentConfigFile() - diskStore := storage.NewDiskStore(path) - store := storage.NewWindowsSyncOnSaveStore(diskStore, path) + store := storage.NewDiskStore(path) reader, err := store.Load() if err != nil { return store, nil, errors.New(err, "could not initialize config store", diff --git a/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go b/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go index c711a1acbb9e..eba95ad778c7 100644 --- a/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go +++ b/x-pack/elastic-agent/pkg/agent/application/gateway/fleet/fleet_gateway_test.go @@ -125,8 +125,7 @@ func withGateway(agentInfo agentInfo, settings *fleetGatewaySettings, fn withGat defer cancel() diskStore := storage.NewDiskStore(paths.AgentStateStoreFile()) - syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, paths.AgentStateStoreFile()) - stateStore, err := store.NewStateStore(log, syncStore) + stateStore, err := store.NewStateStore(log, diskStore) require.NoError(t, err) gateway, err := newFleetGatewayWithScheduler( @@ -257,8 +256,7 @@ func TestFleetGateway(t *testing.T) { log, _ := logger.New("tst", false) diskStore := storage.NewDiskStore(paths.AgentStateStoreFile()) - syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, paths.AgentStateStoreFile()) - stateStore, err := store.NewStateStore(log, syncStore) + stateStore, err := store.NewStateStore(log, diskStore) require.NoError(t, err) gateway, err := newFleetGatewayWithScheduler( @@ -350,8 +348,7 @@ func TestFleetGateway(t *testing.T) { log, _ := logger.New("tst", false) diskStore := storage.NewDiskStore(paths.AgentStateStoreFile()) - syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, paths.AgentStateStoreFile()) - stateStore, err := store.NewStateStore(log, syncStore) + stateStore, err := store.NewStateStore(log, diskStore) require.NoError(t, err) gateway, err := newFleetGatewayWithScheduler( diff --git a/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go b/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go index 6c65afac38d1..386beabca611 100644 --- a/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go +++ b/x-pack/elastic-agent/pkg/agent/application/info/agent_id.go @@ -52,10 +52,9 @@ func updateLogLevel(level string) error { agentConfigFile := paths.AgentConfigFile() diskStore := storage.NewDiskStore(agentConfigFile) - syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, agentConfigFile) ai.LogLevel = level - return updateAgentInfo(syncStore, ai) + return updateAgentInfo(diskStore, ai) } func generateAgentID() (string, error) { @@ -193,9 +192,8 @@ func loadAgentInfo(forceUpdate bool, logLevel string, createAgentID bool) (*pers agentConfigFile := paths.AgentConfigFile() diskStore := storage.NewDiskStore(agentConfigFile) - syncStore := storage.NewWindowsSyncOnSaveStore(diskStore, agentConfigFile) - agentinfo, err := getInfoFromStore(syncStore, logLevel) + agentinfo, err := getInfoFromStore(diskStore, logLevel) if err != nil { return nil, err } @@ -204,7 +202,7 @@ func loadAgentInfo(forceUpdate bool, logLevel string, createAgentID bool) (*pers return agentinfo, nil } - if err := updateID(agentinfo, syncStore); err != nil { + if err := updateID(agentinfo, diskStore); err != nil { return nil, err } diff --git a/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go b/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go index e1e0df6a8f65..c7453f326f9c 100644 --- a/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go +++ b/x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go @@ -133,13 +133,11 @@ func newEnrollCmd( storage.NewDiskStore(paths.AgentConfigFile()), ) - syncStore := storage.NewWindowsSyncOnSaveStore(store, paths.AgentStateStoreFile()) - return newEnrollCmdWithStore( log, options, configPath, - syncStore, + store, ) } diff --git a/x-pack/elastic-agent/pkg/agent/cmd/run.go b/x-pack/elastic-agent/pkg/agent/cmd/run.go index 06296b4c6556..b8a2445d82e8 100644 --- a/x-pack/elastic-agent/pkg/agent/cmd/run.go +++ b/x-pack/elastic-agent/pkg/agent/cmd/run.go @@ -225,8 +225,7 @@ func getOverwrites(rawConfig *config.Config) error { return nil } path := paths.AgentConfigFile() - diskStore := storage.NewDiskStore(path) - store := storage.NewWindowsSyncOnSaveStore(diskStore, path) + store := storage.NewDiskStore(path) reader, err := store.Load() if err != nil && errors.Is(err, os.ErrNotExist) { diff --git a/x-pack/elastic-agent/pkg/agent/storage/disk_store.go b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go index 9276b497539b..838a4c4de02a 100644 --- a/x-pack/elastic-agent/pkg/agent/storage/disk_store.go +++ b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go @@ -60,6 +60,8 @@ func (d *DiskStore) Save(in io.Reader) error { errors.M(errors.MetaKeyPath, tmpFile)) } + _ = fd.Sync() + if err := fd.Close(); err != nil { return errors.New(err, "could not close temporary file", errors.TypeFilesystem, diff --git a/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go b/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go deleted file mode 100644 index 9bb1cc99c4dc..000000000000 --- a/x-pack/elastic-agent/pkg/agent/storage/sync_on_save_store.go +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// or more contributor license agreements. Licensed under the Elastic License; -// you may not use this file except in compliance with the Elastic License. - -package storage - -import ( - "io" - "os" - "runtime" - - "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors" -) - -// SyncOnSaveStore syncs paths after successful write. -// we use this sync store to avoid issues seen on -// low spec windows environment where agent is faster -// than filesystem and what we read is different(stale) -// than what we just wrote. -type SyncOnSaveStore struct { - enabled bool - syncPath string - wrapped Store -} - -// NewSyncStore creates always syncing store. -func NewSyncStore(wrappedStore Store, syncPath string) *SyncOnSaveStore { - return &SyncOnSaveStore{ - enabled: true, - syncPath: syncPath, - wrapped: wrappedStore, - } -} - -// NewWindowsSyncOnSaveStore creates windows syncing store. -func NewWindowsSyncOnSaveStore(wrappedStore Store, syncPath string) *SyncOnSaveStore { - // TODO: think of windows 7 only syncing store as this is the slowest - return &SyncOnSaveStore{ - enabled: runtime.GOOS == "windows", - syncPath: syncPath, - wrapped: wrappedStore, - } -} - -// Save accepts a persistedConfig and saved it to a target file, to do so we will -// make a temporary files if the write is successful we are replacing the target file with the -// original content. -func (s *SyncOnSaveStore) Save(in io.Reader) error { - if err := s.wrapped.Save(in); err != nil { - return err - } - - if !s.enabled { - return nil - } - - f, err := os.OpenFile(s.syncPath, os.O_RDWR, 0777) - if os.IsNotExist(err) { - return nil - } - - if err != nil { - return err - } - - defer f.Close() - return f.Sync() -} - -// Load return a io.ReadCloser for the target file. -func (s *SyncOnSaveStore) Load() (io.ReadCloser, error) { - type loader interface { - Load() (io.ReadCloser, error) - } - - if loader, ok := s.wrapped.(loader); ok { - return loader.Load() - } - - return nil, errors.New("load is not supported for this store") -} From cdc79e65edb606bc2a756f9d8a9193273a5efff9 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 26 May 2021 11:56:17 +0200 Subject: [PATCH 6/6] closing --- x-pack/elastic-agent/pkg/agent/storage/disk_store.go | 2 ++ x-pack/elastic-agent/pkg/agent/storage/replace_store.go | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/x-pack/elastic-agent/pkg/agent/storage/disk_store.go b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go index 838a4c4de02a..6c7b12041540 100644 --- a/x-pack/elastic-agent/pkg/agent/storage/disk_store.go +++ b/x-pack/elastic-agent/pkg/agent/storage/disk_store.go @@ -55,6 +55,8 @@ func (d *DiskStore) Save(in io.Reader) error { defer os.Remove(tmpFile) if _, err := io.Copy(fd, in); err != nil { + _ = fd.Close() + return errors.New(err, "could not save content on disk", errors.TypeFilesystem, errors.M(errors.MetaKeyPath, tmpFile)) diff --git a/x-pack/elastic-agent/pkg/agent/storage/replace_store.go b/x-pack/elastic-agent/pkg/agent/storage/replace_store.go index 44c076ddc1c2..18b938f2f2fd 100644 --- a/x-pack/elastic-agent/pkg/agent/storage/replace_store.go +++ b/x-pack/elastic-agent/pkg/agent/storage/replace_store.go @@ -85,6 +85,7 @@ func (r *ReplaceOnSuccessStore) Save(in io.Reader) error { errors.M("backup_path", backFilename)) } } + defer fd.Close() if _, err := fd.Write(r.replaceWith); err != nil { if err := file.SafeFileRotate(r.target, backFilename); err != nil { @@ -103,5 +104,11 @@ func (r *ReplaceOnSuccessStore) Save(in io.Reader) error { errors.M(errors.MetaKeyPath, r.target)) } + if err := fd.Sync(); err != nil { + return errors.New(err, + fmt.Sprintf("could not sync target file %s", r.target), + errors.TypeFilesystem, + errors.M(errors.MetaKeyPath, r.target)) + } return nil }