Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(host_sysctl): add host sysctl collector #1676

Merged
merged 9 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions config/crds/troubleshoot.sh_collectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17393,6 +17393,13 @@ spec:
- CIDRRangeAlloc
- desiredCIDR
type: object
sysctl:
properties:
collectorName:
type: string
exclude:
type: BoolString
type: object
systemPackages:
properties:
amzn:
Expand Down
7 changes: 7 additions & 0 deletions config/crds/troubleshoot.sh_hostcollectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1719,6 +1719,13 @@ spec:
- CIDRRangeAlloc
- desiredCIDR
type: object
sysctl:
properties:
collectorName:
type: string
exclude:
type: BoolString
type: object
systemPackages:
properties:
amzn:
Expand Down
7 changes: 7 additions & 0 deletions config/crds/troubleshoot.sh_hostpreflights.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1719,6 +1719,13 @@ spec:
- CIDRRangeAlloc
- desiredCIDR
type: object
sysctl:
properties:
collectorName:
type: string
exclude:
type: BoolString
type: object
systemPackages:
properties:
amzn:
Expand Down
7 changes: 7 additions & 0 deletions config/crds/troubleshoot.sh_supportbundles.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20366,6 +20366,13 @@ spec:
- CIDRRangeAlloc
- desiredCIDR
type: object
sysctl:
properties:
collectorName:
type: string
exclude:
type: BoolString
type: object
systemPackages:
properties:
amzn:
Expand Down
8 changes: 8 additions & 0 deletions examples/collect/host/sysctl.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: troubleshoot.sh/v1beta2
kind: HostCollector
metadata:
name: sysctl
spec:
collectors:
- sysctl:
collectorName: sysctl
10 changes: 10 additions & 0 deletions examples/preflight/host/sysctl.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
apiVersion: troubleshoot.sh/v1beta2
kind: HostPreflight
metadata:
name: sysctl
spec:
collectors:
- sysctl:
collectorName: sysctl
#TODO add analyzer once implemented
analyzers: []
5 changes: 5 additions & 0 deletions pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ type HostDNS struct {
Hostnames []string `json:"hostnames" yaml:"hostnames"`
}

type HostSysctl struct {
HostCollectorMeta `json:",inline" yaml:",inline"`
}

type HostCollect struct {
CPU *CPU `json:"cpu,omitempty" yaml:"cpu,omitempty"`
Memory *Memory `json:"memory,omitempty" yaml:"memory,omitempty"`
Expand Down Expand Up @@ -260,6 +264,7 @@ type HostCollect struct {
HostCGroups *HostCGroups `json:"cgroups,omitempty" yaml:"cgroups,omitempty"`
HostDNS *HostDNS `json:"dns,omitempty" yaml:"dns,omitempty"`
NetworkNamespaceConnectivity *HostNetworkNamespaceConnectivity `json:"networkNamespaceConnectivity,omitempty" yaml:"networkNamespaceConnectivity,omitempty"`
HostSysctl *HostSysctl `json:"sysctl,omitempty" yaml:"sysctl,omitempty"`
}

// GetName gets the name of the collector
Expand Down
21 changes: 21 additions & 0 deletions pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/collect/host_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ func GetHostCollector(collector *troubleshootv1beta2.HostCollect, bundlePath str
return &CollectHostDNS{collector.HostDNS, bundlePath}, true
case collector.NetworkNamespaceConnectivity != nil:
return &CollectHostNetworkNamespaceConnectivity{collector.NetworkNamespaceConnectivity, bundlePath}, true
case collector.HostSysctl != nil:
return &CollectHostSysctl{collector.HostSysctl, bundlePath}, true
default:
return nil, false
}
Expand Down
88 changes: 88 additions & 0 deletions pkg/collect/host_sysctl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package collect

import (
"bufio"
"bytes"
"encoding/json"
"os/exec"
"regexp"

"github.com/pkg/errors"
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
"k8s.io/klog/v2"
)

// Ensure `CollectHostSysctl` implements `HostCollector` interface at compile time.
var _ HostCollector = (*CollectHostSysctl)(nil)
JGAntunes marked this conversation as resolved.
Show resolved Hide resolved

// Helper var to allow stubbing `exec.Command` for tests
var execCommand = exec.Command

const HostSysctlPath = `host-collectors/system/sysctl.json`

type CollectHostSysctl struct {
hostCollector *troubleshootv1beta2.HostSysctl
BundlePath string
}

func (c *CollectHostSysctl) Title() string {
return hostCollectorTitleOrDefault(c.hostCollector.HostCollectorMeta, "Sysctl")
}

func (c *CollectHostSysctl) IsExcluded() (bool, error) {
return isExcluded(c.hostCollector.Exclude)
}

func (c *CollectHostSysctl) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
klog.V(2).Info("Running sysctl collector")
cmd := execCommand("sysctl", "-a")
JGAntunes marked this conversation as resolved.
Show resolved Hide resolved
out, err := cmd.Output()
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an error here

E1107 12:10:03.235230   56942 host_sysctl.go:39] Failed to initialize sysctl client: could not create client: directory /proc/sys/ does not exist

I think we should revert back to calling sysctl instead of relying on a library, otherwise we need to handle those rare environments, e.g darwin that don't have /proc/sys.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah true, darwin won't have procfs, however the props will be completely different too and even the output will differ between sysctl -a in darwin vs linux. But agree it's probably the best approach out of both options. Gonna go back to that and have a common output from the collector

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, let me know what you think @banjoh

klog.V(2).ErrorS(err, "failed to run sysctl")
if exitErr, ok := err.(*exec.ExitError); ok {
return nil, errors.Wrapf(err, "failed to run sysctl exit-code=%d stderr=%s", exitErr.ExitCode(), exitErr.Stderr)
} else {
return nil, errors.Wrap(err, "failed to run sysctl")
}
}
values := parseSysctlParameters(out)

payload, err := json.Marshal(values)
if err != nil {
klog.V(2).ErrorS(err, "failed to marshal data to json")
return nil, errors.Wrap(err, "failed to marshal data to json")
}

output := NewResult()
output.SaveResult(c.BundlePath, HostSysctlPath, bytes.NewBuffer(payload))
klog.V(2).Info("Finished writing JSON output")
return output, nil
}

// Linux sysctl outputs <key> = <value> where in Darwin you get <key> : <value>
// where <value> can be a string, number or multiple space separated strings
var sysctlLineRegex = regexp.MustCompile(`(\S+)\s*(=|:)\s*(.*)$`)

func parseSysctlParameters(output []byte) map[string]string {
scanner := bufio.NewScanner(bytes.NewReader(output))

result := map[string]string{}
for scanner.Scan() {
l := scanner.Text()
// <1:key> <2:separator> <3:value>
matches := sysctlLineRegex.FindStringSubmatch(l)

switch len(matches) {
// there are no matches for the value and separator, ignore and log
case 0, 1, 2:
klog.V(2).Infof("skipping sysctl line since we found no matches for it: %s", l)
// key exists but value could be empty, register as an empty string value but log something for reference
case 3:
klog.V(2).Infof("found no value for sysctl line, keeping it with an empty value: %s", l)
result[matches[1]] = ""
default:
result[matches[1]] = matches[3]
}
}
return result
}
Loading
Loading