Skip to content

Commit

Permalink
Fix more stuff
Browse files Browse the repository at this point in the history
  • Loading branch information
marctc committed Sep 27, 2023
1 parent 96f13a2 commit 26fc637
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 61 deletions.
5 changes: 2 additions & 3 deletions collector/diskstats_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ const (
var (
diskLabelNames = []string{"device"}

diskstatsDeviceExcludeSet bool

readsCompletedDesc = prometheus.NewDesc(
prometheus.BuildFQName(namespace, diskSubsystem, "reads_completed_total"),
"The total number of reads completed successfully.",
Expand Down Expand Up @@ -81,13 +79,14 @@ var (

type DiskstatsDeviceFilterConfig struct {
DiskstatsDeviceExclude *string
DiskstatsDeviceExcludeSet bool
OldDiskstatsDeviceExclude *string
DiskstatsDeviceInclude *string
}

func newDiskstatsDeviceFilter(config DiskstatsDeviceFilterConfig, logger log.Logger) (deviceFilter, error) {
if *config.OldDiskstatsDeviceExclude != "" {
if !diskstatsDeviceExcludeSet {
if !config.DiskstatsDeviceExcludeSet {
level.Warn(logger).Log("msg", "--collector.diskstats.ignored-devices is DEPRECATED and will be removed in 2.0.0, use --collector.diskstats.device-exclude")
*config.DiskstatsDeviceExclude = *config.OldDiskstatsDeviceExclude
} else {
Expand Down
24 changes: 19 additions & 5 deletions collector/diskstats_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,25 @@ func NewTestDiskStatsCollector(config NodeCollectorConfig, logger log.Logger) (p
}

func TestDiskStats(t *testing.T) {
*sysPath = "fixtures/sys"
*procPath = "fixtures/proc"
*udevDataPath = "fixtures/udev/data"
*diskstatsDeviceExclude = "^(ram|loop|fd|(h|s|v|xv)d[a-z]|nvme\\d+n\\d+p)\\d+$"
empty := ""
config := NodeCollectorConfig{
DiskstatsDeviceFilter: DiskstatsDeviceFilterConfig{
DiskstatsDeviceExclude: &empty,
DiskstatsDeviceInclude: &empty,
OldDiskstatsDeviceExclude: &empty,
},
}
sysPath := "fixtures/sys"
config.Path.SysPath = &sysPath

procPath := "fixtures/proc"
config.Path.ProcPath = &procPath

udevDataPath := "fixtures/udev/data"
config.Path.UdevDataPath = &udevDataPath

diskstatsDeviceExclude := "^(ram|loop|fd|(h|s|v|xv)d[a-z]|nvme\\d+n\\d+p)\\d+$"
config.DiskstatsDeviceFilter.DiskstatsDeviceExclude = &diskstatsDeviceExclude
testcase := `# HELP node_disk_ata_rotation_rate_rpm ATA disk rotation rate in RPMs (0 for SSDs).
# TYPE node_disk_ata_rotation_rate_rpm gauge
node_disk_ata_rotation_rate_rpm{device="sda"} 7200
Expand Down Expand Up @@ -314,7 +329,6 @@ node_disk_written_bytes_total{device="sr0"} 0
node_disk_written_bytes_total{device="vda"} 1.0938236928e+11
`

config := NodeCollectorConfig{}
logger := log.NewLogfmtLogger(os.Stderr)
collector, err := NewDiskstatsCollector(config, logger)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions collector/ethtool_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"syscall"
"testing"

"github.com/docker/cli/cli/config"
"github.com/go-kit/log"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
Expand Down Expand Up @@ -283,6 +282,7 @@ func TestBuildEthtoolFQName(t *testing.T) {
}

func TestEthToolCollector(t *testing.T) {
config := NodeCollectorConfig{}
testcase := `# HELP node_ethtool_align_errors Network interface align_errors
# TYPE node_ethtool_align_errors untyped
node_ethtool_align_errors{device="eth0"} 0
Expand Down Expand Up @@ -366,9 +366,9 @@ node_network_supported_speed_bytes{device="eth0",duplex="full",mode="10baseT"} 1
node_network_supported_speed_bytes{device="eth0",duplex="half",mode="100baseT"} 1.25e+07
node_network_supported_speed_bytes{device="eth0",duplex="half",mode="10baseT"} 1.25e+06
`
*config.Path.SysPath = "fixtures/sys"
sysPath := "fixtures/sys"
config.Path.SysPath = &sysPath

config := NodeCollectorConfig{}
logger := log.NewLogfmtLogger(os.Stderr)
collector, err := NewEthtoolTestCollector(config, logger)
if err != nil {
Expand Down
9 changes: 4 additions & 5 deletions collector/filesystem_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ import (
// * filesystemCollector.GetStats

var (
mountPointsExcludeSet bool
fsTypesExcludeSet bool

filesystemLabelNames = []string{"device", "mountpoint", "fstype"}
)

Expand All @@ -62,8 +59,10 @@ type filesystemStats struct {
}
type FilesystemConfig struct {
MountPointsExclude *string
MountPointsExcludeSet bool
OldMountPointsExcluded *string
FSTypesExclude *string
FSTypesExcludeSet bool
OldFSTypesExcluded *string
MountTimeout *time.Duration
StatWorkerCount *int
Expand All @@ -76,7 +75,7 @@ func init() {
// NewFilesystemCollector returns a new Collector exposing filesystems stats.
func NewFilesystemCollector(config NodeCollectorConfig, logger log.Logger) (Collector, error) {
if *config.Filesystem.OldMountPointsExcluded != "" {
if !mountPointsExcludeSet {
if !config.Filesystem.MountPointsExcludeSet {
level.Warn(logger).Log("msg", "--collector.filesystem.ignored-mount-points is DEPRECATED and will be removed in 2.0.0, use --collector.filesystem.mount-points-exclude")
*config.Filesystem.MountPointsExclude = *config.Filesystem.OldMountPointsExcluded
} else {
Expand All @@ -90,7 +89,7 @@ func NewFilesystemCollector(config NodeCollectorConfig, logger log.Logger) (Coll
}

if *config.Filesystem.OldFSTypesExcluded != "" {
if !fsTypesExcludeSet {
if !config.Filesystem.FSTypesExcludeSet {
level.Warn(logger).Log("msg", "--collector.filesystem.ignored-fs-types is DEPRECATED and will be removed in 2.0.0, use --collector.filesystem.fs-types-exclude")
*config.Filesystem.FSTypesExclude = *config.Filesystem.OldFSTypesExcluded
} else {
Expand Down
25 changes: 17 additions & 8 deletions collector/filesystem_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@
package collector

import (
"github.com/go-kit/log"
"strings"
"testing"

"github.com/go-kit/log"

"github.com/alecthomas/kingpin/v2"
)

func Test_parseFilesystemLabelsError(t *testing.T) {
config := NodeCollectorConfig{}
tests := []struct {
name string
in string
Expand All @@ -34,17 +36,20 @@ func Test_parseFilesystemLabelsError(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if _, err := parseFilesystemLabels(strings.NewReader(tt.in)); err == nil {
if _, err := parseFilesystemLabels(config, strings.NewReader(tt.in)); err == nil {
t.Fatal("expected an error, but none occurred")
}
})
}
}

func TestMountPointDetails(t *testing.T) {
if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./fixtures/proc"}); err != nil {
t.Fatal(err)
}
path := "./fixtures/proc"
config := NodeCollectorConfig{Path: PathConfig{ProcPath: &path, SysPath: &path, RootfsPath: &path, UdevDataPath: &path}}

// if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./fixtures/proc"}); err != nil {
// t.Fatal(err)
// }

expected := map[string]string{
"/": "",
Expand Down Expand Up @@ -79,7 +84,7 @@ func TestMountPointDetails(t *testing.T) {
"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[vsanDatastore] bafb9e5a-8856-7e6c-699c-801844e77a4a/kubernetes-dynamic-pvc-3eba5bba-48a3-11e8-89ab-005056b92113.vmdk": "",
}

filesystems, err := mountPointDetails(log.NewNopLogger())
filesystems, err := mountPointDetails(config, log.NewNopLogger())
if err != nil {
t.Log(err)
}
Expand All @@ -92,6 +97,8 @@ func TestMountPointDetails(t *testing.T) {
}

func TestMountsFallback(t *testing.T) {
config := NodeCollectorConfig{}

if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./fixtures_hidepid/proc"}); err != nil {
t.Fatal(err)
}
Expand All @@ -100,7 +107,7 @@ func TestMountsFallback(t *testing.T) {
"/": "",
}

filesystems, err := mountPointDetails(log.NewNopLogger())
filesystems, err := mountPointDetails(config, log.NewNopLogger())
if err != nil {
t.Log(err)
}
Expand All @@ -113,6 +120,8 @@ func TestMountsFallback(t *testing.T) {
}

func TestPathRootfs(t *testing.T) {
config := NodeCollectorConfig{}

if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./fixtures_bindmount/proc", "--path.rootfs", "/host"}); err != nil {
t.Fatal(err)
}
Expand All @@ -128,7 +137,7 @@ func TestPathRootfs(t *testing.T) {
"/sys/fs/cgroup": "",
}

filesystems, err := mountPointDetails(log.NewNopLogger())
filesystems, err := mountPointDetails(config, log.NewNopLogger())
if err != nil {
t.Log(err)
}
Expand Down
4 changes: 3 additions & 1 deletion collector/loadavg_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ package collector
import "testing"

func TestLoad(t *testing.T) {
config := NodeCollectorConfig{}

want := []float64{0.21, 0.37, 0.39}
loads, err := parseLoad("0.21 0.37 0.39 1/719 19737")
loads, err := parseLoad(config, "0.21 0.37 0.39 1/719 19737")
if err != nil {
t.Fatal(err)
}
Expand Down
27 changes: 17 additions & 10 deletions collector/paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,57 +21,64 @@ import (
)

func TestDefaultProcPath(t *testing.T) {
config := PathConfig{}

if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", procfs.DefaultMountPoint}); err != nil {
t.Fatal(err)
}

if got, want := c.config.Path.procFilePath("somefile"), "/proc/somefile"; got != want {
if got, want := config.procFilePath("somefile"), "/proc/somefile"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}

if got, want := c.config.Path.procFilePath("some/file"), "/proc/some/file"; got != want {
if got, want := config.procFilePath("some/file"), "/proc/some/file"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}
}

func TestCustomProcPath(t *testing.T) {
config := PathConfig{}

if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./../some/./place/"}); err != nil {
t.Fatal(err)
}

if got, want := c.config.Path.procFilePath("somefile"), "../some/place/somefile"; got != want {
if got, want := config.procFilePath("somefile"), "../some/place/somefile"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}

if got, want := c.config.Path.procFilePath("some/file"), "../some/place/some/file"; got != want {
if got, want := config.procFilePath("some/file"), "../some/place/some/file"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}
}

func TestDefault*config.Path.SysPath(t *testing.T) {
func TestDefaultSysPath(t *testing.T) {
config := PathConfig{}

if _, err := kingpin.CommandLine.Parse([]string{"--path.sysfs", "/sys"}); err != nil {
t.Fatal(err)
}

if got, want := *config.Path.SysPath("somefile"), "/sys/somefile"; got != want {
if got, want := config.sysFilePath("somefile"), "/sys/somefile"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}

if got, want := *config.Path.SysPath("some/file"), "/sys/some/file"; got != want {
if got, want := config.sysFilePath("some/file"), "/sys/some/file"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}
}

func TestCustom*config.Path.SysPath(t *testing.T) {
func TestCustomSysPath(t *testing.T) {
config := PathConfig{}
if _, err := kingpin.CommandLine.Parse([]string{"--path.sysfs", "./../some/./place/"}); err != nil {
t.Fatal(err)
}

if got, want := *config.Path.SysPath("somefile"), "../some/place/somefile"; got != want {
if got, want := config.sysFilePath("somefile"), "../some/place/somefile"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}

if got, want := *config.Path.SysPath("some/file"), "../some/place/some/file"; got != want {
if got, want := config.sysFilePath("some/file"), "../some/place/some/file"; got != want {
t.Errorf("Expected: %s, Got: %s", want, got)
}
}
6 changes: 4 additions & 2 deletions collector/perf_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ func TestPerfCollector(t *testing.T) {
}

func TestPerfCollectorStride(t *testing.T) {
config := NodeCollectorConfig{}

canTestPerf(t)

tests := []struct {
Expand Down Expand Up @@ -97,8 +99,8 @@ func TestPerfCollectorStride(t *testing.T) {
t.Skipf("Skipping test because runtime.NumCPU < %d", cpu)
}
}
perfCPUsFlag = &test.flag
collector, err := NewPerfCollector(NodeCollectorConfig{}, log.NewNopLogger())
config.Perf.CPUs = &test.flag
collector, err := NewPerfCollector(config, log.NewNopLogger())
if err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion collector/processes_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ import (
"testing"

"github.com/alecthomas/kingpin/v2"
"github.com/docker/cli/cli/config"
"github.com/go-kit/log"
"github.com/prometheus/procfs"
)

func TestReadProcessStatus(t *testing.T) {
config := NodeCollectorConfig{}
if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "fixtures/proc"}); err != nil {
t.Fatal(err)
}
Expand Down
9 changes: 4 additions & 5 deletions collector/systemd_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ const (
)

var (
systemdUnitIncludeSet bool
systemdUnitExcludeSet bool

systemdVersionRE = regexp.MustCompile(`[0-9]{3,}(\.[0-9]+)?`)
)

Expand Down Expand Up @@ -75,7 +72,9 @@ func init() {

type SystemdConfig struct {
UnitInclude *string
UnitIncludeSet bool
UnitExclude *string
UnitExcludeSet bool
OldUnitInclude *string
OldUnitExclude *string
Private *bool
Expand Down Expand Up @@ -132,15 +131,15 @@ func NewSystemdCollector(config NodeCollectorConfig, logger log.Logger) (Collect
"Detected systemd version", []string{"version"}, nil)

if *config.Systemd.OldUnitExclude != "" {
if !systemdUnitExcludeSet {
if !config.Systemd.UnitExcludeSet {
level.Warn(logger).Log("msg", "--collector.systemd.unit-blacklist is DEPRECATED and will be removed in 2.0.0, use --collector.systemd.unit-exclude")
*config.Systemd.UnitExclude = *config.Systemd.OldUnitExclude
} else {
return nil, errors.New("--collector.systemd.unit-blacklist and --collector.systemd.unit-exclude are mutually exclusive")
}
}
if *config.Systemd.OldUnitInclude != "" {
if !systemdUnitIncludeSet {
if !config.Systemd.UnitIncludeSet {
level.Warn(logger).Log("msg", "--collector.systemd.unit-whitelist is DEPRECATED and will be removed in 2.0.0, use --collector.systemd.unit-include")
*config.Systemd.UnitInclude = *config.Systemd.OldUnitInclude
} else {
Expand Down
Loading

0 comments on commit 26fc637

Please sign in to comment.