From dcb2ce367aff835ad75ce325ea21c60778d02de4 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Sun, 2 Aug 2026 16:20:16 +0300 Subject: [PATCH 1/2] PMM-15285 Remove dead supervisord event-scraping code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pmm-managed ran a goroutine that shelled out to `supervisorctl maintail -f`, scanned every line of supervisord's main log against eight regexes and dispatched typed events to subscribers. There were no subscribers: PMM-14132 (#4110) removed `subscribe` and `UpdateRunning`, the only writer to `subs` and the only reader of `lastEvents`, but left the producer side in place. Remove maintail.go and its test, the `Run` loop, the `subs`/`lastEvents`/ `eventsM` fields, the `sub` type and `parseStatus`, plus the call sites in pmm-managed's main and the devcontainer test. This also drops the only place PMM depended on supervisord's log text, which would otherwise have to be ported to any process-manager replacement. Folded in two adjacent cleanups: - Remove [unix_http_server] and [supervisorctl] from the generated pmm.ini (existing TODO). Both duplicate /etc/supervisord.conf; the removed `chmod = 0700` is supervisord's default, verified identical (0700 pmm:root) on a running container built with this change. - Pass the program name to supervisorctl as its own argv element in the encryption rotation helpers. This is not a live bug — supervisorctl rejoins its argv, so the single-string form works today — but it only works by accident of that quirk and breaks under any other CLI. --- managed/cmd/pmm-managed/main.go | 4 - .../encryption/encryption_rotation.go | 6 +- .../services/supervisord/devcontainer_test.go | 5 - managed/services/supervisord/maintail.go | 93 --------------- managed/services/supervisord/maintail_test.go | 107 ------------------ managed/services/supervisord/pmm_config.go | 12 +- managed/services/supervisord/supervisord.go | 107 ------------------ .../services/supervisord/supervisord_test.go | 13 --- .../supervisord.d/pmm-ch_low_memory.ini | 9 -- .../supervisord.d/pmm-db_disabled.ini | 9 -- .../testdata/supervisord.d/pmm-db_enabled.ini | 9 -- 11 files changed, 4 insertions(+), 370 deletions(-) delete mode 100644 managed/services/supervisord/maintail.go delete mode 100644 managed/services/supervisord/maintail_test.go diff --git a/managed/cmd/pmm-managed/main.go b/managed/cmd/pmm-managed/main.go index 2468e9778a..227f86921c 100644 --- a/managed/cmd/pmm-managed/main.go +++ b/managed/cmd/pmm-managed/main.go @@ -1154,10 +1154,6 @@ func main() { //nolint:gocognit,maintidx,cyclop return nil })) - wg.Go(func() { - supervisord.Run(ctx) - }) - wg.Go(func() { updater.Run(ctx) }) diff --git a/managed/services/encryption/encryption_rotation.go b/managed/services/encryption/encryption_rotation.go index f9bbb3aeb0..c9cee8c514 100644 --- a/managed/services/encryption/encryption_rotation.go +++ b/managed/services/encryption/encryption_rotation.go @@ -71,7 +71,7 @@ func startPMMServer() error { return nil } - cmd := exec.Command("supervisorctl", "start pmm-managed") + cmd := exec.Command("supervisorctl", "start", "pmm-managed") //nolint:noctx output, err := cmd.CombinedOutput() if err != nil { return fmt.Errorf("%w: %s", err, output) @@ -90,7 +90,7 @@ func stopPMMServer() error { return nil } - cmd := exec.Command("supervisorctl", "stop pmm-managed") + cmd := exec.Command("supervisorctl", "stop", "pmm-managed") //nolint:noctx output, err := cmd.CombinedOutput() if err != nil { return fmt.Errorf("%w: %s", err, output) @@ -104,7 +104,7 @@ func stopPMMServer() error { } func pmmServerStatus(status string) bool { - cmd := exec.Command("supervisorctl", "status pmm-managed") + cmd := exec.Command("supervisorctl", "status", "pmm-managed") //nolint:noctx output, _ := cmd.CombinedOutput() return strings.Contains(string(output), strings.ToUpper(status)) diff --git a/managed/services/supervisord/devcontainer_test.go b/managed/services/supervisord/devcontainer_test.go index 40e91e8e0d..724191226c 100644 --- a/managed/services/supervisord/devcontainer_test.go +++ b/managed/services/supervisord/devcontainer_test.go @@ -16,7 +16,6 @@ package supervisord import ( - "context" "os" "path/filepath" "testing" @@ -38,10 +37,6 @@ func TestDevContainer(t *testing.T) { s := New("/etc/supervisord.d", &models.Params{VMParams: vmParams, PGParams: &models.PGParams{}, HAParams: &models.HAParams{}}) require.NotEmpty(t, s.supervisorctlPath) - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() - go s.Run(ctx) - // restore original files after test originals := make(map[string][]byte) matches, err := filepath.Glob("/etc/supervisord.d/*.ini") diff --git a/managed/services/supervisord/maintail.go b/managed/services/supervisord/maintail.go deleted file mode 100644 index cbc7db1df6..0000000000 --- a/managed/services/supervisord/maintail.go +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright (C) 2023 Percona LLC -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -package supervisord - -import ( - "regexp" - "strings" - "time" -) - -type eventType string - -const ( - // mirror http://supervisord.org/subprocess.html#process-states - stopped eventType = "STOPPED" - stopping eventType = "STOPPING" - starting eventType = "STARTING" - running eventType = "RUNNING" - exitedExpected eventType = "EXITED (expected)" - exitedUnexpected eventType = "EXITED (unexpected)" - fatal eventType = "FATAL" - - logReopen eventType = "logreopen" -) - -var ( - stoppedRE = regexp.MustCompile(`^stopped\: ([\w-]+) \(exit status \d+\)$`) - stoppingRE = regexp.MustCompile(`^waiting for ([\w-]+) to stop$`) - startingRE = regexp.MustCompile(`^spawned\: '([\w-]+)' with pid \d+$`) - runningRE = regexp.MustCompile(`^success\: ([\w-]+) entered RUNNING state, process has stayed up for > than \d+ seconds \(startsecs\)$`) - exitedExpectedRE = regexp.MustCompile(`^exited\: ([\w-]+) \(exit status \d+; expected\)$`) - exitedUnexpectedRE = regexp.MustCompile(`^exited\: ([\w-]+) \(exit status \d+; not expected\)$`) - fatalRE = regexp.MustCompile(`^gave up\: ([\w-]+) entered FATAL state, too many start retries too quickly$`) - logReopenRE = regexp.MustCompile(`^([\w-]+) logreopen$`) - - events = map[*regexp.Regexp]eventType{ - stoppedRE: stopped, - stoppingRE: stopping, - startingRE: starting, - runningRE: running, - exitedExpectedRE: exitedExpected, - exitedUnexpectedRE: exitedUnexpected, - fatalRE: fatal, - logReopenRE: logReopen, - } -) - -// event represents supervisord program event. -type event struct { - Time time.Time - Type eventType - Program string -} - -// parseEvent returns parsed event from supervisord maintail line, or nil. -func parseEvent(line string) *event { - parts := strings.SplitN(line, " ", 4) //nolint:mnd - if len(parts) != 4 { //nolint:mnd - return nil - } - - // see https://github.com/golang/go/issues/6189 - ts := strings.Replace(parts[0]+" "+parts[1], ",", ".", 1) - t, err := time.Parse("2006-01-02 15:04:05.000", ts) - if err != nil { - return nil - } - - for re, typ := range events { - if m := re.FindStringSubmatch(parts[3]); m != nil { - return &event{ - Time: t, - Type: typ, - Program: m[1], - } - } - } - - return nil -} diff --git a/managed/services/supervisord/maintail_test.go b/managed/services/supervisord/maintail_test.go deleted file mode 100644 index 1461b74b08..0000000000 --- a/managed/services/supervisord/maintail_test.go +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright (C) 2023 Percona LLC -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -package supervisord - -import ( - "strings" - "testing" - "time" - - "github.com/stretchr/testify/assert" -) - -func TestParseEvent(t *testing.T) { - t.Parallel() - t.Run("Normal", func(t *testing.T) { - t.Parallel() - - log := strings.Split(` - 2019-08-08 17:09:57,284 INFO received SIGUSR2 indicating log reopen request - 2019-08-08 17:09:57,284 INFO supervisord logreopen - 2019-08-08 17:09:57,854 INFO waiting for pmm-managed to stop - 2019-08-08 17:09:59,854 INFO waiting for pmm-managed to stop - 2019-08-08 17:10:00,863 INFO stopped: pmm-managed (exit status 0) - 2019-08-08 17:10:01,932 INFO spawned: 'pmm-managed' with pid 13191 - 2019-08-08 17:10:03,006 INFO success: pmm-managed entered RUNNING state, process has stayed up for > than 1 seconds (startsecs) - 2019-08-08 17:10:09,878 INFO reaped unknown pid 12411 - 2019-08-08 17:10:27,686 INFO spawned: 'dashboard-upgrade' with pid 13888 - 2019-08-08 17:10:27,686 INFO success: dashboard-upgrade entered RUNNING state, process has stayed up for > than 0 seconds (startsecs) - 2019-08-08 17:10:27,761 INFO exited: dashboard-upgrade (exit status 0; expected) - `, "\n") - - var actual []*event - for _, line := range log { - line = strings.TrimSpace(line) - if line == "" { - continue - } - if e := parseEvent(line); e != nil { - actual = append(actual, e) - } - } - expected := []*event{ - {Time: time.Date(2019, 8, 8, 17, 9, 57, 284000000, time.UTC), Type: logReopen, Program: "supervisord"}, - {Time: time.Date(2019, 8, 8, 17, 9, 57, 854000000, time.UTC), Type: stopping, Program: "pmm-managed"}, - {Time: time.Date(2019, 8, 8, 17, 9, 59, 854000000, time.UTC), Type: stopping, Program: "pmm-managed"}, - {Time: time.Date(2019, 8, 8, 17, 10, 0, 863000000, time.UTC), Type: stopped, Program: "pmm-managed"}, - {Time: time.Date(2019, 8, 8, 17, 10, 1, 932000000, time.UTC), Type: starting, Program: "pmm-managed"}, - {Time: time.Date(2019, 8, 8, 17, 10, 3, 6000000, time.UTC), Type: running, Program: "pmm-managed"}, - {Time: time.Date(2019, 8, 8, 17, 10, 27, 686000000, time.UTC), Type: starting, Program: "dashboard-upgrade"}, - {Time: time.Date(2019, 8, 8, 17, 10, 27, 686000000, time.UTC), Type: running, Program: "dashboard-upgrade"}, - {Time: time.Date(2019, 8, 8, 17, 10, 27, 761000000, time.UTC), Type: exitedExpected, Program: "dashboard-upgrade"}, - } - assert.Equal(t, expected, actual) - }) - - t.Run("Fatal", func(t *testing.T) { - t.Parallel() - - log := strings.Split(` - 2019-08-09 09:18:25,667 INFO spawned: 'pmm-init' with pid 11410 - 2019-08-09 09:18:26,539 INFO exited: pmm-init (exit status 0; not expected) - 2019-08-09 09:18:27,543 INFO spawned: 'pmm-init' with pid 11421 - 2019-08-09 09:18:28,324 INFO exited: pmm-init (exit status 0; not expected) - 2019-08-09 09:18:30,335 INFO spawned: 'pmm-init' with pid 11432 - 2019-08-09 09:18:31,109 INFO exited: pmm-init (exit status 0; not expected) - 2019-08-09 09:18:34,119 INFO spawned: 'pmm-init' with pid 11443 - 2019-08-09 09:18:34,883 INFO exited: pmm-init (exit status 0; not expected) - 2019-08-09 09:18:35,885 INFO gave up: pmm-init entered FATAL state, too many start retries too quickly - `, "\n") - - var actual []*event - for _, line := range log { - line = strings.TrimSpace(line) - if line == "" { - continue - } - if e := parseEvent(line); e != nil { - actual = append(actual, e) - } - } - expected := []*event{ - {Time: time.Date(2019, 8, 9, 9, 18, 25, 667000000, time.UTC), Type: starting, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 26, 539000000, time.UTC), Type: exitedUnexpected, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 27, 543000000, time.UTC), Type: starting, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 28, 324000000, time.UTC), Type: exitedUnexpected, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 30, 335000000, time.UTC), Type: starting, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 31, 109000000, time.UTC), Type: exitedUnexpected, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 34, 119000000, time.UTC), Type: starting, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 34, 883000000, time.UTC), Type: exitedUnexpected, Program: "pmm-init"}, - {Time: time.Date(2019, 8, 9, 9, 18, 35, 885000000, time.UTC), Type: fatal, Program: "pmm-init"}, - } - assert.Equal(t, expected, actual) - }) -} diff --git a/managed/services/supervisord/pmm_config.go b/managed/services/supervisord/pmm_config.go index 71606d7b1d..ecbcabed24 100644 --- a/managed/services/supervisord/pmm_config.go +++ b/managed/services/supervisord/pmm_config.go @@ -85,17 +85,7 @@ func saveConfig(path string, cfg []byte) (err error) { return err } -// TODO: remove [unix_http_server] and [supervisorctl] as they duplicate supervisord.conf. -var pmmTemplate = template.Must(template.New("").Option("missingkey=error").Parse(`[unix_http_server] -chmod = 0700 -username = dummy -password = dummy - -[supervisorctl] -username = dummy -password = dummy - -[program:pmm-init] +var pmmTemplate = template.Must(template.New("").Option("missingkey=error").Parse(`[program:pmm-init] command = /usr/bin/ansible-playbook /opt/ansible/pmm-docker/init.yml directory = / autorestart = unexpected diff --git a/managed/services/supervisord/supervisord.go b/managed/services/supervisord/supervisord.go index 48833ed0e9..ecb86a6981 100644 --- a/managed/services/supervisord/supervisord.go +++ b/managed/services/supervisord/supervisord.go @@ -17,9 +17,7 @@ package supervisord import ( - "bufio" "bytes" - "context" "errors" "fmt" "io/fs" @@ -28,12 +26,10 @@ import ( "os/exec" "path/filepath" "reflect" - "slices" "strconv" "strings" "sync" "text/template" - "time" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -68,10 +64,6 @@ type Service struct { supervisorctlPath string l *logrus.Entry - eventsM sync.Mutex - subs map[chan *event]sub - lastEvents map[string]eventType - supervisordConfigsM sync.Mutex vmParams *models.VictoriaMetricsParams @@ -79,11 +71,6 @@ type Service struct { haParams *models.HAParams } -type sub struct { - program string - eventTypes []eventType -} - // values from supervisord configuration. const ( pmmConfig = "/etc/supervisord.d/pmm.ini" @@ -96,88 +83,12 @@ func New(configDir string, params *models.Params) *Service { configDir: configDir, supervisorctlPath: path, l: logrus.WithField("component", "supervisord"), - subs: make(map[chan *event]sub), - lastEvents: make(map[string]eventType), vmParams: params.VMParams, pgParams: params.PGParams, haParams: params.HAParams, } } -// Run reads supervisord's log (maintail) and sends events to subscribers. -func (s *Service) Run(ctx context.Context) { //nolint:gocognit - if s.supervisorctlPath == "" { - s.l.Errorf("supervisorctl not found, updates are disabled.") - return - } - - var lastEvent *event - for ctx.Err() == nil { - cmd := exec.CommandContext(ctx, s.supervisorctlPath, "maintail", "-f") //nolint:gosec - cmdLine := strings.Join(cmd.Args, " ") - pdeathsig.Set(cmd, unix.SIGKILL) - stdout, err := cmd.StdoutPipe() - if err != nil { - s.l.Errorf("%s: StdoutPipe failed: %s", cmdLine, err) - time.Sleep(time.Second) - continue - } - - err = cmd.Start() - if err != nil { - s.l.Errorf("%s: Start failed: %s", cmdLine, err) - time.Sleep(time.Second) - continue - } - - scanner := bufio.NewScanner(stdout) - for scanner.Scan() { - e := parseEvent(scanner.Text()) - if e == nil { - continue - } - s.l.Debugf("Got event: %+v", e) - - // skip old events (and events with exactly the same time as old events) if maintail was restarted - if lastEvent != nil && !lastEvent.Time.Before(e.Time) { - continue - } - lastEvent = e - - s.eventsM.Lock() - - s.lastEvents[e.Program] = e.Type - - var toDelete []chan *event - for ch, sub := range s.subs { - if e.Program == sub.program { - if slices.Contains(sub.eventTypes, e.Type) { - ch <- e - close(ch) - toDelete = append(toDelete, ch) - } - } - } - - for _, ch := range toDelete { - delete(s.subs, ch) - } - - s.eventsM.Unlock() - } - - err = scanner.Err() - if err != nil { - s.l.Errorf("Scanner: %s", err) - } - - err = cmd.Wait() - if err != nil { - s.l.Errorf("%s: wait failed: %s", cmdLine, err) - } - } -} - // UpdateConfiguration updates VictoriaMetrics, Grafana and qan-api2 configurations, restarting them if needed. func (s *Service) UpdateConfiguration(settings *models.Settings) error { if s.supervisorctlPath == "" { @@ -424,24 +335,6 @@ func (s *Service) supervisorctl(args ...string) error { return nil } -// parseStatus parses `supervisorctl status ` output, returns true if is running, -// false if definitely not, and nil if status can't be determined. -func parseStatus(status string) *bool { - if f := strings.Fields(status); len(f) > 1 { - switch status := f[1]; status { - case "FATAL", "STOPPED": // will not be restarted - return new(false) - case "STARTING", "RUNNING", "BACKOFF", "STOPPING": - return new(true) - case "EXITED": - // it might be restarted - we need to inspect last event - default: - // something else - we need to inspect last event - } - } - return nil -} - // reload asks supervisord to reload configuration. func (s *Service) reload(name string) error { err := s.supervisorctl("reread") diff --git a/managed/services/supervisord/supervisord_test.go b/managed/services/supervisord/supervisord_test.go index d0943888b2..c262aa2860 100644 --- a/managed/services/supervisord/supervisord_test.go +++ b/managed/services/supervisord/supervisord_test.go @@ -114,16 +114,3 @@ func TestConfigVictoriaMetricsEnvvars(t *testing.T) { }) } } - -func TestParseStatus(t *testing.T) { - t.Parallel() - - for str, expected := range map[string]*bool{ - `pmm-agent STOPPED Sep 20 08:55 AM`: new(false), - `pmm-managed RUNNING pid 826, uptime 0:19:36`: new(true), - `pmm-init EXITED Sep 20 07:42 AM`: nil, - `pmm-init STARTING`: new(true), // no last column in that case - } { - assert.Equal(t, expected, parseStatus(str), "%q", str) - } -} diff --git a/managed/testdata/supervisord.d/pmm-ch_low_memory.ini b/managed/testdata/supervisord.d/pmm-ch_low_memory.ini index 0421e193c0..1df9ed069c 100644 --- a/managed/testdata/supervisord.d/pmm-ch_low_memory.ini +++ b/managed/testdata/supervisord.d/pmm-ch_low_memory.ini @@ -1,12 +1,3 @@ -[unix_http_server] -chmod = 0700 -username = dummy -password = dummy - -[supervisorctl] -username = dummy -password = dummy - [program:pmm-init] command = /usr/bin/ansible-playbook /opt/ansible/pmm-docker/init.yml directory = / diff --git a/managed/testdata/supervisord.d/pmm-db_disabled.ini b/managed/testdata/supervisord.d/pmm-db_disabled.ini index be1256c78c..6594764f05 100644 --- a/managed/testdata/supervisord.d/pmm-db_disabled.ini +++ b/managed/testdata/supervisord.d/pmm-db_disabled.ini @@ -1,12 +1,3 @@ -[unix_http_server] -chmod = 0700 -username = dummy -password = dummy - -[supervisorctl] -username = dummy -password = dummy - [program:pmm-init] command = /usr/bin/ansible-playbook /opt/ansible/pmm-docker/init.yml directory = / diff --git a/managed/testdata/supervisord.d/pmm-db_enabled.ini b/managed/testdata/supervisord.d/pmm-db_enabled.ini index d5a98106f4..ac3deb5311 100644 --- a/managed/testdata/supervisord.d/pmm-db_enabled.ini +++ b/managed/testdata/supervisord.d/pmm-db_enabled.ini @@ -1,12 +1,3 @@ -[unix_http_server] -chmod = 0700 -username = dummy -password = dummy - -[supervisorctl] -username = dummy -password = dummy - [program:pmm-init] command = /usr/bin/ansible-playbook /opt/ansible/pmm-docker/init.yml directory = / From 13e79a883e281205b86720b4b583630b887f0435 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Sun, 2 Aug 2026 22:29:49 +0300 Subject: [PATCH 2/2] PMM-7 Fix the user permissions for unix_http_server section --- build/ansible/roles/supervisord/tasks/main.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/build/ansible/roles/supervisord/tasks/main.yml b/build/ansible/roles/supervisord/tasks/main.yml index c9be2f092d..d615db1899 100644 --- a/build/ansible/roles/supervisord/tasks/main.yml +++ b/build/ansible/roles/supervisord/tasks/main.yml @@ -26,6 +26,20 @@ option: file value: /run/supervisor/supervisor.sock +- name: Modify supervisord.conf + ini_file: + dest: /etc/supervisord.conf + section: unix_http_server + option: username + value: dummy + +- name: Modify supervisord.conf + ini_file: + dest: /etc/supervisord.conf + section: unix_http_server + option: password + value: dummy + - name: Modify supervisord.conf ini_file: dest: /etc/supervisord.conf