From 46ecd31e6a0ea1597bf01f44add79465dacfaf14 Mon Sep 17 00:00:00 2001 From: Alex Tymchuk Date: Mon, 24 Jul 2023 19:00:12 +0300 Subject: [PATCH] PMM-11658 move the exporter configs away from /tmp (#2341) * 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 --- .github/workflows/admin.yml | 1 - agent/agents/supervisor/supervisor.go | 6 ++ agent/commands/run.go | 6 +- agent/config/config.go | 28 ++++++-- agent/config/config_test.go | 95 +++++++++++++++++++-------- build/docker/server/Dockerfile.el9 | 13 ++-- managed/cmd/pmm-managed/main.go | 16 +++-- 7 files changed, 118 insertions(+), 47 deletions(-) diff --git a/.github/workflows/admin.yml b/.github/workflows/admin.yml index fbb8459402..fbd0d26e49 100644 --- a/.github/workflows/admin.yml +++ b/.github/workflows/admin.yml @@ -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 diff --git a/agent/agents/supervisor/supervisor.go b/agent/agents/supervisor/supervisor.go index 3538644ba4..f33595efa6 100644 --- a/agent/agents/supervisor/supervisor.go +++ b/agent/agents/supervisor/supervisor.go @@ -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 diff --git a/agent/commands/run.go b/agent/commands/run.go index f5acd13386..22878c350b 100644 --- a/agent/commands/run.go +++ b/agent/commands/run.go @@ -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.") @@ -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() }() @@ -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) diff --git a/agent/config/config.go b/agent/config/config.go index eebca14167..0ca42d6c96 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -17,6 +17,7 @@ package config import ( "fmt" + "io/fs" "net" "net/url" "os" @@ -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" @@ -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 { @@ -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 { @@ -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", @@ -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) } @@ -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 @@ -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) } @@ -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 diff --git a/agent/config/config_test.go b/agent/config/config_test.go index 7099e2688b..dad8c43458 100644 --- a/agent/config/config_test.go +++ b/agent/config/config_test.go @@ -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) @@ -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) @@ -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) @@ -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", @@ -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())) @@ -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", @@ -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", @@ -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", @@ -234,7 +254,14 @@ 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", @@ -242,11 +269,10 @@ func TestGet(t *testing.T) { 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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", diff --git a/build/docker/server/Dockerfile.el9 b/build/docker/server/Dockerfile.el9 index 603a341565..925e4e6c08 100644 --- a/build/docker/server/Dockerfile.el9 +++ b/build/docker/server/Dockerfile.el9 @@ -6,6 +6,7 @@ ARG BUILD_DATE ENV LANG=en_US.utf8 ENV LC_ALL=en_US.utf8 ENV GF_PLUGIN_DIR=/srv/grafana/plugins +ENV PS1="[\u@\h \W] # " LABEL org.opencontainers.image.created ${BUILD_DATE} LABEL org.opencontainers.image.licenses AGPL-3.0 @@ -26,10 +27,10 @@ WORKDIR /opt # yum -y install epel-release RUN microdnf -y install yum && yum -y install python3-pip && \ - yum -y install oracle-epel-release-el9 ansible-core && \ - yum -y install epel-release && \ - yum -y install glibc-langpack-en && \ - yum -y install ansible vi + yum -y install oracle-epel-release-el9 ansible-core && \ + yum -y install epel-release && \ + yum -y install glibc-langpack-en && \ + yum -y install ansible vi COPY RPMS /tmp/RPMS COPY gitCommit /tmp/gitCommit @@ -38,8 +39,8 @@ COPY ansible /opt/ansible # NOTE: this needs to be refactored, since some of the playbooks are duplicates RUN cp -r /opt/ansible/roles /opt/ansible/pmm2-docker/roles RUN ansible-playbook -vvv -i 'localhost,' -c local /opt/ansible/pmm2-docker/main.yml \ - && ansible-playbook -vvv -i 'localhost,' -c local /usr/share/pmm-update/ansible/playbook/tasks/update.yml \ - && ansible-playbook -vvv -i 'localhost,' -c local /opt/ansible/pmm2/post-build-actions.yml + && ansible-playbook -vvv -i 'localhost,' -c local /usr/share/pmm-update/ansible/playbook/tasks/update.yml \ + && ansible-playbook -vvv -i 'localhost,' -c local /opt/ansible/pmm2/post-build-actions.yml COPY entrypoint.sh /opt/entrypoint.sh HEALTHCHECK --interval=3s --timeout=2s --start-period=10s --retries=3 CMD curl -f http://127.0.0.1/v1/readyz || exit 1 diff --git a/managed/cmd/pmm-managed/main.go b/managed/cmd/pmm-managed/main.go index 962cd55166..2eac6ed7cf 100644 --- a/managed/cmd/pmm-managed/main.go +++ b/managed/cmd/pmm-managed/main.go @@ -49,6 +49,7 @@ import ( "golang.org/x/sync/semaphore" "golang.org/x/sys/unix" "google.golang.org/grpc" + "google.golang.org/grpc/backoff" channelz "google.golang.org/grpc/channelz/service" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/grpclog" @@ -558,15 +559,15 @@ func setup(ctx context.Context, deps *setupDeps) bool { deps.l.Infof("Updating supervisord configuration...") settings, err := models.GetSettings(db.Querier) if err != nil { - deps.l.Warnf("Failed to get settings: %+v.", err) + deps.l.Warnf("Failed to get settings: %s.", err) return false } ssoDetails, err := models.GetPerconaSSODetails(ctx, db.Querier) - if err != nil { - deps.l.Warnf("Failed to get Percona SSO Details: %+v.", err) + if err != nil && !errors.Is(err, models.ErrNotConnectedToPortal) { + deps.l.Warnf("Failed to get Percona SSO Details: %s.", err) } if err = deps.supervisord.UpdateConfiguration(settings, ssoDetails); err != nil { - deps.l.Warnf("Failed to update supervisord configuration: %+v.", err) + deps.l.Warnf("Failed to update supervisord configuration: %s.", err) return false } @@ -596,9 +597,14 @@ func setup(ctx context.Context, deps *setupDeps) bool { } func getQANClient(ctx context.Context, sqlDB *sql.DB, dbName, qanAPIAddr string) *qan.Client { + bc := backoff.DefaultConfig + bc.MaxDelay = time.Second + opts := []grpc.DialOption{ grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithBackoffMaxDelay(time.Second), //nolint:staticcheck + grpc.WithConnectParams(grpc.ConnectParams{ + Backoff: bc, + }), grpc.WithUserAgent("pmm-managed/" + version.Version), grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(gRPCMessageMaxSize)), }