Repository: replicatedhq/troubleshoot
Pull request #874 (2022-11-26 16:32:06)

Includes virtual memory parameters in Sysctl
## Description, Motivation and Context

TL;DR
-----

Updates Sysctl collector and analyzer for virtual memory parameters

Details
-------

Adds supoort for virtual memory parameters to the Sysctl collector and analyzers. I uncovered this writing a pre-flight for a Helm chart that includes ECK as a subchart. Since ECK requires a specific minimum value for `vm.max_map_count` I wanted to use the Sysctl analyzer to check for the expected value, but wasn't able to because of the limited values it collected. I also learned that Sonarqube expects the same parameter to be increased, so it seemed like a general enough requirement to add it in.

The code updates the collector to collect values under `/proc/sys/vm`and adds tests to the analyzer to based on the ECK requirements. Making the tests pass required adding operators to the when expression, since the existing code only allowed for `=`, `==`, and `===`. The when expression now supports `>`, `<`, `>=`, and `<=`.

## Checklist

- [X] New and existing tests pass locally with the changes.
- [X] Tests for the changes have been added (for bug fixes / features)
- [X] The commit message(s) are informative and highlight any breaking changes

## Does this PR introduce a breaking change?
- [ ] Yes
- [X] No


diamonwiggins: going to go ahead and merge. be sure to update the docs at https://troubleshoot.sh as well - https://github.com/replicatedhq/troubleshoot.sh 

Patch:
diff --git a/pkg/analyze/sysctl.go b/pkg/analyze/sysctl.go
index 1e13f18c..9ab717b9 100644
--- a/pkg/analyze/sysctl.go
+++ b/pkg/analyze/sysctl.go
@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -124,7 +125,7 @@ func evalSysctlOutcome(nodeParams map[string]map[string]string, outcome *trouble
 }
 
 // Example: net.ipv4.ip_forward = 0
-var sysctlWhenRX = regexp.MustCompile(`([^\s]+)\s+(=+)\s+(.+)`)
+var sysctlWhenRX = regexp.MustCompile(`([^\s]+)\s+([><=]=*)\s+(.+)`)
 
 // Returns the list of node names the condition is true for. The condition is not considered true
 // if the parameter is missing for the node.
@@ -151,6 +152,95 @@ func evalSysctlWhen(nodeParams map[string]map[string]string, when string) ([]str
 		sort.Strings(nodes)
 
 		return nodes, nil
+
+	case "<":
+		var nodes []string
+
+		for nodeName, params := range nodeParams {
+			nodeValue, err := strconv.ParseInt(params[matches[1]], 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+
+			expectedValue, err := strconv.ParseInt(strings.TrimSpace(matches[3]), 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+			if nodeValue < expectedValue {
+				nodes = append(nodes, nodeName)
+			}
+		}
+
+		sort.Strings(nodes)
+
+		return nodes, nil
+
+	case "<=":
+		var nodes []string
+
+		for nodeName, params := range nodeParams {
+			nodeValue, err := strconv.ParseInt(params[matches[1]], 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+
+			expectedValue, err := strconv.ParseInt(strings.TrimSpace(matches[3]), 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+			if nodeValue <= expectedValue {
+				nodes = append(nodes, nodeName)
+			}
+		}
+
+		sort.Strings(nodes)
+
+		return nodes, nil
+
+	case ">":
+		var nodes []string
+
+		for nodeName, params := range nodeParams {
+			nodeValue, err := strconv.ParseInt(params[matches[1]], 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+
+			expectedValue, err := strconv.ParseInt(strings.TrimSpace(matches[3]), 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+			if nodeValue > expectedValue {
+				nodes = append(nodes, nodeName)
+			}
+		}
+
+		sort.Strings(nodes)
+
+		return nodes, nil
+
+	case ">=":
+		var nodes []string
+
+		for nodeName, params := range nodeParams {
+			nodeValue, err := strconv.ParseInt(params[matches[1]], 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+
+			expectedValue, err := strconv.ParseInt(strings.TrimSpace(matches[3]), 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to parse when %q", when)
+			}
+			if nodeValue >= expectedValue {
+				nodes = append(nodes, nodeName)
+			}
+		}
+
+		sort.Strings(nodes)
+
+		return nodes, nil
+
 	default:
 		return nil, fmt.Errorf("Unknown operator %q", matches[2])
 	}
diff --git a/pkg/collect/sysctl.go b/pkg/collect/sysctl.go
index dfddfce0..ef36012e 100644
--- a/pkg/collect/sysctl.go
+++ b/pkg/collect/sysctl.go
@@ -67,6 +67,7 @@ func (c *CollectSysctl) Collect(progressChan chan<- interface{}) (CollectorResul
 	command := `
 find /proc/sys/net/ipv4 -type f | while read f; do v=$(cat $f 2>/dev/null); echo "$f = $v"; done
 find /proc/sys/net/bridge -type f | while read f; do v=$(cat $f 2>/dev/null); echo "$f = $v"; done
+find /proc/sys/vm -type f | while read f; do v=$(cat $f 2>/dev/null); echo "$f = $v"; done
 `
 	runPodOptions.Command = []string{"sh", "-c", command}
 


Test patch:
diff --git a/pkg/analyze/sysctl_test.go b/pkg/analyze/sysctl_test.go
index 4026f3c9..6e1599f4 100644
--- a/pkg/analyze/sysctl_test.go
+++ b/pkg/analyze/sysctl_test.go
@@ -12,12 +12,14 @@ func TestParseSysctlParameters(t *testing.T) {
 /proc/sys/net/ipv4/ip_forward = 1
 /proc/sys/net/ipv4/ip_local_port_range = 32768 60999
 /proc/sys/net/bridge/bridge-nf-call-iptables = 0
+/proc/sys/vm/max_map_count = 65530
 `
 	got := parseSysctlParameters([]byte(parameters))
 	expect := map[string]string{
 		"net.ipv4.ip_forward":                "1",
 		"net.ipv4.ip_local_port_range":       "32768 60999",
 		"net.bridge.bridge-nf-call-iptables": "0",
+		"vm.max_map_count":                   "65530",
 	}
 
 	assert.Equal(t, expect, got)
@@ -69,6 +71,36 @@ func TestEvalSysctlWhen(t *testing.T) {
 			expect:    []string{"node-b"},
 			expectErr: false,
 		},
+		{
+			name: "No nodes have max map count > 65530",
+			when: "vm.max_map_count > 65530",
+			nodeParams: map[string]map[string]string{
+				"node-a": {"vm.max_map_count": "65530"},
+				"node-b": {"vm.max_map_count": "65530"},
+			},
+			expect:    []string{},
+			expectErr: false,
+		},
+		{
+			name: "One node has max map count > 65530",
+			when: "vm.max_map_count > 65530",
+			nodeParams: map[string]map[string]string{
+				"node-a": {"vm.max_map_count": "65530"},
+				"node-b": {"vm.max_map_count": "262144"},
+			},
+			expect:    []string{"node-b"},
+			expectErr: false,
+		},
+		{
+			name: "All nodes have max map count > 65530",
+			when: "vm.max_map_count > 65530",
+			nodeParams: map[string]map[string]string{
+				"node-a": {"vm.max_map_count": "262144"},
+				"node-b": {"vm.max_map_count": "262144"},
+			},
+			expect:    []string{"node-a", "node-b"},
+			expectErr: false,
+		},
 	}
 
 	for _, test := range tests {
@@ -270,6 +302,100 @@ func TestAnalyzeSysctl(t *testing.T) {
 			},
 			expect: nil,
 		},
+		{
+			name: "Fail one node too low on max map count",
+			files: map[string][]byte{
+				"a": []byte(`
+/proc/sys/vm/max_map_count = 65530
+`),
+				"b": []byte(`
+/proc/sys/vm/max_map_count = 262144
+`),
+			},
+			analyzer: &troubleshootv1beta2.SysctlAnalyze{
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Fail: &troubleshootv1beta2.SingleOutcome{
+							When:    "vm.max_map_count < 262144 ",
+							Message: "Max map count too low",
+						},
+					},
+					{
+						Pass: &troubleshootv1beta2.SingleOutcome{
+							Message: "Max map count sufficient",
+						},
+					},
+				},
+			},
+			expect: &AnalyzeResult{
+				Title:   "Sysctl",
+				IsFail:  true,
+				Message: "Node a: Max map count too low",
+			},
+		},
+		{
+			name: "Fail two nodes too low on max map count",
+			files: map[string][]byte{
+				"a": []byte(`
+/proc/sys/vm/max_map_count = 65530
+`),
+				"b": []byte(`
+/proc/sys/vm/max_map_count = 65530
+`),
+			},
+			analyzer: &troubleshootv1beta2.SysctlAnalyze{
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Fail: &troubleshootv1beta2.SingleOutcome{
+							When:    "vm.max_map_count < 262144 ",
+							Message: "Max map count too low",
+						},
+					},
+					{
+						Pass: &troubleshootv1beta2.SingleOutcome{
+							Message: "Max map count sufficient",
+						},
+					},
+				},
+			},
+			expect: &AnalyzeResult{
+				Title:   "Sysctl",
+				IsFail:  true,
+				Message: "Nodes a, b: Max map count too low",
+			},
+		},
+		{
+			name: "Pass sufficient max map count on both nodes",
+			files: map[string][]byte{
+				"a": []byte(`
+/proc/sys/vm/max_map_count = 262144
+`),
+				"b": []byte(`
+/proc/sys/vm/max_map_count = 262144
+`),
+			},
+			analyzer: &troubleshootv1beta2.SysctlAnalyze{
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Fail: &troubleshootv1beta2.SingleOutcome{
+							When:    "vm.max_map_count < 262144 ",
+							Message: "Max map count too low",
+						},
+					},
+					{
+						Pass: &troubleshootv1beta2.SingleOutcome{
+							When:    "vm.max_map_count >= 262144 ",
+							Message: "Max map count sufficient",
+						},
+					},
+				},
+			},
+			expect: &AnalyzeResult{
+				Title:   "Sysctl",
+				IsPass:  true,
+				Message: "Nodes a, b: Max map count sufficient",
+			},
+		},
 	}
 
 	for _, test := range tests {