Skip to content

Commit

Permalink
Fix behavior of cgroups path discovery (#136)
Browse files Browse the repository at this point in the history
## What does this PR do?

Fixes #132
and fixes
#139

This sanitizes the "relative" namespace mounts that we get when we
monitor the host system from within newer versions of docker.

This also adds much more complex logic for setting the v2 rootpath we
use for fetching metrics from v2 cgroups paths.

This also aggressively cleans up the unit tests in `cgroup` because they
were a mess

With this change, cgroups metric collection works properly on docker
configs where where the container is using a private namespace. This
value is configurable from docker 1.41+

## Checklist


- [x] My code follows the style guidelines of this project
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added an entry in `CHANGELOG.md`


## How to test this PR

- run `go test ./tests` from the repo root
- build metricbeat with this version of `elastic-agent-system-metrics`,
run `system/process` metrics.
  • Loading branch information
fearful-symmetry authored May 1, 2024
1 parent cea4f3e commit 182b3ee
Show file tree
Hide file tree
Showing 236 changed files with 627 additions and 1,310 deletions.
2 changes: 1 addition & 1 deletion .buildkite/scripts/system-container-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ install_go_dependencies
# curl -fsSL https://get.docker.com -o get-docker.sh
# sudo sh ./get-docker.sh --version $DOCKER_VERSION

go test -v ./tests
go test -timeout 20m -v ./tests
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
*beat/data
x-pack/functionbeat/pkg

metric/system/cgroup/testdata/amzn2
metric/system/cgroup/testdata/docker
metric/system/cgroup/testdata/docker2
metric/system/cgroup/testdata/ubuntu1804

# Files
.DS_Store
/beats.iml
Expand Down
18 changes: 11 additions & 7 deletions dev-tools/systemtests/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ type testCase struct {
}

func (tc testCase) String() string {
return fmt.Sprintf("%s-priv:%v-user:%s", tc.nsmode, tc.priv, tc.user)
return fmt.Sprintf("nsmode:%s_priv:%v_user:%s", tc.nsmode, tc.priv, tc.user)
}

// CreateAndRunPermissionMatrix is a helper that runs RunTestsOnDocker() across a range of possible docker settings.
Expand Down Expand Up @@ -182,24 +182,28 @@ func (tr *DockerTestRunner) RunTestsOnDocker(ctx context.Context) {

require.Equal(tr.Runner, int64(0), result.ReturnCode, "got bad docker return code. stdout: %s \nstderr: %s", result.Stdout, result.Stderr)

if tr.Verbose {
fmt.Fprintf(os.Stdout, "stderr: %s\n", result.Stderr)
fmt.Fprintf(os.Stdout, "stdout: %s\n", result.Stdout)
}

// iterate by lines to make this easier to read
if len(tr.FatalLogMessages) > 0 {
for _, badLine := range tr.FatalLogMessages {
for _, line := range strings.Split(result.Stdout, "\n") {
require.NotContains(tr.Runner, line, badLine)
}
for _, line := range strings.Split(result.Stderr, "\n") {
require.NotContains(tr.Runner, line, badLine)
// filter our the go mod package download messages
if !strings.Contains(line, "go: downloading") {
require.NotContains(tr.Runner, line, badLine)
}

}
}

}

if tr.Verbose {
fmt.Fprintf(os.Stdout, "stderr: %s\n", result.Stderr)
fmt.Fprintf(os.Stdout, "stdout: %s\n", result.Stdout)
}

}

// createTestContainer creates a container with the given test path and test name
Expand Down
13 changes: 13 additions & 0 deletions metric/system/cgroup/cgcommon/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,30 @@
// specific language governing permissions and limitations
// under the License.

//go:build linux
// +build linux

package cgcommon

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"

"github.com/elastic/elastic-agent-libs/opt"
"github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/testhelpers"
)

var testFileList = []string{
"../testdata/docker.zip",
}

func TestMain(m *testing.M) {
os.Exit(testhelpers.MainTestWrapper(m, testFileList))
}

func TestPressure(t *testing.T) {
v2Path := "../testdata/docker/sys/fs/cgroup/system.slice/docker-1c8fa019edd4b9d4b2856f4932c55929c5c118c808ed5faee9a135ca6e84b039.scope"

Expand Down
11 changes: 7 additions & 4 deletions metric/system/cgroup/cgv1/blkio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
// specific language governing permissions and limitations
// under the License.

//go:build linux
// +build linux

package cgv1

import (
Expand Down Expand Up @@ -58,12 +61,12 @@ func TestBlkioThrottle(t *testing.T) {
if err != nil {
t.Fatal(err)
}
assert.Equal(t, uint64(48), blkio.Total.Ios)
assert.Equal(t, uint64(1651200), blkio.Total.Bytes)
assert.Equal(t, uint64(0x2e), blkio.Total.Ios)
assert.Equal(t, uint64(0x192600), blkio.Total.Bytes)
assert.Equal(t, uint64(46), blkio.Reads.Ios)
assert.Equal(t, uint64(1648128), blkio.Reads.Bytes)
assert.Equal(t, uint64(2), blkio.Writes.Ios)
assert.Equal(t, uint64(3072), blkio.Writes.Bytes)
assert.Equal(t, uint64(0), blkio.Writes.Ios)
assert.Equal(t, uint64(0), blkio.Writes.Bytes)

}

Expand Down
14 changes: 14 additions & 0 deletions metric/system/cgroup/cgv1/cpu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,29 @@
// specific language governing permissions and limitations
// under the License.

//go:build linux
// +build linux

package cgv1

import (
"encoding/json"
"os"
"testing"

"github.com/stretchr/testify/assert"

"github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/testhelpers"
)

var testFileList = []string{
"../testdata/docker.zip",
}

func TestMain(m *testing.M) {
os.Exit(testhelpers.MainTestWrapper(m, testFileList))
}

const cpuPath = "../testdata/docker/sys/fs/cgroup/cpu/docker/b29faf21b7eff959f64b4192c34d5d67a707fe8561e9eaa608cb27693fba4242"

func TestCpuStats(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions metric/system/cgroup/cgv1/cpuacct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
// specific language governing permissions and limitations
// under the License.

//go:build linux
// +build linux

package cgv1

import (
Expand Down
3 changes: 3 additions & 0 deletions metric/system/cgroup/cgv1/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
// specific language governing permissions and limitations
// under the License.

//go:build linux
// +build linux

package cgv1

import (
Expand Down
14 changes: 14 additions & 0 deletions metric/system/cgroup/cgv2/v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,32 @@
// specific language governing permissions and limitations
// under the License.

//go:build linux
// +build linux

package cgv2

import (
"os"
"testing"

"github.com/stretchr/testify/assert"

"github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/testhelpers"
)

const v2Path = "../testdata/docker/sys/fs/cgroup/system.slice/docker-1c8fa019edd4b9d4b2856f4932c55929c5c118c808ed5faee9a135ca6e84b039.scope"
const ubuntu = "../testdata/io_statfiles/ubuntu"
const ubuntu2 = "../testdata/io_statfiles/ubuntu2"

var testFileList = []string{
"../testdata/docker.zip",
}

func TestMain(m *testing.M) {
os.Exit(testhelpers.MainTestWrapper(m, testFileList))
}

func TestGetIO(t *testing.T) {
ioTest := IOSubsystem{}
err := ioTest.Get(v2Path, false)
Expand Down
16 changes: 10 additions & 6 deletions metric/system/cgroup/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (r *Reader) CgroupsVersion(pid int) (CgroupsVersion, error) {
return CgroupsV2, nil
}
// Otherwise, check to see what's in the controllers file
controllers, err := readControllerList(cgstring, r.cgroupMountpoints.V2Loc)
controllers, err := r.readControllerList(cgstring)
if err != nil {
return CgroupsV1, fmt.Errorf("error fetching cgroup controller list for pid %d: %w", pid, err)
}
Expand Down Expand Up @@ -353,9 +353,9 @@ func getCommonCgroupMetadata(mounts map[string]ControllerPath, ignoreRoot bool)
}

// Read a cgroup.controllers list from a v2 cgroup
func readControllerList(cgroupsFile string, v2path string) ([]string, error) {
func (r *Reader) readControllerList(cgroupsFile string) ([]string, error) {
// edge case: There's no V2 controller
if v2path == "" {
if r.cgroupMountpoints.V2Loc == "" {
return []string{}, nil
}
controllers := strings.Split(cgroupsFile, "\n")
Expand All @@ -370,10 +370,14 @@ func readControllerList(cgroupsFile string, v2path string) ([]string, error) {
if cgpath == "" {
return []string{}, nil
}
file := filepath.Join(v2path, cgpath, "cgroup.controllers")
controllersRaw, err := os.ReadFile(file)
cgFilePath := filepath.Join(r.cgroupMountpoints.V2Loc, cgpath, "cgroup.controllers")
if cgroupNSStateFetch() && r.rootfsMountpoint.IsSet() {
cgFilePath = filepath.Join(r.cgroupMountpoints.V2Loc, r.cgroupMountpoints.ContainerizedRootMount, cgpath, "cgroup.controllers")
}

controllersRaw, err := os.ReadFile(cgFilePath)
if err != nil {
return nil, fmt.Errorf("error reading %s: %w", file, err)
return nil, fmt.Errorf("error reading cgroup '%s': file %s: %w", cgpath, cgFilePath, err)
}

if len(controllersRaw) == 0 {
Expand Down
10 changes: 0 additions & 10 deletions metric/system/cgroup/testdata/docker/proc/1/cgroup

This file was deleted.

1 change: 0 additions & 1 deletion metric/system/cgroup/testdata/docker/proc/312/cgroup

This file was deleted.

10 changes: 0 additions & 10 deletions metric/system/cgroup/testdata/docker/proc/985/cgroup

This file was deleted.

13 changes: 0 additions & 13 deletions metric/system/cgroup/testdata/docker/proc/cgroups

This file was deleted.

30 changes: 0 additions & 30 deletions metric/system/cgroup/testdata/docker/proc/self/mountinfo

This file was deleted.

Empty file.
Loading

0 comments on commit 182b3ee

Please sign in to comment.