From b558d2036c795a37e7dedd6cb849dd97f30f1995 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 1 Sep 2026 13:53:42 +0300 Subject: [PATCH 1/2] PMM-15304 Fix node type detection PMM Server always runs in a container, but its own Inventory Node was created as "generic": the setup fixtures hardcoded GenericNodeType, both for a single server and for every HA replica. The client-side heuristic behind the node-type default was broken too. checkContainer() only looked for "/docker/" or "/lxc/" in /proc/1/cgroup, which under cgroup v2 holds just "0::/" - so Docker, Podman and Kubernetes all read as a plain host. It now checks the runtime marker files, the "container" and KUBERNETES_SERVICE_HOST variables, and a wider set of cgroup v1 paths. pmm-admin register hardcoded "generic" as its default instead of using the detected ${nodeTypeDefault} that pmm-admin config already honors. Existing Nodes keep the type they were registered with; only fresh installs get the corrected fixture. Signed-off-by: Alex Demidoff --- admin/commands/management/register.go | 2 +- managed/models/database.go | 4 +- managed/models/node_helpers_test.go | 10 +++- managed/services/management/node_test.go | 8 +-- managed/services/qan/client_test.go | 2 +- utils/nodeinfo/nodeinfo.go | 38 ++++++++++++-- utils/nodeinfo/nodeinfo_test.go | 65 +++++++++++++++++++++++- 7 files changed, 114 insertions(+), 15 deletions(-) diff --git a/admin/commands/management/register.go b/admin/commands/management/register.go index 85af143c844..00ddb09ec61 100644 --- a/admin/commands/management/register.go +++ b/admin/commands/management/register.go @@ -48,7 +48,7 @@ type RegisterCommand struct { flags.MetricsModeFlags Address string `name:"node-address" arg:"" default:"${nodeIp}" help:"Node address (autodetected, default: ${nodeIp})"` - NodeType string `arg:"" enum:"generic,container" default:"generic" help:"Node type. One of: [${enum}]. Default: ${default}"` + NodeType string `arg:"" enum:"generic,container" default:"${nodeTypeDefault}" help:"Node type. One of: [${enum}]. Default: ${default}"` NodeName string `arg:"" default:"${hostname}" help:"Node name (autodetected, default: ${hostname})"` MachineID string `default:"${defaultMachineID}" help:"Node machine-id (autodetected, default: ${defaultMachineID})"` Distro string `default:"${distro}" help:"Node OS distribution (autodetected, default: ${distro})"` diff --git a/managed/models/database.go b/managed/models/database.go index ce82f304ca0..d9a7e6627c4 100644 --- a/managed/models/database.go +++ b/managed/models/database.go @@ -1637,7 +1637,7 @@ func setupPMMServerHAAgents(q *reform.Querier, params SetupDBParams) error { "environment": "pmm", } - node, err := createNodeWithID(q, nodeID, GenericNodeType, &CreateNodeParams{ + node, err := createNodeWithID(q, nodeID, ContainerNodeType, &CreateNodeParams{ NodeName: params.HANodeID, Address: LocalhostAddr, CustomLabels: labels, @@ -1669,7 +1669,7 @@ func setupPMMServerHAAgents(q *reform.Querier, params SetupDBParams) error { func setupPMMServerAgents(q *reform.Querier, params SetupDBParams) error { // create PMM Server Node and associated Agents - node, err := createNodeWithID(q, PMMServerNodeID, GenericNodeType, &CreateNodeParams{ + node, err := createNodeWithID(q, PMMServerNodeID, ContainerNodeType, &CreateNodeParams{ NodeName: "pmm-server", Address: LocalhostAddr, IsPMMServerNode: true, diff --git a/managed/models/node_helpers_test.go b/managed/models/node_helpers_test.go index 0f5453408a1..a8c52bf32a2 100644 --- a/managed/models/node_helpers_test.go +++ b/managed/models/node_helpers_test.go @@ -191,7 +191,7 @@ func TestNodeHelpers(t *testing.T) { UpdatedAt: now, }, { NodeID: models.PMMServerNodeID, - NodeType: models.GenericNodeType, + NodeType: models.ContainerNodeType, NodeName: "pmm-server", Address: "127.0.0.1", CreatedAt: now, @@ -216,6 +216,14 @@ func TestNodeHelpers(t *testing.T) { MachineID: new("MySQLNode"), CreatedAt: now, UpdatedAt: now, + }, { + NodeID: models.PMMServerNodeID, + NodeType: models.ContainerNodeType, + NodeName: "pmm-server", + Address: "127.0.0.1", + CreatedAt: now, + UpdatedAt: now, + IsPMMServerNode: true, }, } require.Equal(t, expected, nodes) diff --git a/managed/services/management/node_test.go b/managed/services/management/node_test.go index 9b83eecdc58..d15af6fd4f8 100644 --- a/managed/services/management/node_test.go +++ b/managed/services/management/node_test.go @@ -323,7 +323,7 @@ func TestNodeService(t *testing.T) { Nodes: []*managementv1.UniversalNode{ { NodeId: "pmm-server", - NodeType: "generic", + NodeType: "container", NodeName: "pmm-server", MachineId: "", Distro: "", @@ -401,7 +401,7 @@ func TestNodeService(t *testing.T) { s.r.(*mockAgentsRegistry).On("IsConnected", nodeExporterID).Return(true).Once() res, err := s.ListNodes(ctx, &managementv1.ListNodesRequest{ - NodeType: inventoryv1.NodeType_NODE_TYPE_GENERIC_NODE, + NodeType: inventoryv1.NodeType_NODE_TYPE_CONTAINER_NODE, }) require.NoError(t, err) @@ -409,7 +409,7 @@ func TestNodeService(t *testing.T) { Nodes: []*managementv1.UniversalNode{ { NodeId: "pmm-server", - NodeType: "generic", + NodeType: "container", NodeName: "pmm-server", MachineId: "", Distro: "", @@ -590,7 +590,7 @@ func TestNodeService(t *testing.T) { expected := &managementv1.GetNodeResponse{ Node: &managementv1.UniversalNode{ NodeId: "pmm-server", - NodeType: "generic", + NodeType: "container", NodeName: "pmm-server", MachineId: "", Distro: "", diff --git a/managed/services/qan/client_test.go b/managed/services/qan/client_test.go index 7c9fe2a9fac..bcb36b5bb4d 100644 --- a/managed/services/qan/client_test.go +++ b/managed/services/qan/client_test.go @@ -502,7 +502,7 @@ func TestClientPerformance(t *testing.T) { ServiceName: "test-mysql", NodeId: "pmm-server", NodeName: "pmm-server", - NodeType: "generic", + NodeType: "container", ServiceId: "0d350868-4d85-4884-b972-dff130129c23", ServiceType: "mysql", AgentId: "6b74c6bf-642d-43f0-bee1-0faddd1a2e28", diff --git a/utils/nodeinfo/nodeinfo.go b/utils/nodeinfo/nodeinfo.go index dbb1f279d26..01d2d32c7c2 100644 --- a/utils/nodeinfo/nodeinfo.go +++ b/utils/nodeinfo/nodeinfo.go @@ -18,10 +18,21 @@ package nodeinfo import ( "net" "os" + "path/filepath" "runtime" + "slices" "strings" ) +// containerMarkerFiles are files that container runtimes create inside the container: +// Docker creates /.dockerenv, Podman creates /run/.containerenv. +var containerMarkerFiles = []string{".dockerenv", "run/.containerenv"} + +// containerCgroupMarkers are substrings of the /proc/1/cgroup paths under cgroup v1, where those +// paths carry the runtime name and the container ID. Under cgroup v2 the file usually holds just +// "0::/", so it can confirm a container but never rule one out. +var containerCgroupMarkers = []string{"/docker/", "/lxc/", "/kubepods", "containerd", "crio-", "libpod"} + // NodeInfo contains node information. type NodeInfo struct { Container bool @@ -35,17 +46,34 @@ type NodeInfo struct { // Get returns node information for current node. func Get() *NodeInfo { return &NodeInfo{ - Container: checkContainer(), + Container: checkContainer("/"), Distro: readDistro(), MachineID: readMachineID(), PublicAddress: readPublicAddress(), } } -func checkContainer() bool { - // https://stackoverflow.com/a/20012536 - b, _ := os.ReadFile("/proc/1/cgroup") - return strings.Contains(string(b), "/docker/") || strings.Contains(string(b), "/lxc/") +// checkContainer reports whether the current process runs inside a container. +// The root argument is the filesystem root to probe; it is "/" outside of tests. +func checkContainer(root string) bool { + for _, name := range containerMarkerFiles { + _, err := os.Stat(filepath.Join(root, name)) + if err == nil { + return true + } + } + + // LXC, Podman and systemd-nspawn set "container"; Kubernetes injects its service host into every Pod. + if os.Getenv("container") != "" || os.Getenv("KUBERNETES_SERVICE_HOST") != "" { + return true + } + + b, _ := os.ReadFile(filepath.Join(root, "proc/1/cgroup")) //nolint:gosec + cgroup := string(b) + + return slices.ContainsFunc(containerCgroupMarkers, func(marker string) bool { + return strings.Contains(cgroup, marker) + }) } func readDistro() string { diff --git a/utils/nodeinfo/nodeinfo_test.go b/utils/nodeinfo/nodeinfo_test.go index 270c2efdb6e..f00ff0d048a 100644 --- a/utils/nodeinfo/nodeinfo_test.go +++ b/utils/nodeinfo/nodeinfo_test.go @@ -16,6 +16,8 @@ package nodeinfo import ( "net" + "os" + "path/filepath" "runtime" "strings" "testing" @@ -28,7 +30,6 @@ func TestGet(t *testing.T) { t.Parallel() info := Get() - require.False(t, info.Container, "not expected to be run inside a container") assert.Equal(t, runtime.GOOS, info.Distro) // all our test environments have IPv4 addresses @@ -38,3 +39,65 @@ func TestGet(t *testing.T) { assert.False(t, strings.HasSuffix(info.MachineID, "\n"), "%q", info.MachineID) } + +func TestCheckContainer(t *testing.T) { + for _, tc := range []struct { + name string + files map[string]string + env map[string]string + expected bool + }{ + { + name: "host with cgroup v2", + files: map[string]string{"proc/1/cgroup": "0::/init.scope\n"}, + }, { + name: "host with cgroup v1", + files: map[string]string{"proc/1/cgroup": "1:name=systemd:/init.scope\n0::/init.scope\n"}, + }, { + // the case PMM Server itself hits: cgroup v2 says nothing, only the marker file does + name: "docker with cgroup v2", + files: map[string]string{".dockerenv": "", "proc/1/cgroup": "0::/\n"}, + expected: true, + }, { + name: "docker with cgroup v1", + files: map[string]string{"proc/1/cgroup": "1:name=systemd:/docker/dc4b1a5cb7fd\n"}, + expected: true, + }, { + name: "podman", + files: map[string]string{"run/.containerenv": "engine=\"podman-5.4.0\"\n", "proc/1/cgroup": "0::/\n"}, + expected: true, + }, { + name: "lxc", + files: map[string]string{"proc/1/cgroup": "0::/\n"}, + env: map[string]string{"container": "lxc"}, + expected: true, + }, { + name: "kubernetes pod with cgroup v2", + files: map[string]string{"proc/1/cgroup": "0::/\n"}, + env: map[string]string{"KUBERNETES_SERVICE_HOST": "10.96.0.1"}, + expected: true, + }, { + name: "kubernetes pod with cgroup v1", + files: map[string]string{"proc/1/cgroup": "1:name=systemd:/kubepods/besteffort/pod9f4a\n"}, + expected: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + // keep the outcome independent of the environment the tests themselves run in + t.Setenv("container", "") + t.Setenv("KUBERNETES_SERVICE_HOST", "") + for name, value := range tc.env { + t.Setenv(name, value) + } + + root := t.TempDir() + for name, content := range tc.files { + path := filepath.Join(root, name) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) + } + + assert.Equal(t, tc.expected, checkContainer(root)) + }) + } +} From 54f3ea625a9c9b20877790ffe3581da6b0dadab8 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 1 Sep 2026 14:41:07 +0300 Subject: [PATCH 2/2] PMM-15304 Detect containers managed by systemd systemd sets "container" in PID 1's environment only and strips it from the services it starts, so an agent running as a unit inside LXC or LXD on cgroup v2 saw no signal at all. Check /run/systemd/container, which systemd writes when it boots as PID 1 in a container. Also match /system.slice/docker-.scope, the path form the systemd cgroup driver produces, which "/docker/" cannot. Signed-off-by: Alex Demidoff --- utils/nodeinfo/nodeinfo.go | 19 ++++++++++++------- utils/nodeinfo/nodeinfo_test.go | 10 ++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/utils/nodeinfo/nodeinfo.go b/utils/nodeinfo/nodeinfo.go index 01d2d32c7c2..015f89c9056 100644 --- a/utils/nodeinfo/nodeinfo.go +++ b/utils/nodeinfo/nodeinfo.go @@ -24,14 +24,18 @@ import ( "strings" ) -// containerMarkerFiles are files that container runtimes create inside the container: -// Docker creates /.dockerenv, Podman creates /run/.containerenv. -var containerMarkerFiles = []string{".dockerenv", "run/.containerenv"} +// containerMarkerFiles are files created inside the container: Docker creates /.dockerenv, Podman +// creates /run/.containerenv, and systemd running as PID 1 in a container (LXC, LXD, +// systemd-nspawn) writes the runtime name to /run/systemd/container. The last one matters because +// systemd does not pass its own "container" variable on to the services it starts, so an agent +// running as a unit cannot see it. +var containerMarkerFiles = []string{".dockerenv", "run/.containerenv", "run/systemd/container"} // containerCgroupMarkers are substrings of the /proc/1/cgroup paths under cgroup v1, where those -// paths carry the runtime name and the container ID. Under cgroup v2 the file usually holds just -// "0::/", so it can confirm a container but never rule one out. -var containerCgroupMarkers = []string{"/docker/", "/lxc/", "/kubepods", "containerd", "crio-", "libpod"} +// paths carry the runtime name and the container ID; "docker-" catches the systemd cgroup driver, +// which nests containers as /system.slice/docker-.scope. Under cgroup v2 the file usually +// holds just "0::/", so it can confirm a container but never rule one out. +var containerCgroupMarkers = []string{"/docker/", "docker-", "/lxc/", "/kubepods", "containerd", "crio-", "libpod"} // NodeInfo contains node information. type NodeInfo struct { @@ -63,7 +67,8 @@ func checkContainer(root string) bool { } } - // LXC, Podman and systemd-nspawn set "container"; Kubernetes injects its service host into every Pod. + // Podman and LXC set "container" for the processes they start; Kubernetes injects its service + // host into every Pod. if os.Getenv("container") != "" || os.Getenv("KUBERNETES_SERVICE_HOST") != "" { return true } diff --git a/utils/nodeinfo/nodeinfo_test.go b/utils/nodeinfo/nodeinfo_test.go index f00ff0d048a..d74e71bf8df 100644 --- a/utils/nodeinfo/nodeinfo_test.go +++ b/utils/nodeinfo/nodeinfo_test.go @@ -66,11 +66,21 @@ func TestCheckContainer(t *testing.T) { name: "podman", files: map[string]string{"run/.containerenv": "engine=\"podman-5.4.0\"\n", "proc/1/cgroup": "0::/\n"}, expected: true, + }, { + name: "docker with the systemd cgroup driver", + files: map[string]string{"proc/1/cgroup": "1:name=systemd:/system.slice/docker-dc4b1a5cb7fd.scope\n"}, + expected: true, }, { name: "lxc", files: map[string]string{"proc/1/cgroup": "0::/\n"}, env: map[string]string{"container": "lxc"}, expected: true, + }, { + // systemd strips its own "container" variable from the services it starts, so an agent + // running as a unit only has the file to go by + name: "lxc with an agent started by systemd", + files: map[string]string{"run/systemd/container": "lxc\n", "proc/1/cgroup": "0::/\n"}, + expected: true, }, { name: "kubernetes pod with cgroup v2", files: map[string]string{"proc/1/cgroup": "0::/\n"},