Skip to content

Commit

Permalink
PMM-11658 move the exporter configs away from /tmp (#2341)
Browse files Browse the repository at this point in the history 
* PMM-11658 move TempDir inside the main dist tree

* PMM-11658 add panic on error for tmp creation

* PMM-11658 panic using the logger

* PMM-11658 refactor old APIs

PMM_11658 change the message wording

* PMM-11658 throw away redundant installs

* PMM-11658 cleanup the tmp directory an agent shutdown

PMM-11658 remove a debug statement

PMM-11658 format the code

PMM-11658 fix a linter error

* PMM-11658 clean TmpDir on agent stop event

* PMM-11658 fix the tmp path in tests

* PMM-11658 fix timing in TmpDir path assignment and creation

* PMM-11658 refactor TempDir assignment logic

PMM-11658 skip the failing test

PMM-11658 print some debug info

* PMM-11658 set a nicer user prompt for docker

PMM-11658 print more debug info

* PMM-11658 refactor TmpDir to be relative of cwd

PMM-11658 format the code

* PMM-11658 do not use root dirs in tests

* PMM-11658 fix linter errors

* PMM-11658 use t.Cleanup vs defer

* PMM-11658 adress review comments

* PMM-11658 make TempDir relative to BasePath, fix tests

* PMM-11658 minor corrections

* PMM-11658 fix the prompt

* PMM-11658 revert .golangci.yml

* PMM-11658 remove redundant TmpDir creation

* PMM-11658 fix the lint error
  • Loading branch information
Alex Tymchuk authored Jul 24, 2023
1 parent b8ead6d commit 46ecd31
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 47 deletions.
1 change: 0 additions & 1 deletion .github/workflows/admin.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ jobs:

- name: Setup tools
run: |
sudo apt-get install -y wget jq
sudo ln -sf /home/runner/go/bin/pmm /usr/bin
sudo chown -R runner:docker /usr/bin/pmm
Expand Down
6 changes: 6 additions & 0 deletions agent/agents/supervisor/supervisor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,12 @@ func (s *Supervisor) setBuiltinAgents(builtinAgents map[string]*agentpb.SetState
<-agent.done

delete(s.builtinAgents, agentID)

agentTmp := filepath.Join(s.cfg.Get().Paths.TempDir, strings.ToLower(agent.requestedState.Type.String()), agentID)
err := os.RemoveAll(agentTmp)
if err != nil {
s.l.Warnf("Failed to cleanup directory '%s': %s", agentTmp, err.Error())
}
}

// restart
Expand Down
6 changes: 5 additions & 1 deletion agent/commands/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (

// Run implements `pmm-agent run` default command.
func Run() {
var cfg *config.Config
l := logrus.WithField("component", "main")
ctx, cancel := context.WithCancel(context.Background())
defer l.Info("Done.")
Expand All @@ -55,6 +56,9 @@ func Run() {
s := <-signals
signal.Stop(signals)
l.Warnf("Got %s, shutting down...", unix.SignalName(s.(unix.Signal))) //nolint:forcetypeassert
if cfg != nil {
cleanupTmp(cfg.Paths.TempDir, l)
}
cancel()
}()

Expand All @@ -64,7 +68,7 @@ func Run() {
l.Fatalf("Failed to load configuration: %s.", err)
}

cfg := configStorage.Get()
cfg = configStorage.Get()

cleanupTmp(cfg.Paths.TempDir, l)
connectionUptimeService := connectionuptime.NewService(cfg.WindowConnectedTime)
Expand Down
28 changes: 21 additions & 7 deletions agent/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package config

import (
"fmt"
"io/fs"
"net"
"net/url"
"os"
Expand All @@ -25,6 +26,7 @@ import (
"strings"
"time"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
"gopkg.in/alecthomas/kingpin.v2"
Expand All @@ -34,7 +36,10 @@ import (
"github.com/percona/pmm/version"
)

const pathBaseDefault = "/usr/local/percona/pmm2"
const (
pathBaseDefault = "/usr/local/percona/pmm2"
agentTmpPath = "tmp" // temporary directory to keep exporters' config files, relative to pathBase
)

// Server represents PMM Server configuration.
type Server struct {
Expand Down Expand Up @@ -179,7 +184,7 @@ func getFromCmdLine(cfg *Config, l *logrus.Entry) (string, error) {
}

// get is Get for unit tests: it parses args instead of command-line.
func get(args []string, cfg *Config, l *logrus.Entry) (configFileF string, err error) { //nolint:nonamedreturns
func get(args []string, cfg *Config, l *logrus.Entry) (configFileF string, err error) { //nolint:nonamedreturns,cyclop
// tweak configuration on exit to cover all return points
defer func() {
if cfg == nil {
Expand Down Expand Up @@ -212,7 +217,6 @@ func get(args []string, cfg *Config, l *logrus.Entry) (configFileF string, err e
&cfg.Paths.RDSExporter: "rds_exporter",
&cfg.Paths.AzureExporter: "azure_exporter",
&cfg.Paths.VMAgent: "vmagent",
&cfg.Paths.TempDir: os.TempDir(),
&cfg.Paths.PTSummary: "tools/pt-summary",
&cfg.Paths.PTPGSummary: "tools/pt-pg-summary",
&cfg.Paths.PTMongoDBSummary: "tools/pt-mongodb-summary",
Expand All @@ -237,6 +241,16 @@ func get(args []string, cfg *Config, l *logrus.Entry) (configFileF string, err e
cfg.Paths.ExportersBase = abs
}

if cfg.Paths.TempDir == "" {
cfg.Paths.TempDir = filepath.Join(cfg.Paths.PathsBase, agentTmpPath)
l.Infof("Temporary directory is not configured and will be set to %s", cfg.Paths.TempDir)
}

if !filepath.IsAbs(cfg.Paths.TempDir) {
cfg.Paths.TempDir = filepath.Join(cfg.Paths.PathsBase, cfg.Paths.TempDir)
l.Debugf("Temporary directory is configured as %s", cfg.Paths.TempDir)
}

if !filepath.IsAbs(cfg.Paths.PTSummary) {
cfg.Paths.PTSummary = filepath.Join(cfg.Paths.PathsBase, cfg.Paths.PTSummary)
}
Expand Down Expand Up @@ -308,7 +322,7 @@ func get(args []string, cfg *Config, l *logrus.Entry) (configFileF string, err e
}

*cfg = *fileCfg
return //nolint:nakedret
return configFileF, nil
}

// Application returns kingpin application that will parse command-line flags and environment variables
Expand Down Expand Up @@ -474,7 +488,7 @@ func Application(cfg *Config) (*kingpin.Application, *string) {
// Other errors are returned if file exists, but configuration can't be loaded due to permission problems,
// YAML parsing problems, etc.
func loadFromFile(path string) (*Config, error) {
if _, err := os.Stat(path); os.IsNotExist(err) {
if _, err := os.Stat(path); errors.Is(err, fs.ErrNotExist) {
return nil, ConfigFileDoesNotExistError(path)
}

Expand Down Expand Up @@ -510,8 +524,8 @@ func SaveToFile(path string, cfg *Config, comment string) error {
func IsWritable(path string) error {
_, err := os.Stat(path)
if err != nil {
// File doesn't exists, check if folder is writable.
if os.IsNotExist(err) {
// File doesn't exist, check if folder is writable.
if errors.Is(err, fs.ErrNotExist) {
return unix.Access(filepath.Dir(path), unix.W_OK)
}
return err
Expand Down
95 changes: 68 additions & 27 deletions agent/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,15 @@ func removeConfig(t *testing.T, name string) {
require.NoError(t, os.Remove(name))
}

func generateTempDirPath(t *testing.T, basePath string) string {
t.Helper()
return filepath.Join(basePath, agentTmpPath)
}

func TestLoadFromFile(t *testing.T) {
t.Run("Normal", func(t *testing.T) {
name := writeConfig(t, &Config{ID: "agent-id"})
defer removeConfig(t, name)
t.Cleanup(func() { removeConfig(t, name) })

cfg, err := loadFromFile(name)
require.NoError(t, err)
Expand All @@ -61,7 +66,7 @@ func TestLoadFromFile(t *testing.T) {
t.Run("PermissionDenied", func(t *testing.T) {
name := writeConfig(t, &Config{ID: "agent-id"})
require.NoError(t, os.Chmod(name, 0o000))
defer removeConfig(t, name)
t.Cleanup(func() { removeConfig(t, name) })

cfg, err := loadFromFile(name)
require.IsType(t, (*os.PathError)(nil), err)
Expand All @@ -73,7 +78,7 @@ func TestLoadFromFile(t *testing.T) {
t.Run("NotYAML", func(t *testing.T) {
name := writeConfig(t, nil)
require.NoError(t, os.WriteFile(name, []byte(`not YAML`), 0o666)) //nolint:gosec
defer removeConfig(t, name)
t.Cleanup(func() { removeConfig(t, name) })

cfg, err := loadFromFile(name)
require.IsType(t, (*yaml.TypeError)(nil), err)
Expand Down Expand Up @@ -110,7 +115,7 @@ func TestGet(t *testing.T) {
RDSExporter: "/usr/local/percona/pmm2/exporters/rds_exporter",
AzureExporter: "/usr/local/percona/pmm2/exporters/azure_exporter",
VMAgent: "/usr/local/percona/pmm2/exporters/vmagent",
TempDir: os.TempDir(),
TempDir: "/usr/local/percona/pmm2/tmp",
PTSummary: "/usr/local/percona/pmm2/tools/pt-summary",
PTPGSummary: "/usr/local/percona/pmm2/tools/pt-pg-summary",
PTMySQLSummary: "/usr/local/percona/pmm2/tools/pt-mysql-summary",
Expand All @@ -128,16 +133,25 @@ func TestGet(t *testing.T) {
})

t.Run("OnlyConfig", func(t *testing.T) {
name := writeConfig(t, &Config{
var name string
var actual Config

tmpDir := generateTempDirPath(t, pathBaseDefault)
t.Cleanup(func() {
removeConfig(t, name)
})

name = writeConfig(t, &Config{
ID: "agent-id",
ListenAddress: "0.0.0.0",
Server: Server{
Address: "127.0.0.1",
},
Paths: Paths{
TempDir: tmpDir,
},
})
defer removeConfig(t, name)

var actual Config
configFilepath, err := get([]string{
"--config-file=" + name,
}, &actual, logrus.WithField("test", t.Name()))
Expand All @@ -161,7 +175,7 @@ func TestGet(t *testing.T) {
RDSExporter: "/usr/local/percona/pmm2/exporters/rds_exporter",
AzureExporter: "/usr/local/percona/pmm2/exporters/azure_exporter",
VMAgent: "/usr/local/percona/pmm2/exporters/vmagent",
TempDir: os.TempDir(),
TempDir: "/usr/local/percona/pmm2/tmp",
PTSummary: "/usr/local/percona/pmm2/tools/pt-summary",
PTPGSummary: "/usr/local/percona/pmm2/tools/pt-pg-summary",
PTMongoDBSummary: "/usr/local/percona/pmm2/tools/pt-mongodb-summary",
Expand All @@ -178,18 +192,24 @@ func TestGet(t *testing.T) {
assert.Equal(t, name, configFilepath)
})

t.Run("Mix", func(t *testing.T) {
name := writeConfig(t, &Config{
t.Run("BothFlagsAndConfig", func(t *testing.T) {
var name string
var actual Config
tmpDir := generateTempDirPath(t, "/foo/bar")
t.Cleanup(func() {
removeConfig(t, name)
})

name = writeConfig(t, &Config{
ID: "config-id",
Server: Server{
Address: "127.0.0.1",
},
})
defer removeConfig(t, name)

var actual Config
configFilepath, err := get([]string{
"--config-file=" + name,
"--paths-tempdir=" + tmpDir,
"--id=flag-id",
"--log-level=info",
"--debug",
Expand All @@ -214,7 +234,7 @@ func TestGet(t *testing.T) {
RDSExporter: "/usr/local/percona/pmm2/exporters/rds_exporter",
AzureExporter: "/usr/local/percona/pmm2/exporters/azure_exporter",
VMAgent: "/usr/local/percona/pmm2/exporters/vmagent",
TempDir: os.TempDir(),
TempDir: "/foo/bar/tmp",
PTSummary: "/usr/local/percona/pmm2/tools/pt-summary",
PTPGSummary: "/usr/local/percona/pmm2/tools/pt-pg-summary",
PTMySQLSummary: "/usr/local/percona/pmm2/tools/pt-mysql-summary",
Expand All @@ -234,19 +254,25 @@ func TestGet(t *testing.T) {
})

t.Run("MixExportersBase", func(t *testing.T) {
name := writeConfig(t, &Config{
var name string
var actual Config

t.Cleanup(func() {
removeConfig(t, name)
})

name = writeConfig(t, &Config{
ID: "config-id",
Server: Server{
Address: "127.0.0.1",
},
Paths: Paths{
PostgresExporter: "/bar/postgres_exporter",
ProxySQLExporter: "pro_exporter",
TempDir: "tmp",
},
})
defer removeConfig(t, name)

var actual Config
configFilepath, err := get([]string{
"--config-file=" + name,
"--id=flag-id",
Expand Down Expand Up @@ -275,7 +301,7 @@ func TestGet(t *testing.T) {
RDSExporter: "/base/rds_exporter", // default value
AzureExporter: "/base/azure_exporter", // default value
VMAgent: "/base/vmagent", // default value
TempDir: os.TempDir(),
TempDir: "/usr/local/percona/pmm2/tmp",
PTSummary: "/usr/local/percona/pmm2/tools/pt-summary",
PTPGSummary: "/usr/local/percona/pmm2/tools/pt-pg-summary",
PTMongoDBSummary: "/usr/local/percona/pmm2/tools/pt-mongodb-summary",
Expand All @@ -294,7 +320,14 @@ func TestGet(t *testing.T) {
})

t.Run("MixPathsBase", func(t *testing.T) {
name := writeConfig(t, &Config{
var name string
var actual Config

t.Cleanup(func() {
removeConfig(t, name)
})

name = writeConfig(t, &Config{
ID: "config-id",
Server: Server{
Address: "127.0.0.1",
Expand All @@ -304,9 +337,7 @@ func TestGet(t *testing.T) {
ProxySQLExporter: "/base/exporters/pro_exporter",
},
})
defer removeConfig(t, name)

var actual Config
configFilepath, err := get([]string{
"--config-file=" + name,
"--id=flag-id",
Expand Down Expand Up @@ -335,7 +366,7 @@ func TestGet(t *testing.T) {
RDSExporter: "/base/exporters/rds_exporter", // default value
AzureExporter: "/base/exporters/azure_exporter", // default value
VMAgent: "/base/exporters/vmagent", // default value
TempDir: os.TempDir(),
TempDir: "/base/tmp",
PTSummary: "/base/tools/pt-summary",
PTPGSummary: "/base/tools/pt-pg-summary",
PTMongoDBSummary: "/base/tools/pt-mongodb-summary",
Expand All @@ -354,18 +385,24 @@ func TestGet(t *testing.T) {
})

t.Run("MixPathsBaseExporterBase", func(t *testing.T) {
name := writeConfig(t, &Config{
var name string
var actual Config

t.Cleanup(func() {
removeConfig(t, name)
})

name = writeConfig(t, &Config{
ID: "config-id",
Server: Server{
Address: "127.0.0.1",
},
Paths: Paths{
ExportersBase: "/foo/exporters",
TempDir: "/foo/tmp",
},
})
defer removeConfig(t, name)

var actual Config
configFilepath, err := get([]string{
"--config-file=" + name,
"--id=flag-id",
Expand All @@ -392,7 +429,7 @@ func TestGet(t *testing.T) {
RDSExporter: "/foo/exporters/rds_exporter", // default value
AzureExporter: "/foo/exporters/azure_exporter", // default value
VMAgent: "/foo/exporters/vmagent", // default value
TempDir: os.TempDir(),
TempDir: "/foo/tmp",
PTSummary: "/base/tools/pt-summary",
PTPGSummary: "/base/tools/pt-pg-summary",
PTMongoDBSummary: "/base/tools/pt-mongodb-summary",
Expand All @@ -413,14 +450,18 @@ func TestGet(t *testing.T) {
t.Run("NoFile", func(t *testing.T) {
wd, err := os.Getwd()
require.NoError(t, err)

name := t.Name()
tmpDir := generateTempDirPath(t, pathBaseDefault)

var actual Config
configFilepath, err := get([]string{
"--config-file=" + name,
"--paths-tempdir=" + tmpDir,
"--id=flag-id",
"--debug",
}, &actual, logrus.WithField("test", t.Name()))
}, &actual, logrus.WithField("test", name))

expected := Config{
ID: "flag-id",
ListenAddress: "127.0.0.1",
Expand All @@ -436,7 +477,7 @@ func TestGet(t *testing.T) {
RDSExporter: "/usr/local/percona/pmm2/exporters/rds_exporter",
AzureExporter: "/usr/local/percona/pmm2/exporters/azure_exporter",
VMAgent: "/usr/local/percona/pmm2/exporters/vmagent",
TempDir: os.TempDir(),
TempDir: "/usr/local/percona/pmm2/tmp",
PTSummary: "/usr/local/percona/pmm2/tools/pt-summary",
PTPGSummary: "/usr/local/percona/pmm2/tools/pt-pg-summary",
PTMongoDBSummary: "/usr/local/percona/pmm2/tools/pt-mongodb-summary",
Expand Down
Loading

0 comments on commit 46ecd31

Please sign in to comment.