From b5ad2bb849f4ec32d93e83862226fbd10605b6ef Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Thu, 2 Feb 2023 16:39:28 +1000 Subject: [PATCH 01/27] boilerplate code for the subnet available collector and analyzer, still need to get the actual logic in next --- pkg/analyze/host_analyzer.go | 2 + pkg/analyze/host_subnetavailable.go | 25 +++++++++ .../v1beta2/hostanalyzer_shared.go | 8 +++ .../v1beta2/hostcollector_shared.go | 5 ++ .../v1beta2/remote_collector_shared.go | 5 ++ pkg/collect/host_collector.go | 2 + pkg/collect/host_subnetavailable.go | 55 +++++++++++++++++++ pkg/collect/host_subnetavailable_test.go | 40 ++++++++++++++ pkg/collect/remote_collector.go | 7 +++ 9 files changed, 149 insertions(+) create mode 100644 pkg/analyze/host_subnetavailable.go create mode 100644 pkg/collect/host_subnetavailable.go create mode 100644 pkg/collect/host_subnetavailable_test.go diff --git a/pkg/analyze/host_analyzer.go b/pkg/analyze/host_analyzer.go index 0f2f05c00..ce8d8d6c2 100644 --- a/pkg/analyze/host_analyzer.go +++ b/pkg/analyze/host_analyzer.go @@ -38,6 +38,8 @@ func GetHostAnalyzer(analyzer *troubleshootv1beta2.HostAnalyze) (HostAnalyzer, b return &AnalyzeHostTCPConnect{analyzer.TCPConnect}, true case analyzer.IPV4Interfaces != nil: return &AnalyzeHostIPV4Interfaces{analyzer.IPV4Interfaces}, true + case analyzer.SubnetAvailable != nil: + return &AnalyzeHostSubnetAvailable{analyzer.SubnetAvailable}, true case analyzer.FilesystemPerformance != nil: return &AnalyzeHostFilesystemPerformance{analyzer.FilesystemPerformance}, true case analyzer.Certificate != nil: diff --git a/pkg/analyze/host_subnetavailable.go b/pkg/analyze/host_subnetavailable.go new file mode 100644 index 000000000..7855a41fe --- /dev/null +++ b/pkg/analyze/host_subnetavailable.go @@ -0,0 +1,25 @@ +package analyzer + +import ( + troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" +) + +type AnalyzeHostSubnetAvailable struct { + hostAnalyzer *troubleshootv1beta2.SubnetAvailableAnalyze +} + +func (a *AnalyzeHostSubnetAvailable) Title() string { + return hostAnalyzerTitleOrDefault(a.hostAnalyzer.AnalyzeMeta, "Subnet Available") +} + +func (a *AnalyzeHostSubnetAvailable) IsExcluded() (bool, error) { + return isExcluded(a.hostAnalyzer.Exclude) +} + +func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(string) ([]byte, error)) ([]*AnalyzeResult, error) { + // TODO: implement + + var coll resultCollector + + return coll.get(a.Title()), nil +} diff --git a/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go b/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go index a97ce918b..3fa8c757b 100644 --- a/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go @@ -86,6 +86,12 @@ type IPV4InterfacesAnalyze struct { Outcomes []*Outcome `json:"outcomes" yaml:"outcomes"` } +type SubnetAvailableAnalyze struct { + AnalyzeMeta `json:",inline" yaml:",inline"` + CollectorName string `json:"collectorName,omitempty" yaml:"collectorName,omitempty"` + Outcomes []*Outcome `json:"outcomes" yaml:"outcomes"` +} + type FilesystemPerformanceAnalyze struct { AnalyzeMeta `json:",inline" yaml:",inline"` CollectorName string `json:"collectorName,omitempty" yaml:"collectorName,omitempty"` @@ -137,6 +143,8 @@ type HostAnalyze struct { IPV4Interfaces *IPV4InterfacesAnalyze `json:"ipv4Interfaces,omitempty" yaml:"ipv4Interfaces,omitempty"` + SubnetAvailable *SubnetAvailableAnalyze `json:"subnetAvailable,omitempty" yaml:"subnetAvailable,omitempty"` + FilesystemPerformance *FilesystemPerformanceAnalyze `json:"filesystemPerformance,omitempty" yaml:"filesystemPerformance,omitempty"` Certificate *CertificateAnalyze `json:"certificate,omitempty" yaml:"certificate,omitempty"` diff --git a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go index d3ee8cceb..3bd43e637 100644 --- a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go @@ -53,6 +53,10 @@ type IPV4Interfaces struct { HostCollectorMeta `json:",inline" yaml:",inline"` } +type SubnetAvailable struct { + HostCollectorMeta `json:",inline" yaml:",inline"` +} + type DiskUsage struct { HostCollectorMeta `json:",inline" yaml:",inline"` Path string `json:"path"` @@ -171,6 +175,7 @@ type HostCollect struct { UDPPortStatus *UDPPortStatus `json:"udpPortStatus,omitempty" yaml:"udpPortStatus,omitempty"` Kubernetes *Kubernetes `json:"kubernetes,omitempty" yaml:"kubernetes,omitempty"` IPV4Interfaces *IPV4Interfaces `json:"ipv4Interfaces,omitempty" yaml:"ipv4Interfaces,omitempty"` + SubnetAvailable *SubnetAvailable `json:"subnetAvailable,omitempty" yaml:"subnetAvailable,omitempty"` DiskUsage *DiskUsage `json:"diskUsage,omitempty" yaml:"diskUsage,omitempty"` HTTP *HostHTTP `json:"http,omitempty" yaml:"http,omitempty"` Time *HostTime `json:"time,omitempty" yaml:"time,omitempty"` diff --git a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go index 6fd8f4cb2..62d63e713 100644 --- a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go @@ -56,6 +56,10 @@ type RemoteIPV4Interfaces struct { RemoteCollectorMeta `json:",inline" yaml:",inline"` } +type RemoteSubnetAvailable struct { + RemoteCollectorMeta `json:",inline" yaml:",inline"` +} + type RemoteDiskUsage struct { RemoteCollectorMeta `json:",inline" yaml:",inline"` Path string `json:"path"` @@ -149,6 +153,7 @@ type RemoteCollect struct { TCPPortStatus *RemoteTCPPortStatus `json:"tcpPortStatus,omitempty" yaml:"tcpPortStatus,omitempty"` UDPPortStatus *RemoteUDPPortStatus `json:"udpPortStatus,omitempty" yaml:"udpPortStatus,omitempty"` IPV4Interfaces *RemoteIPV4Interfaces `json:"ipv4Interfaces,omitempty" yaml:"ipv4Interfaces,omitempty"` + SubnetAvailable *RemoteSubnetAvailable `json:"subnetAvailable,omitempty" yaml:"subnetAvailable,omitempty"` DiskUsage *RemoteDiskUsage `json:"diskUsage,omitempty" yaml:"diskUsage,omitempty"` HTTP *RemoteHTTP `json:"http,omitempty" yaml:"http,omitempty"` Time *RemoteTime `json:"time,omitempty" yaml:"time,omitempty"` diff --git a/pkg/collect/host_collector.go b/pkg/collect/host_collector.go index 9e538922b..ff9e4448f 100644 --- a/pkg/collect/host_collector.go +++ b/pkg/collect/host_collector.go @@ -45,6 +45,8 @@ func GetHostCollector(collector *troubleshootv1beta2.HostCollect, bundlePath str return &CollectHostTCPConnect{collector.TCPConnect, bundlePath}, true case collector.IPV4Interfaces != nil: return &CollectHostIPV4Interfaces{collector.IPV4Interfaces, bundlePath}, true + case collector.SubnetAvailable != nil: + return &CollectHostSubnetAvailable{collector.SubnetAvailable, bundlePath}, true case collector.FilesystemPerformance != nil: return &CollectHostFilesystemPerformance{collector.FilesystemPerformance, bundlePath}, true case collector.Certificate != nil: diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go new file mode 100644 index 000000000..37d676262 --- /dev/null +++ b/pkg/collect/host_subnetavailable.go @@ -0,0 +1,55 @@ +package collect + +import ( + "bytes" + "encoding/json" + "path/filepath" + + "github.com/pkg/errors" + troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/replicatedhq/troubleshoot/pkg/debug" + "github.com/vishvananda/netlink" +) + +type CollectHostSubnetAvailable struct { + hostCollector *troubleshootv1beta2.SubnetAvailable + BundlePath string +} + +func (c *CollectHostSubnetAvailable) Title() string { + return hostCollectorTitleOrDefault(c.hostCollector.HostCollectorMeta, "Subnet Available") +} + +func (c *CollectHostSubnetAvailable) IsExcluded() (bool, error) { + return isExcluded(c.hostCollector.Exclude) +} + +func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (map[string][]byte, error) { + + routes, err := netlink.RouteList(nil, netlink.FAMILY_V4) + if err != nil { + return nil, errors.Wrap(err, "failed to list routes") + } + + debug.Printf("Routes: %+v\n", routes) + + result := []byte{} + + b, err := json.Marshal(result) + if err != nil { + return nil, errors.Wrap(err, "failed to marshal result") + } + + collectorName := c.hostCollector.CollectorName + if collectorName == "" { + collectorName = "subnetAvailable" + } + name := filepath.Join("host-collectors/subnetAvailable", collectorName+".json") + + output := NewResult() + output.SaveResult(c.BundlePath, name, bytes.NewBuffer(b)) + + return map[string][]byte{ + name: b, + }, nil +} diff --git a/pkg/collect/host_subnetavailable_test.go b/pkg/collect/host_subnetavailable_test.go new file mode 100644 index 000000000..fef24a91e --- /dev/null +++ b/pkg/collect/host_subnetavailable_test.go @@ -0,0 +1,40 @@ +package collect + +import ( + "os" + "testing" + + troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/stretchr/testify/require" +) + +func TestCollectHostSubnetAvailable_Collect(t *testing.T) { + type fields struct { + hostCollector *troubleshootv1beta2.SubnetAvailable + } + tests := []struct { + name string + fields fields + want map[string][]byte + }{ + { + name: "TODO", + want: map[string][]byte{ + "host-collectors/subnetAvailable/subnetAvailable.json": []byte(`{"status":"connected","message":""}`), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "bundle") + require.NoError(t, err) + + c := &CollectHostSubnetAvailable{ + hostCollector: &troubleshootv1beta2.SubnetAvailable{ + // TODO: implement + }, + BundlePath: tmpDir, + } + }) + } +} diff --git a/pkg/collect/remote_collector.go b/pkg/collect/remote_collector.go index 6db6ab962..57570c15c 100644 --- a/pkg/collect/remote_collector.go +++ b/pkg/collect/remote_collector.go @@ -268,6 +268,13 @@ func (c *RemoteCollector) toHostCollector() (*troubleshootv1beta2.HostCollect, e Exclude: c.Collect.IPV4Interfaces.Exclude, }, } + case c.Collect.SubnetAvailable != nil: + hostCollect.SubnetAvailable = &troubleshootv1beta2.SubnetAvailable{ + HostCollectorMeta: troubleshootv1beta2.HostCollectorMeta{ + CollectorName: c.Collect.IPV4Interfaces.CollectorName, + Exclude: c.Collect.IPV4Interfaces.Exclude, + }, + } case c.Collect.FilesystemPerformance != nil: hostCollect.FilesystemPerformance = &troubleshootv1beta2.FilesystemPerformance{ HostCollectorMeta: troubleshootv1beta2.HostCollectorMeta{ From 077846bc0a328d5c237072de88be159e06f33097 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Thu, 2 Feb 2023 16:59:47 +1000 Subject: [PATCH 02/27] remove netlink dependency --- pkg/collect/host_subnetavailable.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 37d676262..f17301cd4 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -7,8 +7,6 @@ import ( "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" - "github.com/replicatedhq/troubleshoot/pkg/debug" - "github.com/vishvananda/netlink" ) type CollectHostSubnetAvailable struct { @@ -26,12 +24,9 @@ func (c *CollectHostSubnetAvailable) IsExcluded() (bool, error) { func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (map[string][]byte, error) { - routes, err := netlink.RouteList(nil, netlink.FAMILY_V4) - if err != nil { - return nil, errors.Wrap(err, "failed to list routes") - } + // TODO: run `ip route` and get the output (if available) - debug.Printf("Routes: %+v\n", routes) + //debug.Printf("Routes: %+v\n", routes) result := []byte{} From 8530b20eda2b0850701192d68ee072c0b61470f9 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Sat, 4 Feb 2023 12:07:31 +1000 Subject: [PATCH 03/27] working parsing of /proc/net/route to obtain ipv4 routes (without the netlink package or iproute2 as a host package dependency) --- pkg/collect/host_subnetavailable.go | 75 ++++++++++++++++++++++- pkg/collect/host_subnetavailable_test.go | 77 +++++++++++++++++++++++- 2 files changed, 148 insertions(+), 4 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index f17301cd4..3c6982983 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -2,8 +2,13 @@ package collect import ( "bytes" + "encoding/hex" "encoding/json" + "fmt" + "net" "path/filepath" + "strconv" + "strings" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" @@ -24,8 +29,6 @@ func (c *CollectHostSubnetAvailable) IsExcluded() (bool, error) { func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (map[string][]byte, error) { - // TODO: run `ip route` and get the output (if available) - //debug.Printf("Routes: %+v\n", routes) result := []byte{} @@ -48,3 +51,71 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m name: b, }, nil } + +type systemRoutes []systemRoute + +type systemRoute struct { + Iface string + DestNet net.IPNet + Gateway net.IP + Metric uint32 +} + +// Parses the output of /proc/net/route into something useful +// This only deals with IPv4 - another file /proc/net/ipv6_route deals with IPv6 (not implemented here) +func parseProcNetRoute(input string) (systemRoutes, error) { + routes := systemRoutes{} + for _, line := range strings.Split(input, "\n") { + if line[0:5] == "Iface" { + continue + } + + splitLine := strings.Split(strings.TrimSpace(line), "\t") + if len(splitLine) != 11 { + return []systemRoute{}, errors.Errorf("invalid /proc/net/route line '%s', expected 11 columns got %d", line, len(splitLine)) + } + + dest, err := hex.DecodeString(strings.TrimSpace(splitLine[1])) + if err != nil { + return []systemRoute{}, errors.Wrapf(err, "cannot parse dest column (index 1) for /proc/net/route line '%s'", line) + } + destStr := fmt.Sprintf("%d.%d.%d.%d", dest[3], dest[2], dest[1], dest[0]) + + gw, err := hex.DecodeString(strings.TrimSpace(splitLine[2])) + if err != nil { + return []systemRoute{}, errors.Wrapf(err, "cannot parse gateway column (index 2) for /proc/net/route line '%s'", line) + } + gwStr := fmt.Sprintf("%d.%d.%d.%d", gw[3], gw[2], gw[1], gw[0]) + + mask, err := hex.DecodeString(strings.TrimSpace(splitLine[7])) + if err != nil { + return []systemRoute{}, errors.Wrapf(err, "cannot parse mask column (index 7) for /proc/net/route line '%s'", line) + } + maskStr := fmt.Sprintf("%d.%d.%d.%d", mask[3], mask[2], mask[1], mask[0]) + maskBytes := []byte{} + for _, v := range strings.Split(maskStr, ".") { + maskByte, err := strconv.Atoi(v) + if err != nil { + return []systemRoute{}, errors.Wrapf(err, "cannot convert mask octet '%s' to byte", v) + } + maskBytes = append(maskBytes, byte(maskByte)) + } + + metric, err := strconv.Atoi(strings.TrimSpace(splitLine[6])) + if err != nil { + return []systemRoute{}, errors.Wrapf(err, "cannot parse metric column (index 6) for /proc/net/route line '%s'", line) + } + + routes = append(routes, systemRoute{ + Iface: strings.TrimSpace(splitLine[0]), + DestNet: net.IPNet{ + IP: net.ParseIP(destStr), + Mask: net.IPv4Mask(maskBytes[0], maskBytes[1], maskBytes[2], maskBytes[3]), + }, + Gateway: net.ParseIP(gwStr), + Metric: uint32(metric), + }) + } + + return routes, nil +} diff --git a/pkg/collect/host_subnetavailable_test.go b/pkg/collect/host_subnetavailable_test.go index fef24a91e..5c4853bf7 100644 --- a/pkg/collect/host_subnetavailable_test.go +++ b/pkg/collect/host_subnetavailable_test.go @@ -1,13 +1,14 @@ package collect import ( - "os" + "net" "testing" - troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +/* func TestCollectHostSubnetAvailable_Collect(t *testing.T) { type fields struct { hostCollector *troubleshootv1beta2.SubnetAvailable @@ -38,3 +39,75 @@ func TestCollectHostSubnetAvailable_Collect(t *testing.T) { }) } } +*/ + +func TestParseProcNetRoute(t *testing.T) { + input := `Iface Destination Gateway Flags RefCnt Use Metric Mask MTU Window IRTT +eth1 00000000 016BA8C0 0003 0 0 200 00000000 0 0 0 +eth0 00000000 0205A8C0 0003 0 0 202 00000000 0 0 0 +docker0 000011AC 00000000 0001 0 0 0 0000FFFF 0 0 0 +eth0 0005A8C0 00000000 0001 0 0 0 00FFFFFF 0 0 0 +eth1 006BA8C0 00000000 0001 0 0 0 00FFFFFF 0 0 0` + + /* + Destination Gateway Genmask Flags Metric Ref Use Iface + 0.0.0.0 192.168.107.1 0.0.0.0 UG 200 0 0 eth1 + 0.0.0.0 192.168.5.2 0.0.0.0 UG 202 0 0 eth0 + 172.17.0.0 0.0.0.0 255.255.0.0 U 0 0 0 docker0 + 192.168.5.0 0.0.0.0 255.255.255.0 U 0 0 0 eth0 + 192.168.107.0 0.0.0.0 255.255.255.0 U 0 0 0 eth1 + */ + + expected := systemRoutes{ + systemRoute{ + Iface: "eth1", + DestNet: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Gateway: net.IPv4(192, 168, 107, 1), + Metric: uint32(200), + }, + systemRoute{ + Iface: "eth0", + DestNet: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Gateway: net.IPv4(192, 168, 5, 2), + Metric: uint32(202), + }, + systemRoute{ + Iface: "docker0", + DestNet: net.IPNet{ + IP: net.IPv4(172, 17, 0, 0), + Mask: net.CIDRMask(16, 32), + }, + Gateway: net.IPv4(0, 0, 0, 0), + Metric: uint32(0), + }, + systemRoute{ + Iface: "eth0", + DestNet: net.IPNet{ + IP: net.IPv4(192, 168, 5, 0), + Mask: net.CIDRMask(24, 32), + }, + Gateway: net.IPv4(0, 0, 0, 0), + Metric: uint32(0), + }, + systemRoute{ + Iface: "eth1", + DestNet: net.IPNet{ + IP: net.IPv4(192, 168, 107, 0), + Mask: net.CIDRMask(24, 32), + }, + Gateway: net.IPv4(0, 0, 0, 0), + Metric: uint32(0), + }, + } + + result, err := parseProcNetRoute(input) + + require.NoError(t, err) + assert.Equal(t, expected, result) +} From 2f1d9802d7369328246a16fafb1f11fa507dc856 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Mon, 6 Feb 2023 08:23:42 +1000 Subject: [PATCH 04/27] WIP on subnet available collector/analyzer including an example YAML snippet --- .../troubleshoot_v1beta2_subnetavailable.yaml | 19 +++++++++++++++++++ .../v1beta2/hostcollector_shared.go | 2 ++ .../v1beta2/remote_collector_shared.go | 2 ++ pkg/collect/host_subnetavailable.go | 12 +++++++++++- 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 config/samples/troubleshoot_v1beta2_subnetavailable.yaml diff --git a/config/samples/troubleshoot_v1beta2_subnetavailable.yaml b/config/samples/troubleshoot_v1beta2_subnetavailable.yaml new file mode 100644 index 000000000..b1c532573 --- /dev/null +++ b/config/samples/troubleshoot_v1beta2_subnetavailable.yaml @@ -0,0 +1,19 @@ +apiVersion: troubleshoot.sh/v1beta2 +kind: HostPreflight +metadata: + name: subnet-available +spec: + collectors: + # would output yes/no depending if there is a /22 available in 10.0.0.0/8 + - subnetAvailable: + CIDRRangeAlloc: "10.0.0.0/8" + desiredCIDR: "/22" + analyzers: + - subnetAvailable: + outcomes: + - fail: + when: "no-subnet-available" + message: failed to find available subnet + - pass: + when: "subnet-available" + message: available /22 subnet found diff --git a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go index 3bd43e637..4cebc393b 100644 --- a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go @@ -55,6 +55,8 @@ type IPV4Interfaces struct { type SubnetAvailable struct { HostCollectorMeta `json:",inline" yaml:",inline"` + CIDRRangeAlloc `json:",inline" yaml:",inline"` + desiredCIDR `json:",inline" yaml:",inline"` } type DiskUsage struct { diff --git a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go index 62d63e713..e0a1bc1e4 100644 --- a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go @@ -58,6 +58,8 @@ type RemoteIPV4Interfaces struct { type RemoteSubnetAvailable struct { RemoteCollectorMeta `json:",inline" yaml:",inline"` + CIDRRangeAlloc `json:",inline" yaml:",inline"` + desiredCIDR `json:",inline" yaml:",inline"` } type RemoteDiskUsage struct { diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 3c6982983..c1f8a1cde 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -6,12 +6,14 @@ import ( "encoding/json" "fmt" "net" + "os" "path/filepath" "strconv" "strings" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/replicatedhq/troubleshoot/pkg/debug" ) type CollectHostSubnetAvailable struct { @@ -28,8 +30,16 @@ func (c *CollectHostSubnetAvailable) IsExcluded() (bool, error) { } func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (map[string][]byte, error) { + procNetRoute, err := os.ReadFile("/proc/net/route") + if err != nil { + return nil, errors.Wrap(err, "failed to read contents of /proc/net/route") + } - //debug.Printf("Routes: %+v\n", routes) + routes, err := parseProcNetRoute(string(procNetRoute)) + if err != nil { + return nil, errors.Wrap(err, "failed to parse /proc/net/route") + } + debug.Printf("Routes: %+v\n", routes) result := []byte{} From 04106d26d0efeae73d7aa07cb98b90d05127e8ec Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Mon, 6 Feb 2023 08:30:11 +1000 Subject: [PATCH 05/27] fix api defs for subnet available collector --- pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go | 4 ++-- pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go index 4cebc393b..47dc5b3d2 100644 --- a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go @@ -55,8 +55,8 @@ type IPV4Interfaces struct { type SubnetAvailable struct { HostCollectorMeta `json:",inline" yaml:",inline"` - CIDRRangeAlloc `json:",inline" yaml:",inline"` - desiredCIDR `json:",inline" yaml:",inline"` + CIDRRangeAlloc string `json:"CIDRRangeAlloc" yaml:"CIDRRangeAlloc"` + DesiredCIDR string `json:"desiredCIDR" yaml:"desiredCIDR"` } type DiskUsage struct { diff --git a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go index e0a1bc1e4..30e8c56f6 100644 --- a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go @@ -58,8 +58,8 @@ type RemoteIPV4Interfaces struct { type RemoteSubnetAvailable struct { RemoteCollectorMeta `json:",inline" yaml:",inline"` - CIDRRangeAlloc `json:",inline" yaml:",inline"` - desiredCIDR `json:",inline" yaml:",inline"` + CIDRRangeAlloc string `json:"CIDRRangeAlloc" yaml:"CIDRRangeAlloc"` + DesiredCIDR string `json:"desiredCIDR" yaml:"desiredCIDR"` } type RemoteDiskUsage struct { From 82f07ffa8e0b0704844550372fdbdf9da2c27e3e Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 15 Feb 2023 04:31:54 +1000 Subject: [PATCH 06/27] WIP on subnet available collector logic, tests don't pass yet --- go.mod | 1 + go.sum | 2 + pkg/collect/host_subnetavailable.go | 75 +++++++++++++- pkg/collect/host_subnetavailable_test.go | 123 +++++++++++++++++++++++ 4 files changed, 200 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index ebba8e824..55537332e 100644 --- a/go.mod +++ b/go.mod @@ -49,6 +49,7 @@ require ( require ( cloud.google.com/go/compute/metadata v0.2.1 // indirect + github.com/apparentlymart/go-cidr v1.1.0 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect github.com/docker/distribution v2.8.1+incompatible // indirect github.com/emicklei/go-restful/v3 v3.9.0 // indirect diff --git a/go.sum b/go.sum index 4f20df97a..da53ddcf3 100644 --- a/go.sum +++ b/go.sum @@ -108,6 +108,8 @@ github.com/alexflint/go-filemutex v0.0.0-20171022225611-72bdc8eae2ae/go.mod h1:C github.com/andybalholm/brotli v1.0.1 h1:KqhlKozYbRtJvsPrrEeXcO+N2l6NYT5A2QAFmSULpEc= github.com/andybalholm/brotli v1.0.1/go.mod h1:loMXtMfwqflxFJPmdbJO0a3KNoPuLBgiu3qAvBg8x/Y= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= +github.com/apparentlymart/go-cidr v1.1.0 h1:2mAhrMoF+nhXqxTzSZMUzDHkLjmIHC+Zzn4tdgBZjnU= +github.com/apparentlymart/go-cidr v1.1.0/go.mod h1:EBcsNrHc3zQeuaeCeCtQruQm+n9/YjEn/vI25Lg7Gwc= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index c1f8a1cde..447addf4c 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -11,11 +11,19 @@ import ( "strconv" "strings" + "github.com/apparentlymart/go-cidr/cidr" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" "github.com/replicatedhq/troubleshoot/pkg/debug" ) +type SubnetAvailableResult struct { + CIDRRangeAlloc string `json:"CIDRRangeAlloc"` + DesiredCIDR string `json:"desiredCIDR"` + // If true, at least 1 of the DesiredCIDR size is available within CIDRRangeAlloc + ADesiredIsAvailable bool `json:"aDesiredIsAvailable"` +} + type CollectHostSubnetAvailable struct { hostCollector *troubleshootv1beta2.SubnetAvailable BundlePath string @@ -41,7 +49,14 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m } debug.Printf("Routes: %+v\n", routes) - result := []byte{} + // c.hostCollector.CIDRRangeAlloc + // c.hostCollector.DesiredCIDR + // TODO: parse into types we can use + + result := SubnetAvailableResult{} + result.CIDRRangeAlloc = c.hostCollector.CIDRRangeAlloc + result.DesiredCIDR = c.hostCollector.DesiredCIDR + // TODO: populate result.DesiredIsAvailable true/false b, err := json.Marshal(result) if err != nil { @@ -129,3 +144,61 @@ func parseProcNetRoute(input string) (systemRoutes, error) { return routes, nil } + +// Credit: https://github.com/replicatedhq/kURL/blob/main/kurl_util/cmd/subnet/main.go findAvailableSubnet +// TODOLATER: consolidate some of this logic into a unified library? will need a bit of refactoring if so +// +// isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange +func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *systemRoutes, debug bool) (bool, error) { + forceV4 := len(subnetRange.IP) == net.IPv4len + + startIP, _ := cidr.AddressRange(subnetRange) + + _, subnet, err := net.ParseCIDR(fmt.Sprintf("%s/%d", startIP, cidrRange)) + if err != nil { + return false, errors.Wrap(err, "parse cidr") + } + if debug { + fmt.Fprintf(os.Stderr, "First subnet %s\n", subnet) + } + + for { + firstIP, lastIP := cidr.AddressRange(subnet) + if !subnetRange.Contains(firstIP) || !subnetRange.Contains(lastIP) { + return false, errors.New("no available subnet found") + } + + route := findFirstOverlappingRoute(subnet, routes) + if route == nil { + return true, nil + } + if forceV4 { + // NOTE: this may break with v6 addresses + if ip4 := route.DestNet.IP.To4(); ip4 != nil { + route.DestNet.IP = ip4 + } + } + if debug { + fmt.Fprintf(os.Stderr, "Route %v overlaps with subnet %s\n", *route, subnet) + } + + subnet, _ = cidr.NextSubnet(&route.DestNet, cidrRange) + if debug { + fmt.Fprintf(os.Stderr, "Next subnet %s\n", subnet) + } + } +} + +// findFirstOverlappingRoute will return the first overlapping route with the subnet specified +func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes) *systemRoute { + for _, route := range *routes { + if &route.DestNet != nil && overlaps(&route.DestNet, subnet) { + return &route + } + } + return nil +} + +func overlaps(n1, n2 *net.IPNet) bool { + return n1.Contains(n2.IP) || n2.Contains(n1.IP) +} diff --git a/pkg/collect/host_subnetavailable_test.go b/pkg/collect/host_subnetavailable_test.go index 5c4853bf7..61639d37d 100644 --- a/pkg/collect/host_subnetavailable_test.go +++ b/pkg/collect/host_subnetavailable_test.go @@ -111,3 +111,126 @@ eth1 006BA8C0 00000000 0001 0 0 0 00FFFFFF 0 0 0` require.NoError(t, err) assert.Equal(t, expected, result) } + +func TestIsASubnetAvailableInCIDR(t *testing.T) { + // Single routing table used for all tests + sysRoutes := systemRoutes{ + systemRoute{ + Iface: "eth1", + DestNet: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Gateway: net.IPv4(192, 168, 107, 1), + Metric: uint32(200), + }, + systemRoute{ + Iface: "eth0", + DestNet: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Gateway: net.IPv4(192, 168, 5, 2), + Metric: uint32(202), + }, + systemRoute{ + Iface: "docker0", + DestNet: net.IPNet{ + IP: net.IPv4(172, 17, 0, 0), + Mask: net.CIDRMask(16, 32), + }, + Gateway: net.IPv4(0, 0, 0, 0), + Metric: uint32(0), + }, + systemRoute{ + Iface: "docker1", + DestNet: net.IPNet{ + IP: net.IPv4(172, 16, 0, 0), + Mask: net.CIDRMask(16, 32), + }, + Gateway: net.IPv4(0, 0, 0, 0), + Metric: uint32(0), + }, + systemRoute{ + Iface: "eth0", + DestNet: net.IPNet{ + IP: net.IPv4(192, 168, 5, 0), + Mask: net.CIDRMask(24, 32), + }, + Gateway: net.IPv4(0, 0, 0, 0), + Metric: uint32(0), + }, + systemRoute{ + Iface: "eth1", + DestNet: net.IPNet{ + IP: net.IPv4(192, 168, 107, 0), + Mask: net.CIDRMask(24, 32), + }, + Gateway: net.IPv4(0, 0, 0, 0), + Metric: uint32(0), + }, + } + + tests := []struct { + name string + cidrRange int + subnetRange net.IPNet + expected bool + }{ + { + name: "unavailable 1", + cidrRange: 24, + subnetRange: net.IPNet{ + IP: net.IPv4(172, 17, 0, 0), + Mask: net.CIDRMask(20, 32), + }, + expected: false, + }, + { + name: "unavailable 2", + cidrRange: 27, + subnetRange: net.IPNet{ + IP: net.IPv4(192, 168, 5, 0), + Mask: net.CIDRMask(24, 32), + }, + expected: false, + }, + { + name: "available 1", + cidrRange: 23, + subnetRange: net.IPNet{ + IP: net.IPv4(172, 20, 0, 0), + Mask: net.CIDRMask(16, 32), + }, + expected: true, + }, + { + name: "available 2", + cidrRange: 24, + subnetRange: net.IPNet{ + IP: net.IPv4(10, 0, 0, 0), + Mask: net.CIDRMask(8, 32), + }, + expected: true, + }, + { + name: "available 3", + cidrRange: 24, + subnetRange: net.IPNet{ + IP: net.IPv4(172, 16, 0, 0), + Mask: net.CIDRMask(12, 32), + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := require.New(t) + actual, err := isASubnetAvailableInCIDR(tt.cidrRange, &tt.subnetRange, &sysRoutes, true) + req.NoError(err) + + assert.Equal(t, tt.expected, actual) + }) + } +} From a1d4b198d719d78d44518447751d771f78f907d2 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 15 Feb 2023 05:21:26 +1000 Subject: [PATCH 07/27] more WIP on subnet available collector, tests don't pass yet still --- pkg/collect/host_subnetavailable.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 447addf4c..28408838c 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -148,7 +148,7 @@ func parseProcNetRoute(input string) (systemRoutes, error) { // Credit: https://github.com/replicatedhq/kURL/blob/main/kurl_util/cmd/subnet/main.go findAvailableSubnet // TODOLATER: consolidate some of this logic into a unified library? will need a bit of refactoring if so // -// isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange +// isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange (IPv4 only) func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *systemRoutes, debug bool) (bool, error) { forceV4 := len(subnetRange.IP) == net.IPv4len @@ -159,7 +159,8 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys return false, errors.Wrap(err, "parse cidr") } if debug { - fmt.Fprintf(os.Stderr, "First subnet %s\n", subnet) + fmt.Fprintf(os.Stderr, "Looking for a /%d within %s\n", cidrRange, subnetRange) + fmt.Fprintf(os.Stderr, "First subnet to test %s\n", subnet) } for { @@ -170,6 +171,9 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys route := findFirstOverlappingRoute(subnet, routes) if route == nil { + if debug { + fmt.Fprintf(os.Stderr, "No overlapping routes found\n") + } return true, nil } if forceV4 { @@ -179,7 +183,7 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys } } if debug { - fmt.Fprintf(os.Stderr, "Route %v overlaps with subnet %s\n", *route, subnet) + fmt.Fprintf(os.Stderr, "Route %s overlaps with subnet %s\n", &route.DestNet, subnet) } subnet, _ = cidr.NextSubnet(&route.DestNet, cidrRange) @@ -191,14 +195,24 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys // findFirstOverlappingRoute will return the first overlapping route with the subnet specified func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes) *systemRoute { + defaultRoute := net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + } + for _, route := range *routes { - if &route.DestNet != nil && overlaps(&route.DestNet, subnet) { + // Exclude default routes (0.0.0.0/0) + if route.DestNet.IP.Equal(defaultRoute.IP) && route.DestNet.Mask.String() == defaultRoute.Mask.String() { + continue + } + + if netOverlaps(&route.DestNet, subnet) { return &route } } return nil } -func overlaps(n1, n2 *net.IPNet) bool { +func netOverlaps(n1, n2 *net.IPNet) bool { return n1.Contains(n2.IP) || n2.Contains(n1.IP) } From be1591c8c6805116c1503ad4d746ded4eb35e01f Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 15 Feb 2023 06:17:34 +1000 Subject: [PATCH 08/27] WIP on subnet available analyzer, not quite there yet (will likely wait until the collector is done to test it) --- pkg/analyze/host_subnetavailable.go | 46 +++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/pkg/analyze/host_subnetavailable.go b/pkg/analyze/host_subnetavailable.go index 7855a41fe..90c318629 100644 --- a/pkg/analyze/host_subnetavailable.go +++ b/pkg/analyze/host_subnetavailable.go @@ -17,9 +17,49 @@ func (a *AnalyzeHostSubnetAvailable) IsExcluded() (bool, error) { } func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(string) ([]byte, error)) ([]*AnalyzeResult, error) { - // TODO: implement + hostAnalyzer := a.hostAnalyzer - var coll resultCollector + result := &AnalyzeResult{ + Title: a.Title(), + } - return coll.get(a.Title()), nil + for _, outcome := range hostAnalyzer.Outcomes { + if outcome.Fail != nil { + if outcome.Fail.When == "" { + result.IsFail = true + result.Message = outcome.Fail.Message + result.URI = outcome.Fail.URI + + return []*AnalyzeResult{result}, nil + } + + // TODO: implement + if string("TODO") == outcome.Fail.When { + result.IsFail = true + result.Message = outcome.Fail.Message + result.URI = outcome.Fail.URI + + return []*AnalyzeResult{result}, nil + } + } else if outcome.Pass != nil { + if outcome.Pass.When == "" { + result.IsPass = true + result.Message = outcome.Pass.Message + result.URI = outcome.Pass.URI + + return []*AnalyzeResult{result}, nil + } + + // TODO: implement + if string("TODO") == outcome.Pass.When { + result.IsPass = true + result.Message = outcome.Pass.Message + result.URI = outcome.Pass.URI + + return []*AnalyzeResult{result}, nil + } + } + } + + return []*AnalyzeResult{result}, nil } From aeaac587231a61d20efa2708d0fb8e649125f21e Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 15 Feb 2023 06:17:55 +1000 Subject: [PATCH 09/27] validate the cidr size for the subnet available collector --- pkg/collect/host_subnetavailable.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 28408838c..43faa7d03 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -150,6 +150,10 @@ func parseProcNetRoute(input string) (systemRoutes, error) { // // isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange (IPv4 only) func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *systemRoutes, debug bool) (bool, error) { + if cidrRange < 1 || cidrRange > 32 { + return false, errors.New("CIDR range size %d invalid, must be between 1 and 32") + } + forceV4 := len(subnetRange.IP) == net.IPv4len startIP, _ := cidr.AddressRange(subnetRange) From 5e238117dceb09ecc331873fec02a9b0e568d0d7 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 15 Feb 2023 20:35:26 +1000 Subject: [PATCH 10/27] WIP on subnet available collector, more logic added but it's not 100% there yet... --- .../v1beta2/hostcollector_shared.go | 2 +- .../v1beta2/remote_collector_shared.go | 2 +- pkg/collect/host_subnetavailable.go | 34 +++++++++++++++---- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go index 47dc5b3d2..3f309bdda 100644 --- a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go @@ -56,7 +56,7 @@ type IPV4Interfaces struct { type SubnetAvailable struct { HostCollectorMeta `json:",inline" yaml:",inline"` CIDRRangeAlloc string `json:"CIDRRangeAlloc" yaml:"CIDRRangeAlloc"` - DesiredCIDR string `json:"desiredCIDR" yaml:"desiredCIDR"` + DesiredCIDR int `json:"desiredCIDR" yaml:"desiredCIDR"` } type DiskUsage struct { diff --git a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go index 30e8c56f6..f2fe62905 100644 --- a/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/remote_collector_shared.go @@ -59,7 +59,7 @@ type RemoteIPV4Interfaces struct { type RemoteSubnetAvailable struct { RemoteCollectorMeta `json:",inline" yaml:",inline"` CIDRRangeAlloc string `json:"CIDRRangeAlloc" yaml:"CIDRRangeAlloc"` - DesiredCIDR string `json:"desiredCIDR" yaml:"desiredCIDR"` + DesiredCIDR int `json:"desiredCIDR" yaml:"desiredCIDR"` } type RemoteDiskUsage struct { diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 43faa7d03..f507d0efb 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -19,7 +19,7 @@ import ( type SubnetAvailableResult struct { CIDRRangeAlloc string `json:"CIDRRangeAlloc"` - DesiredCIDR string `json:"desiredCIDR"` + DesiredCIDR int `json:"desiredCIDR"` // If true, at least 1 of the DesiredCIDR size is available within CIDRRangeAlloc ADesiredIsAvailable bool `json:"aDesiredIsAvailable"` } @@ -49,14 +49,34 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m } debug.Printf("Routes: %+v\n", routes) - // c.hostCollector.CIDRRangeAlloc - // c.hostCollector.DesiredCIDR - // TODO: parse into types we can use + // IPv4 only right now... + if c.hostCollector.DesiredCIDR < 1 || c.hostCollector.DesiredCIDR > 32 { + return nil, errors.Wrap(err, fmt.Sprintf("CIDR range size %d invalid, must be between 1 and 32", c.hostCollector.DesiredCIDR)) + } + + splitCIDRRangeAlloc := strings.Split(c.hostCollector.CIDRRangeAlloc, "/") + if len(splitCIDRRangeAlloc) != 2 { + return nil, errors.Wrap(err, fmt.Sprintf("CIDRRangeAlloc value %s invalid, expected format x.x.x.x/##", c.hostCollector.CIDRRangeAlloc)) + } + maskInt, err := strconv.Atoi(splitCIDRRangeAlloc[1]) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("CIDRRangeAlloc mask %s invalid, expected integer", splitCIDRRangeAlloc[1])) + } + if maskInt < 1 || maskInt > 32 { + return nil, errors.Wrap(err, fmt.Sprintf("CIDRRangeAlloc mask %d invalid, must be between 1 and 32", maskInt)) + } + cidrRangeAllocIPNet := net.IPNet{ + IP: net.ParseIP(splitCIDRRangeAlloc[0]), + Mask: net.CIDRMask(maskInt, 32), + } result := SubnetAvailableResult{} result.CIDRRangeAlloc = c.hostCollector.CIDRRangeAlloc result.DesiredCIDR = c.hostCollector.DesiredCIDR - // TODO: populate result.DesiredIsAvailable true/false + result.ADesiredIsAvailable, err = isASubnetAvailableInCIDR(c.hostCollector.DesiredCIDR, &cidrRangeAllocIPNet, &routes, false) + if err != nil { + return nil, errors.Wrap(err, "failed to determine if desired CIDR is available within subnet") + } b, err := json.Marshal(result) if err != nil { @@ -151,7 +171,7 @@ func parseProcNetRoute(input string) (systemRoutes, error) { // isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange (IPv4 only) func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *systemRoutes, debug bool) (bool, error) { if cidrRange < 1 || cidrRange > 32 { - return false, errors.New("CIDR range size %d invalid, must be between 1 and 32") + return false, errors.New(fmt.Sprintf("CIDR range size %d invalid, must be between 1 and 32", cidrRange)) } forceV4 := len(subnetRange.IP) == net.IPv4len @@ -190,6 +210,8 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys fmt.Fprintf(os.Stderr, "Route %s overlaps with subnet %s\n", &route.DestNet, subnet) } + // TODO: somethings "off" with this part of the logic... debug further + fmt.Printf("route %s\n", route.DestNet.String()) subnet, _ = cidr.NextSubnet(&route.DestNet, cidrRange) if debug { fmt.Fprintf(os.Stderr, "Next subnet %s\n", subnet) From 4c8fcac5185c1ac3b8f599d994c1e773ba3cd0c6 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 15 Feb 2023 20:57:39 +1000 Subject: [PATCH 11/27] more WIP on subnet available collector, dealing with IP address parsing shenanigans --- pkg/collect/host_subnetavailable.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index f507d0efb..869533b83 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -174,7 +174,14 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys return false, errors.New(fmt.Sprintf("CIDR range size %d invalid, must be between 1 and 32", cidrRange)) } + // TODO: this isn't reliable, refactor. from the golang net does: + // Note that in this documentation, referring to an IP address as an IPv4 address or an IPv6 address is a semantic property of the address, not just the length of the byte slice: a 16-byte slice can still be an IPv4 address. forceV4 := len(subnetRange.IP) == net.IPv4len + if debug { + fmt.Fprintf(os.Stderr, "forceV4 %t ip len %d\n", forceV4, len(subnetRange.IP)) + } + // TODO: remove once above is refactored + forceV4 = true startIP, _ := cidr.AddressRange(subnetRange) @@ -193,7 +200,7 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys return false, errors.New("no available subnet found") } - route := findFirstOverlappingRoute(subnet, routes) + route := findFirstOverlappingRoute(subnet, routes, debug) if route == nil { if debug { fmt.Fprintf(os.Stderr, "No overlapping routes found\n") @@ -203,6 +210,9 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys if forceV4 { // NOTE: this may break with v6 addresses if ip4 := route.DestNet.IP.To4(); ip4 != nil { + if debug { + fmt.Fprintf(os.Stderr, "converting route.DestNet.IP to ensure IPv4 %+v\n", ip4) + } route.DestNet.IP = ip4 } } @@ -220,7 +230,7 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys } // findFirstOverlappingRoute will return the first overlapping route with the subnet specified -func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes) *systemRoute { +func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes, debug bool) *systemRoute { defaultRoute := net.IPNet{ IP: net.IPv4(0, 0, 0, 0), Mask: net.CIDRMask(0, 32), @@ -232,10 +242,19 @@ func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes) *systemR continue } + if debug { + fmt.Fprintf(os.Stderr, "Checking if route %s overlaps with subnet %s\n", &route.DestNet, subnet) + } if netOverlaps(&route.DestNet, subnet) { + if debug { + fmt.Fprintf(os.Stderr, "Overlaps\n") + } return &route } } + if debug { + fmt.Fprintf(os.Stderr, "No overlap\n") + } return nil } From 3b43f59ac34996f98b100b41d7f31160bae1605d Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Fri, 17 Feb 2023 13:27:07 +1000 Subject: [PATCH 12/27] fixed the isASubnetAvailableInCIDR function, pretty sure it was buggy (wrote it from scratch and commented it for easier understanding in future). tests pass --- pkg/collect/host_subnetavailable.go | 85 +++++++++++++----------- pkg/collect/host_subnetavailable_test.go | 2 +- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 869533b83..52075878d 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -168,69 +168,62 @@ func parseProcNetRoute(input string) (systemRoutes, error) { // Credit: https://github.com/replicatedhq/kURL/blob/main/kurl_util/cmd/subnet/main.go findAvailableSubnet // TODOLATER: consolidate some of this logic into a unified library? will need a bit of refactoring if so // -// isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange (IPv4 only) +// isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange (IPv4 only), checking against system routes for conflicts func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *systemRoutes, debug bool) (bool, error) { + // Sanity check that the CIDR range size is IPv4 valid (/0 to /32) if cidrRange < 1 || cidrRange > 32 { return false, errors.New(fmt.Sprintf("CIDR range size %d invalid, must be between 1 and 32", cidrRange)) } - // TODO: this isn't reliable, refactor. from the golang net does: - // Note that in this documentation, referring to an IP address as an IPv4 address or an IPv6 address is a semantic property of the address, not just the length of the byte slice: a 16-byte slice can still be an IPv4 address. - forceV4 := len(subnetRange.IP) == net.IPv4len - if debug { - fmt.Fprintf(os.Stderr, "forceV4 %t ip len %d\n", forceV4, len(subnetRange.IP)) + // Check that cidrRange is equal to or smaller than the subnet size of subnetRange + // Always exit false if not... + subnetRangeSize, subnetRangeSizeBits := subnetRange.Mask.Size() + if subnetRangeSizeBits != 32 { + return false, errors.New(fmt.Sprintf("subnetRange size is not IPv4 compatible? expected 32 got %d", subnetRangeSizeBits)) + } + // NOTE: reversed operator as we're talking about CIDR blocks (smaller integer = more IPs) + if subnetRangeSize > cidrRange { + return false, errors.New(fmt.Sprintf("subnetRange size (%d) must be larger than or equal to cidrRange size (%d), can't check if a range larger than itself is available", subnetRangeSize, cidrRange)) } - // TODO: remove once above is refactored - forceV4 = true + // Find the start IP of subnetRange, this will become the first subnet to be tested (with cidrRange as the size) startIP, _ := cidr.AddressRange(subnetRange) _, subnet, err := net.ParseCIDR(fmt.Sprintf("%s/%d", startIP, cidrRange)) if err != nil { return false, errors.Wrap(err, "parse cidr") } - if debug { - fmt.Fprintf(os.Stderr, "Looking for a /%d within %s\n", cidrRange, subnetRange) - fmt.Fprintf(os.Stderr, "First subnet to test %s\n", subnet) - } for { + // This is the extents of the subnet to be tested firstIP, lastIP := cidr.AddressRange(subnet) + if debug { + fmt.Fprintf(os.Stderr, "Checking subnet: %s firstIP: %s lastIP: %s\n", subnet.String(), firstIP, lastIP) + } + + // Make sure the smaller subnet is (still) within the large subnetRange. If subnetRange has been exhausted one of these will be false if !subnetRange.Contains(firstIP) || !subnetRange.Contains(lastIP) { - return false, errors.New("no available subnet found") + return false, nil } + // Check if any system routes overlap with the smaller subnet being tested route := findFirstOverlappingRoute(subnet, routes, debug) if route == nil { - if debug { - fmt.Fprintf(os.Stderr, "No overlapping routes found\n") - } + // No system routes match, this (smaller) subnet is available return true, nil } - if forceV4 { - // NOTE: this may break with v6 addresses - if ip4 := route.DestNet.IP.To4(); ip4 != nil { - if debug { - fmt.Fprintf(os.Stderr, "converting route.DestNet.IP to ensure IPv4 %+v\n", ip4) - } - route.DestNet.IP = ip4 - } - } - if debug { - fmt.Fprintf(os.Stderr, "Route %s overlaps with subnet %s\n", &route.DestNet, subnet) - } - // TODO: somethings "off" with this part of the logic... debug further - fmt.Printf("route %s\n", route.DestNet.String()) - subnet, _ = cidr.NextSubnet(&route.DestNet, cidrRange) - if debug { - fmt.Fprintf(os.Stderr, "Next subnet %s\n", subnet) - } + // Try the next subnet in the range + subnet, _ = cidr.NextSubnet(subnet, cidrRange) } + + // No subnets of cidrRange size were available within subnetRange (exhausted) + return false, nil } // findFirstOverlappingRoute will return the first overlapping route with the subnet specified func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes, debug bool) *systemRoute { + // NOTE: IPv4 specific defaultRoute := net.IPNet{ IP: net.IPv4(0, 0, 0, 0), Mask: net.CIDRMask(0, 32), @@ -243,21 +236,39 @@ func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes, debug bo } if debug { - fmt.Fprintf(os.Stderr, "Checking if route %s overlaps with subnet %s\n", &route.DestNet, subnet) + fmt.Fprintf(os.Stderr, "Checking if route %s overlaps with subnet %s - ", &route.DestNet, subnet) } + // TODOLATER: can we use cidr.VerifyNoOverlap to replace this? tests fail right now trying to do so... + //if cidr.VerifyNoOverlap([]*net.IPNet{subnet}, &route.DestNet) != nil { if netOverlaps(&route.DestNet, subnet) { if debug { fmt.Fprintf(os.Stderr, "Overlaps\n") } return &route + } else { + if debug { + fmt.Fprintf(os.Stderr, "No overlap\n") + } } } if debug { - fmt.Fprintf(os.Stderr, "No overlap\n") + fmt.Fprintf(os.Stderr, "Subnet %s has no overlap with any system routes\n", subnet) } return nil } func netOverlaps(n1, n2 *net.IPNet) bool { - return n1.Contains(n2.IP) || n2.Contains(n1.IP) + n1FirstIP, n1LastIP := cidr.AddressRange(n1) + n2FirstIP, n2LastIP := cidr.AddressRange(n2) + + // Check if the first net contains either the first or last IP of the second net + if n1.Contains(n2FirstIP) || n1.Contains(n2LastIP) { + return true + } + // Now do the reverse: check if the second net contains either the first or last IP of the first net + if n2.Contains(n1FirstIP) || n2.Contains(n1LastIP) { + return true + } + + return false } diff --git a/pkg/collect/host_subnetavailable_test.go b/pkg/collect/host_subnetavailable_test.go index 61639d37d..55ff3ab61 100644 --- a/pkg/collect/host_subnetavailable_test.go +++ b/pkg/collect/host_subnetavailable_test.go @@ -227,7 +227,7 @@ func TestIsASubnetAvailableInCIDR(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := require.New(t) - actual, err := isASubnetAvailableInCIDR(tt.cidrRange, &tt.subnetRange, &sysRoutes, true) + actual, err := isASubnetAvailableInCIDR(tt.cidrRange, &tt.subnetRange, &sysRoutes, false) // debug bool is useful for fixing bugs here, but off by default for noise req.NoError(err) assert.Equal(t, tt.expected, actual) From 49767841f8be1153e9ddef82de53ef3859d15687 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Fri, 17 Feb 2023 14:25:06 +1000 Subject: [PATCH 13/27] subnet available collector and analyzer all working as expected --- .../troubleshoot_v1beta2_subnetavailable.yaml | 4 +-- pkg/analyze/host_subnetavailable.go | 31 ++++++++++++++++--- pkg/collect/host_subnetavailable.go | 24 +++++++++++--- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/config/samples/troubleshoot_v1beta2_subnetavailable.yaml b/config/samples/troubleshoot_v1beta2_subnetavailable.yaml index b1c532573..2e5673b42 100644 --- a/config/samples/troubleshoot_v1beta2_subnetavailable.yaml +++ b/config/samples/troubleshoot_v1beta2_subnetavailable.yaml @@ -7,7 +7,7 @@ spec: # would output yes/no depending if there is a /22 available in 10.0.0.0/8 - subnetAvailable: CIDRRangeAlloc: "10.0.0.0/8" - desiredCIDR: "/22" + desiredCIDR: 22 analyzers: - subnetAvailable: outcomes: @@ -15,5 +15,5 @@ spec: when: "no-subnet-available" message: failed to find available subnet - pass: - when: "subnet-available" + when: "a-subnet-is-available" message: available /22 subnet found diff --git a/pkg/analyze/host_subnetavailable.go b/pkg/analyze/host_subnetavailable.go index 90c318629..262fbad2a 100644 --- a/pkg/analyze/host_subnetavailable.go +++ b/pkg/analyze/host_subnetavailable.go @@ -1,7 +1,13 @@ package analyzer import ( + "encoding/json" + "fmt" + "path/filepath" + + "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/replicatedhq/troubleshoot/pkg/collect" ) type AnalyzeHostSubnetAvailable struct { @@ -19,12 +25,30 @@ func (a *AnalyzeHostSubnetAvailable) IsExcluded() (bool, error) { func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(string) ([]byte, error)) ([]*AnalyzeResult, error) { hostAnalyzer := a.hostAnalyzer + name := filepath.Join("host-collectors/subnetAvailable", "result.json") + if hostAnalyzer.CollectorName != "" { + name = filepath.Join("host-collectors/subnetAvailable", hostAnalyzer.CollectorName+".json") + } + contents, err := getCollectedFileContents(name) + if err != nil { + return nil, errors.Wrap(err, "failed to get collected file") + } + + isSubnetAvailable := &collect.SubnetAvailableResult{} + if err := json.Unmarshal(contents, isSubnetAvailable); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal subnetAvailable result") + } + + fmt.Printf("%+v\n", isSubnetAvailable) + result := &AnalyzeResult{ Title: a.Title(), } for _, outcome := range hostAnalyzer.Outcomes { + fmt.Printf("fail: %+v pass: %+v\n", outcome.Fail, outcome.Pass) if outcome.Fail != nil { + fmt.Printf("fail.when: %s\n", outcome.Fail.When) if outcome.Fail.When == "" { result.IsFail = true result.Message = outcome.Fail.Message @@ -33,8 +57,7 @@ func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(strin return []*AnalyzeResult{result}, nil } - // TODO: implement - if string("TODO") == outcome.Fail.When { + if string(isSubnetAvailable.Status) == outcome.Fail.When { result.IsFail = true result.Message = outcome.Fail.Message result.URI = outcome.Fail.URI @@ -42,6 +65,7 @@ func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(strin return []*AnalyzeResult{result}, nil } } else if outcome.Pass != nil { + fmt.Printf("pass.when: %s\n", outcome.Pass.When) if outcome.Pass.When == "" { result.IsPass = true result.Message = outcome.Pass.Message @@ -50,8 +74,7 @@ func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(strin return []*AnalyzeResult{result}, nil } - // TODO: implement - if string("TODO") == outcome.Pass.When { + if string(isSubnetAvailable.Status) == outcome.Pass.When { result.IsPass = true result.Message = outcome.Pass.Message result.URI = outcome.Pass.URI diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 52075878d..ab80a4be0 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -17,11 +17,18 @@ import ( "github.com/replicatedhq/troubleshoot/pkg/debug" ) +type SubnetStatus string + +const ( + SubnetStatusAvailable = "a-subnet-is-available" + SubnetStatusNoneAvailable = "no-subnet-available" +) + type SubnetAvailableResult struct { CIDRRangeAlloc string `json:"CIDRRangeAlloc"` DesiredCIDR int `json:"desiredCIDR"` - // If true, at least 1 of the DesiredCIDR size is available within CIDRRangeAlloc - ADesiredIsAvailable bool `json:"aDesiredIsAvailable"` + // If subnet-available, at least 1 of the DesiredCIDR size is available within CIDRRangeAlloc + Status SubnetStatus `json:"aDesiredIsAvailable"` } type CollectHostSubnetAvailable struct { @@ -73,10 +80,15 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m result := SubnetAvailableResult{} result.CIDRRangeAlloc = c.hostCollector.CIDRRangeAlloc result.DesiredCIDR = c.hostCollector.DesiredCIDR - result.ADesiredIsAvailable, err = isASubnetAvailableInCIDR(c.hostCollector.DesiredCIDR, &cidrRangeAllocIPNet, &routes, false) + available, err := isASubnetAvailableInCIDR(c.hostCollector.DesiredCIDR, &cidrRangeAllocIPNet, &routes, false) if err != nil { return nil, errors.Wrap(err, "failed to determine if desired CIDR is available within subnet") } + if available == true { + result.Status = SubnetStatusAvailable + } else { + result.Status = SubnetStatusNoneAvailable + } b, err := json.Marshal(result) if err != nil { @@ -85,7 +97,7 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m collectorName := c.hostCollector.CollectorName if collectorName == "" { - collectorName = "subnetAvailable" + collectorName = "result" } name := filepath.Join("host-collectors/subnetAvailable", collectorName+".json") @@ -111,6 +123,10 @@ type systemRoute struct { func parseProcNetRoute(input string) (systemRoutes, error) { routes := systemRoutes{} for _, line := range strings.Split(input, "\n") { + if len(line) < 4 { + // Likely a blank line? + continue + } if line[0:5] == "Iface" { continue } From be64f5577c8537373551684578e22f391c7841de Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Fri, 17 Feb 2023 14:26:14 +1000 Subject: [PATCH 14/27] remove debug output --- pkg/analyze/host_subnetavailable.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/analyze/host_subnetavailable.go b/pkg/analyze/host_subnetavailable.go index 262fbad2a..e8b680168 100644 --- a/pkg/analyze/host_subnetavailable.go +++ b/pkg/analyze/host_subnetavailable.go @@ -2,7 +2,6 @@ package analyzer import ( "encoding/json" - "fmt" "path/filepath" "github.com/pkg/errors" @@ -39,16 +38,12 @@ func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(strin return nil, errors.Wrap(err, "failed to unmarshal subnetAvailable result") } - fmt.Printf("%+v\n", isSubnetAvailable) - result := &AnalyzeResult{ Title: a.Title(), } for _, outcome := range hostAnalyzer.Outcomes { - fmt.Printf("fail: %+v pass: %+v\n", outcome.Fail, outcome.Pass) if outcome.Fail != nil { - fmt.Printf("fail.when: %s\n", outcome.Fail.When) if outcome.Fail.When == "" { result.IsFail = true result.Message = outcome.Fail.Message @@ -65,7 +60,6 @@ func (a *AnalyzeHostSubnetAvailable) Analyze(getCollectedFileContents func(strin return []*AnalyzeResult{result}, nil } } else if outcome.Pass != nil { - fmt.Printf("pass.when: %s\n", outcome.Pass.When) if outcome.Pass.When == "" { result.IsPass = true result.Message = outcome.Pass.Message From bb2026a7c8f9addf634c7df608bf0d139613eba5 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Fri, 17 Feb 2023 14:37:13 +1000 Subject: [PATCH 15/27] remove the unnecessary return, apparently its unreachable code... I would have loved to keep it for safety but all good --- pkg/collect/host_subnetavailable.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index ab80a4be0..e29d60fc3 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -232,9 +232,6 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys // Try the next subnet in the range subnet, _ = cidr.NextSubnet(subnet, cidrRange) } - - // No subnets of cidrRange size were available within subnetRange (exhausted) - return false, nil } // findFirstOverlappingRoute will return the first overlapping route with the subnet specified From a3286cc961ea641e58f5a146d15ee1966be6227f Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Fri, 17 Feb 2023 14:49:14 +1000 Subject: [PATCH 16/27] fix spacing in example --- .../troubleshoot_v1beta2_subnetavailable.yaml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/config/samples/troubleshoot_v1beta2_subnetavailable.yaml b/config/samples/troubleshoot_v1beta2_subnetavailable.yaml index 2e5673b42..e932da411 100644 --- a/config/samples/troubleshoot_v1beta2_subnetavailable.yaml +++ b/config/samples/troubleshoot_v1beta2_subnetavailable.yaml @@ -6,14 +6,14 @@ spec: collectors: # would output yes/no depending if there is a /22 available in 10.0.0.0/8 - subnetAvailable: - CIDRRangeAlloc: "10.0.0.0/8" - desiredCIDR: 22 + CIDRRangeAlloc: "10.0.0.0/8" + desiredCIDR: 22 analyzers: - subnetAvailable: - outcomes: - - fail: - when: "no-subnet-available" - message: failed to find available subnet - - pass: - when: "a-subnet-is-available" - message: available /22 subnet found + outcomes: + - fail: + when: "no-subnet-available" + message: failed to find available subnet + - pass: + when: "a-subnet-is-available" + message: available /22 subnet found From cde6172b1273797c81352f29dcb39c4bdddfe14e Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Fri, 17 Feb 2023 14:54:11 +1000 Subject: [PATCH 17/27] subnet available collector: fix status JSON field name --- pkg/collect/host_subnetavailable.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index e29d60fc3..11c93c24c 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -28,7 +28,7 @@ type SubnetAvailableResult struct { CIDRRangeAlloc string `json:"CIDRRangeAlloc"` DesiredCIDR int `json:"desiredCIDR"` // If subnet-available, at least 1 of the DesiredCIDR size is available within CIDRRangeAlloc - Status SubnetStatus `json:"aDesiredIsAvailable"` + Status SubnetStatus `json:"status"` } type CollectHostSubnetAvailable struct { From 9f06b4d019f4ac351a2866f2057a7514ac55e37e Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 22 Feb 2023 12:30:09 +1000 Subject: [PATCH 18/27] subnet available collector: include output of `make generate` --- .../v1beta2/zz_generated.deepcopy.go | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go b/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go index 29b154d1d..beac3cf70 100644 --- a/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go +++ b/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go @@ -1549,6 +1549,11 @@ func (in *HostAnalyze) DeepCopyInto(out *HostAnalyze) { *out = new(IPV4InterfacesAnalyze) (*in).DeepCopyInto(*out) } + if in.SubnetAvailable != nil { + in, out := &in.SubnetAvailable, &out.SubnetAvailable + *out = new(SubnetAvailableAnalyze) + (*in).DeepCopyInto(*out) + } if in.FilesystemPerformance != nil { in, out := &in.FilesystemPerformance, &out.FilesystemPerformance *out = new(FilesystemPerformanceAnalyze) @@ -1640,6 +1645,11 @@ func (in *HostCollect) DeepCopyInto(out *HostCollect) { *out = new(IPV4Interfaces) (*in).DeepCopyInto(*out) } + if in.SubnetAvailable != nil { + in, out := &in.SubnetAvailable, &out.SubnetAvailable + *out = new(SubnetAvailable) + (*in).DeepCopyInto(*out) + } if in.DiskUsage != nil { in, out := &in.DiskUsage, &out.DiskUsage *out = new(DiskUsage) @@ -3121,6 +3131,11 @@ func (in *RemoteCollect) DeepCopyInto(out *RemoteCollect) { *out = new(RemoteIPV4Interfaces) (*in).DeepCopyInto(*out) } + if in.SubnetAvailable != nil { + in, out := &in.SubnetAvailable, &out.SubnetAvailable + *out = new(RemoteSubnetAvailable) + (*in).DeepCopyInto(*out) + } if in.DiskUsage != nil { in, out := &in.DiskUsage, &out.DiskUsage *out = new(RemoteDiskUsage) @@ -3465,6 +3480,22 @@ func (in *RemoteServices) DeepCopy() *RemoteServices { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RemoteSubnetAvailable) DeepCopyInto(out *RemoteSubnetAvailable) { + *out = *in + in.RemoteCollectorMeta.DeepCopyInto(&out.RemoteCollectorMeta) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RemoteSubnetAvailable. +func (in *RemoteSubnetAvailable) DeepCopy() *RemoteSubnetAvailable { + if in == nil { + return nil + } + out := new(RemoteSubnetAvailable) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RemoteSystemPackages) DeepCopyInto(out *RemoteSystemPackages) { *out = *in @@ -3791,6 +3822,49 @@ func (in *StorageClass) DeepCopy() *StorageClass { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SubnetAvailable) DeepCopyInto(out *SubnetAvailable) { + *out = *in + in.HostCollectorMeta.DeepCopyInto(&out.HostCollectorMeta) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SubnetAvailable. +func (in *SubnetAvailable) DeepCopy() *SubnetAvailable { + if in == nil { + return nil + } + out := new(SubnetAvailable) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SubnetAvailableAnalyze) DeepCopyInto(out *SubnetAvailableAnalyze) { + *out = *in + in.AnalyzeMeta.DeepCopyInto(&out.AnalyzeMeta) + if in.Outcomes != nil { + in, out := &in.Outcomes, &out.Outcomes + *out = make([]*Outcome, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Outcome) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SubnetAvailableAnalyze. +func (in *SubnetAvailableAnalyze) DeepCopy() *SubnetAvailableAnalyze { + if in == nil { + return nil + } + out := new(SubnetAvailableAnalyze) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SupportBundle) DeepCopyInto(out *SupportBundle) { *out = *in From 4529eea5d011feeb1b9bb895a5b017da46eabecc Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 22 Feb 2023 12:38:14 +1000 Subject: [PATCH 19/27] move the example subnet available collector/analyzer spec, ref https://github.com/replicatedhq/troubleshoot/pull/1004#discussion_r1111918736 --- .../preflight/host}/troubleshoot_v1beta2_subnetavailable.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {config/samples => examples/preflight/host}/troubleshoot_v1beta2_subnetavailable.yaml (100%) diff --git a/config/samples/troubleshoot_v1beta2_subnetavailable.yaml b/examples/preflight/host/troubleshoot_v1beta2_subnetavailable.yaml similarity index 100% rename from config/samples/troubleshoot_v1beta2_subnetavailable.yaml rename to examples/preflight/host/troubleshoot_v1beta2_subnetavailable.yaml From cee70d6ee3b7590846b2a243e806f8e07a135136 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 22 Feb 2023 12:39:36 +1000 Subject: [PATCH 20/27] rename the subnet available collector/analyzer example spec, fix indents also --- examples/preflight/host/subnet-available.yaml | 19 +++++++++++++++++++ .../troubleshoot_v1beta2_subnetavailable.yaml | 19 ------------------- 2 files changed, 19 insertions(+), 19 deletions(-) create mode 100644 examples/preflight/host/subnet-available.yaml delete mode 100644 examples/preflight/host/troubleshoot_v1beta2_subnetavailable.yaml diff --git a/examples/preflight/host/subnet-available.yaml b/examples/preflight/host/subnet-available.yaml new file mode 100644 index 000000000..34fb1c7f7 --- /dev/null +++ b/examples/preflight/host/subnet-available.yaml @@ -0,0 +1,19 @@ +apiVersion: troubleshoot.sh/v1beta2 +kind: HostPreflight +metadata: + name: subnet-available +spec: + collectors: + # would output yes/no depending if there is a /22 available in 10.0.0.0/8 + - subnetAvailable: + CIDRRangeAlloc: "10.0.0.0/8" + desiredCIDR: 22 + analyzers: + - subnetAvailable: + outcomes: + - fail: + when: "no-subnet-available" + message: failed to find available subnet + - pass: + when: "a-subnet-is-available" + message: available /22 subnet found diff --git a/examples/preflight/host/troubleshoot_v1beta2_subnetavailable.yaml b/examples/preflight/host/troubleshoot_v1beta2_subnetavailable.yaml deleted file mode 100644 index e932da411..000000000 --- a/examples/preflight/host/troubleshoot_v1beta2_subnetavailable.yaml +++ /dev/null @@ -1,19 +0,0 @@ -apiVersion: troubleshoot.sh/v1beta2 -kind: HostPreflight -metadata: - name: subnet-available -spec: - collectors: - # would output yes/no depending if there is a /22 available in 10.0.0.0/8 - - subnetAvailable: - CIDRRangeAlloc: "10.0.0.0/8" - desiredCIDR: 22 - analyzers: - - subnetAvailable: - outcomes: - - fail: - when: "no-subnet-available" - message: failed to find available subnet - - pass: - when: "a-subnet-is-available" - message: available /22 subnet found From d461aa9d7f5e18702a950bb862bb3da7696be36b Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 22 Feb 2023 12:41:53 +1000 Subject: [PATCH 21/27] allow picking a subnet from a /0 (all ipv4 space), ref https://github.com/replicatedhq/troubleshoot/pull/1004#discussion_r1111957159 --- pkg/collect/host_subnetavailable.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 11c93c24c..f54b1547c 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -69,8 +69,8 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("CIDRRangeAlloc mask %s invalid, expected integer", splitCIDRRangeAlloc[1])) } - if maskInt < 1 || maskInt > 32 { - return nil, errors.Wrap(err, fmt.Sprintf("CIDRRangeAlloc mask %d invalid, must be between 1 and 32", maskInt)) + if maskInt < 0 || maskInt > 32 { + return nil, errors.Wrap(err, fmt.Sprintf("CIDRRangeAlloc mask %d invalid, must be between 0 and 32", maskInt)) } cidrRangeAllocIPNet := net.IPNet{ IP: net.ParseIP(splitCIDRRangeAlloc[0]), From 12a334d141b70f4fe6495c9ef74f52e562e89af4 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Wed, 22 Feb 2023 12:58:12 +1000 Subject: [PATCH 22/27] subnet available collector/analyzer: make schemas --- config/crds/troubleshoot.sh_analyzers.yaml | 49 ++++++++++ .../crds/troubleshoot.sh_hostcollectors.yaml | 63 ++++++++++++ .../crds/troubleshoot.sh_hostpreflights.yaml | 77 +++++++++++++++ config/crds/troubleshoot.sh_preflights.yaml | 14 +++ .../troubleshoot.sh_remotecollectors.yaml | 14 +++ .../crds/troubleshoot.sh_supportbundles.yaml | 63 ++++++++++++ schemas/analyzer-troubleshoot-v1beta2.json | 76 +++++++++++++++ schemas/preflight-troubleshoot-v1beta2.json | 21 ++++ .../supportbundle-troubleshoot-v1beta2.json | 97 +++++++++++++++++++ 9 files changed, 474 insertions(+) diff --git a/config/crds/troubleshoot.sh_analyzers.yaml b/config/crds/troubleshoot.sh_analyzers.yaml index aa15a31de..d5d78e642 100644 --- a/config/crds/troubleshoot.sh_analyzers.yaml +++ b/config/crds/troubleshoot.sh_analyzers.yaml @@ -2056,6 +2056,55 @@ spec: required: - outcomes type: object + subnetAvailable: + properties: + annotations: + additionalProperties: + type: string + type: object + checkName: + type: string + collectorName: + type: string + exclude: + type: BoolString + outcomes: + items: + properties: + fail: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + pass: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + warn: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + type: object + type: array + strict: + type: BoolString + required: + - outcomes + type: object systemPackages: properties: annotations: diff --git a/config/crds/troubleshoot.sh_hostcollectors.yaml b/config/crds/troubleshoot.sh_hostcollectors.yaml index df2bfe634..66c4d3855 100644 --- a/config/crds/troubleshoot.sh_hostcollectors.yaml +++ b/config/crds/troubleshoot.sh_hostcollectors.yaml @@ -633,6 +633,55 @@ spec: required: - outcomes type: object + subnetAvailable: + properties: + annotations: + additionalProperties: + type: string + type: object + checkName: + type: string + collectorName: + type: string + exclude: + type: BoolString + outcomes: + items: + properties: + fail: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + pass: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + warn: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + type: object + type: array + strict: + type: BoolString + required: + - outcomes + type: object systemPackages: properties: annotations: @@ -1176,6 +1225,20 @@ spec: - args - command type: object + subnetAvailable: + properties: + CIDRRangeAlloc: + type: string + collectorName: + type: string + desiredCIDR: + type: integer + exclude: + type: BoolString + required: + - CIDRRangeAlloc + - desiredCIDR + type: object systemPackages: properties: amzn: diff --git a/config/crds/troubleshoot.sh_hostpreflights.yaml b/config/crds/troubleshoot.sh_hostpreflights.yaml index 5725cbd31..4ca215383 100644 --- a/config/crds/troubleshoot.sh_hostpreflights.yaml +++ b/config/crds/troubleshoot.sh_hostpreflights.yaml @@ -633,6 +633,55 @@ spec: required: - outcomes type: object + subnetAvailable: + properties: + annotations: + additionalProperties: + type: string + type: object + checkName: + type: string + collectorName: + type: string + exclude: + type: BoolString + outcomes: + items: + properties: + fail: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + pass: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + warn: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + type: object + type: array + strict: + type: BoolString + required: + - outcomes + type: object systemPackages: properties: annotations: @@ -1176,6 +1225,20 @@ spec: - args - command type: object + subnetAvailable: + properties: + CIDRRangeAlloc: + type: string + collectorName: + type: string + desiredCIDR: + type: integer + exclude: + type: BoolString + required: + - CIDRRangeAlloc + - desiredCIDR + type: object systemPackages: properties: amzn: @@ -1524,6 +1587,20 @@ spec: exclude: type: BoolString type: object + subnetAvailable: + properties: + CIDRRangeAlloc: + type: string + collectorName: + type: string + desiredCIDR: + type: integer + exclude: + type: BoolString + required: + - CIDRRangeAlloc + - desiredCIDR + type: object systemPackages: properties: collectorName: diff --git a/config/crds/troubleshoot.sh_preflights.yaml b/config/crds/troubleshoot.sh_preflights.yaml index 702efba0a..c0efb475a 100644 --- a/config/crds/troubleshoot.sh_preflights.yaml +++ b/config/crds/troubleshoot.sh_preflights.yaml @@ -10153,6 +10153,20 @@ spec: exclude: type: BoolString type: object + subnetAvailable: + properties: + CIDRRangeAlloc: + type: string + collectorName: + type: string + desiredCIDR: + type: integer + exclude: + type: BoolString + required: + - CIDRRangeAlloc + - desiredCIDR + type: object systemPackages: properties: collectorName: diff --git a/config/crds/troubleshoot.sh_remotecollectors.yaml b/config/crds/troubleshoot.sh_remotecollectors.yaml index 6a20532a9..7f4f89685 100644 --- a/config/crds/troubleshoot.sh_remotecollectors.yaml +++ b/config/crds/troubleshoot.sh_remotecollectors.yaml @@ -283,6 +283,20 @@ spec: exclude: type: BoolString type: object + subnetAvailable: + properties: + CIDRRangeAlloc: + type: string + collectorName: + type: string + desiredCIDR: + type: integer + exclude: + type: BoolString + required: + - CIDRRangeAlloc + - desiredCIDR + type: object systemPackages: properties: collectorName: diff --git a/config/crds/troubleshoot.sh_supportbundles.yaml b/config/crds/troubleshoot.sh_supportbundles.yaml index 10c424019..8cbd7f3cc 100644 --- a/config/crds/troubleshoot.sh_supportbundles.yaml +++ b/config/crds/troubleshoot.sh_supportbundles.yaml @@ -10565,6 +10565,55 @@ spec: required: - outcomes type: object + subnetAvailable: + properties: + annotations: + additionalProperties: + type: string + type: object + checkName: + type: string + collectorName: + type: string + exclude: + type: BoolString + outcomes: + items: + properties: + fail: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + pass: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + warn: + properties: + message: + type: string + uri: + type: string + when: + type: string + type: object + type: object + type: array + strict: + type: BoolString + required: + - outcomes + type: object systemPackages: properties: annotations: @@ -11108,6 +11157,20 @@ spec: - args - command type: object + subnetAvailable: + properties: + CIDRRangeAlloc: + type: string + collectorName: + type: string + desiredCIDR: + type: integer + exclude: + type: BoolString + required: + - CIDRRangeAlloc + - desiredCIDR + type: object systemPackages: properties: amzn: diff --git a/schemas/analyzer-troubleshoot-v1beta2.json b/schemas/analyzer-troubleshoot-v1beta2.json index 87fa17ef3..bde7047ed 100644 --- a/schemas/analyzer-troubleshoot-v1beta2.json +++ b/schemas/analyzer-troubleshoot-v1beta2.json @@ -3133,6 +3133,82 @@ } } }, + "subnetAvailable": { + "type": "object", + "required": [ + "outcomes" + ], + "properties": { + "annotations": { + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "checkName": { + "type": "string" + }, + "collectorName": { + "type": "string" + }, + "exclude": { + "oneOf": [{"type": "string"},{"type": "boolean"}] + }, + "outcomes": { + "type": "array", + "items": { + "type": "object", + "properties": { + "fail": { + "type": "object", + "properties": { + "message": { + "type": "string" + }, + "uri": { + "type": "string" + }, + "when": { + "type": "string" + } + } + }, + "pass": { + "type": "object", + "properties": { + "message": { + "type": "string" + }, + "uri": { + "type": "string" + }, + "when": { + "type": "string" + } + } + }, + "warn": { + "type": "object", + "properties": { + "message": { + "type": "string" + }, + "uri": { + "type": "string" + }, + "when": { + "type": "string" + } + } + } + } + } + }, + "strict": { + "oneOf": [{"type": "string"},{"type": "boolean"}] + } + } + }, "systemPackages": { "type": "object", "required": [ diff --git a/schemas/preflight-troubleshoot-v1beta2.json b/schemas/preflight-troubleshoot-v1beta2.json index 1329f958e..855699c52 100644 --- a/schemas/preflight-troubleshoot-v1beta2.json +++ b/schemas/preflight-troubleshoot-v1beta2.json @@ -9293,6 +9293,27 @@ } } }, + "subnetAvailable": { + "type": "object", + "required": [ + "CIDRRangeAlloc", + "desiredCIDR" + ], + "properties": { + "CIDRRangeAlloc": { + "type": "string" + }, + "collectorName": { + "type": "string" + }, + "desiredCIDR": { + "type": "integer" + }, + "exclude": { + "oneOf": [{"type": "string"},{"type": "boolean"}] + } + } + }, "systemPackages": { "type": "object", "properties": { diff --git a/schemas/supportbundle-troubleshoot-v1beta2.json b/schemas/supportbundle-troubleshoot-v1beta2.json index 359f12911..54f6b679a 100644 --- a/schemas/supportbundle-troubleshoot-v1beta2.json +++ b/schemas/supportbundle-troubleshoot-v1beta2.json @@ -9982,6 +9982,82 @@ } } }, + "subnetAvailable": { + "type": "object", + "required": [ + "outcomes" + ], + "properties": { + "annotations": { + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "checkName": { + "type": "string" + }, + "collectorName": { + "type": "string" + }, + "exclude": { + "oneOf": [{"type": "string"},{"type": "boolean"}] + }, + "outcomes": { + "type": "array", + "items": { + "type": "object", + "properties": { + "fail": { + "type": "object", + "properties": { + "message": { + "type": "string" + }, + "uri": { + "type": "string" + }, + "when": { + "type": "string" + } + } + }, + "pass": { + "type": "object", + "properties": { + "message": { + "type": "string" + }, + "uri": { + "type": "string" + }, + "when": { + "type": "string" + } + } + }, + "warn": { + "type": "object", + "properties": { + "message": { + "type": "string" + }, + "uri": { + "type": "string" + }, + "when": { + "type": "string" + } + } + } + } + } + }, + "strict": { + "oneOf": [{"type": "string"},{"type": "boolean"}] + } + } + }, "systemPackages": { "type": "object", "required": [ @@ -10770,6 +10846,27 @@ } } }, + "subnetAvailable": { + "type": "object", + "required": [ + "CIDRRangeAlloc", + "desiredCIDR" + ], + "properties": { + "CIDRRangeAlloc": { + "type": "string" + }, + "collectorName": { + "type": "string" + }, + "desiredCIDR": { + "type": "integer" + }, + "exclude": { + "oneOf": [{"type": "string"},{"type": "boolean"}] + } + } + }, "systemPackages": { "type": "object", "properties": { From 0981e7240d2a14c7be58074f5311c4b419eb1644 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Tue, 7 Mar 2023 12:40:25 +1000 Subject: [PATCH 23/27] subnetavailable collector: have to remove this test, it can't be run reliably on MacOS (unless we feed in a mocked /proc/net/route, which is non-trivial) --- pkg/collect/host_subnetavailable_test.go | 33 ------------------------ 1 file changed, 33 deletions(-) diff --git a/pkg/collect/host_subnetavailable_test.go b/pkg/collect/host_subnetavailable_test.go index 55ff3ab61..793e62d29 100644 --- a/pkg/collect/host_subnetavailable_test.go +++ b/pkg/collect/host_subnetavailable_test.go @@ -8,39 +8,6 @@ import ( "github.com/stretchr/testify/require" ) -/* -func TestCollectHostSubnetAvailable_Collect(t *testing.T) { - type fields struct { - hostCollector *troubleshootv1beta2.SubnetAvailable - } - tests := []struct { - name string - fields fields - want map[string][]byte - }{ - { - name: "TODO", - want: map[string][]byte{ - "host-collectors/subnetAvailable/subnetAvailable.json": []byte(`{"status":"connected","message":""}`), - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "bundle") - require.NoError(t, err) - - c := &CollectHostSubnetAvailable{ - hostCollector: &troubleshootv1beta2.SubnetAvailable{ - // TODO: implement - }, - BundlePath: tmpDir, - } - }) - } -} -*/ - func TestParseProcNetRoute(t *testing.T) { input := `Iface Destination Gateway Flags RefCnt Use Metric Mask MTU Window IRTT eth1 00000000 016BA8C0 0003 0 0 200 00000000 0 0 0 From dc4ea46575889ddcb87a0f2f7c5704965f680308 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Thu, 9 Mar 2023 15:17:35 +1000 Subject: [PATCH 24/27] subnetAvailable collector: change to klog --- pkg/collect/host_subnetavailable.go | 34 +++++++++++------------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index f54b1547c..d4f030de1 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -14,7 +14,7 @@ import ( "github.com/apparentlymart/go-cidr/cidr" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" - "github.com/replicatedhq/troubleshoot/pkg/debug" + "k8s.io/klog/v2" ) type SubnetStatus string @@ -54,7 +54,7 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m if err != nil { return nil, errors.Wrap(err, "failed to parse /proc/net/route") } - debug.Printf("Routes: %+v\n", routes) + klog.V(2).Infof("Routes: %+v\n", routes) // IPv4 only right now... if c.hostCollector.DesiredCIDR < 1 || c.hostCollector.DesiredCIDR > 32 { @@ -185,7 +185,7 @@ func parseProcNetRoute(input string) (systemRoutes, error) { // TODOLATER: consolidate some of this logic into a unified library? will need a bit of refactoring if so // // isASubnetAvailableInCIDR will check if a subnet of cidrRange size is available within subnetRange (IPv4 only), checking against system routes for conflicts -func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *systemRoutes, debug bool) (bool, error) { +func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *systemRoutes) (bool, error) { // Sanity check that the CIDR range size is IPv4 valid (/0 to /32) if cidrRange < 1 || cidrRange > 32 { return false, errors.New(fmt.Sprintf("CIDR range size %d invalid, must be between 1 and 32", cidrRange)) @@ -213,9 +213,7 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys for { // This is the extents of the subnet to be tested firstIP, lastIP := cidr.AddressRange(subnet) - if debug { - fmt.Fprintf(os.Stderr, "Checking subnet: %s firstIP: %s lastIP: %s\n", subnet.String(), firstIP, lastIP) - } + klog.V(2).Infof("Checking subnet: %s firstIP: %s lastIP: %s\n", subnet.String(), firstIP, lastIP) // Make sure the smaller subnet is (still) within the large subnetRange. If subnetRange has been exhausted one of these will be false if !subnetRange.Contains(firstIP) || !subnetRange.Contains(lastIP) { @@ -223,19 +221,21 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys } // Check if any system routes overlap with the smaller subnet being tested - route := findFirstOverlappingRoute(subnet, routes, debug) + route := findFirstOverlappingRoute(subnet, routes) if route == nil { // No system routes match, this (smaller) subnet is available + klog.V(1).Infof("Subnet %s is available", subnet.String()) return true, nil } // Try the next subnet in the range - subnet, _ = cidr.NextSubnet(subnet, cidrRange) + //subnet, _ = cidr.NextSubnet(subnet, cidrRange) + subnet, _ = cidr.NextSubnet(&route.DestNet, cidrRange) } } // findFirstOverlappingRoute will return the first overlapping route with the subnet specified -func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes, debug bool) *systemRoute { +func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes) *systemRoute { // NOTE: IPv4 specific defaultRoute := net.IPNet{ IP: net.IPv4(0, 0, 0, 0), @@ -248,25 +248,17 @@ func findFirstOverlappingRoute(subnet *net.IPNet, routes *systemRoutes, debug bo continue } - if debug { - fmt.Fprintf(os.Stderr, "Checking if route %s overlaps with subnet %s - ", &route.DestNet, subnet) - } + klog.V(2).Infof("Checking if route %s overlaps with subnet %s - ", &route.DestNet, subnet) // TODOLATER: can we use cidr.VerifyNoOverlap to replace this? tests fail right now trying to do so... //if cidr.VerifyNoOverlap([]*net.IPNet{subnet}, &route.DestNet) != nil { if netOverlaps(&route.DestNet, subnet) { - if debug { - fmt.Fprintf(os.Stderr, "Overlaps\n") - } + klog.V(2).Infof("Overlaps\n") return &route } else { - if debug { - fmt.Fprintf(os.Stderr, "No overlap\n") - } + klog.V(2).Infof("No overlap\n") } } - if debug { - fmt.Fprintf(os.Stderr, "Subnet %s has no overlap with any system routes\n", subnet) - } + klog.V(2).Infof("Subnet %s has no overlap with any system routes\n", subnet) return nil } From fcfab32cea95ce1a93b5d06704345330d33711e1 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Thu, 9 Mar 2023 15:18:54 +1000 Subject: [PATCH 25/27] subnetAvailable collector: push the routes list even further down the debug output, as its messy --- pkg/collect/host_subnetavailable.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index d4f030de1..a57866a8c 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -54,7 +54,7 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m if err != nil { return nil, errors.Wrap(err, "failed to parse /proc/net/route") } - klog.V(2).Infof("Routes: %+v\n", routes) + klog.V(3).Infof("Routes: %+v\n", routes) // IPv4 only right now... if c.hostCollector.DesiredCIDR < 1 || c.hostCollector.DesiredCIDR > 32 { From 61e489fddb70cc82bcaa5314bd3eed62e029d203 Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Thu, 9 Mar 2023 15:20:52 +1000 Subject: [PATCH 26/27] subnetAvailable collector: oops remove debug param --- pkg/collect/host_subnetavailable.go | 2 +- pkg/collect/host_subnetavailable_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index a57866a8c..9f190e2b2 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -80,7 +80,7 @@ func (c *CollectHostSubnetAvailable) Collect(progressChan chan<- interface{}) (m result := SubnetAvailableResult{} result.CIDRRangeAlloc = c.hostCollector.CIDRRangeAlloc result.DesiredCIDR = c.hostCollector.DesiredCIDR - available, err := isASubnetAvailableInCIDR(c.hostCollector.DesiredCIDR, &cidrRangeAllocIPNet, &routes, false) + available, err := isASubnetAvailableInCIDR(c.hostCollector.DesiredCIDR, &cidrRangeAllocIPNet, &routes) if err != nil { return nil, errors.Wrap(err, "failed to determine if desired CIDR is available within subnet") } diff --git a/pkg/collect/host_subnetavailable_test.go b/pkg/collect/host_subnetavailable_test.go index 793e62d29..27271f677 100644 --- a/pkg/collect/host_subnetavailable_test.go +++ b/pkg/collect/host_subnetavailable_test.go @@ -194,7 +194,7 @@ func TestIsASubnetAvailableInCIDR(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := require.New(t) - actual, err := isASubnetAvailableInCIDR(tt.cidrRange, &tt.subnetRange, &sysRoutes, false) // debug bool is useful for fixing bugs here, but off by default for noise + actual, err := isASubnetAvailableInCIDR(tt.cidrRange, &tt.subnetRange, &sysRoutes) // debug bool is useful for fixing bugs here, but off by default for noise req.NoError(err) assert.Equal(t, tt.expected, actual) From 3fc02f97a6d4686bca24f4f1d93b5ca1912f1a3a Mon Sep 17 00:00:00 2001 From: Nathan Sullivan Date: Thu, 9 Mar 2023 15:31:40 +1000 Subject: [PATCH 27/27] oops... :) --- pkg/collect/host_subnetavailable.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/collect/host_subnetavailable.go b/pkg/collect/host_subnetavailable.go index 9f190e2b2..a1457a185 100644 --- a/pkg/collect/host_subnetavailable.go +++ b/pkg/collect/host_subnetavailable.go @@ -229,8 +229,7 @@ func isASubnetAvailableInCIDR(cidrRange int, subnetRange *net.IPNet, routes *sys } // Try the next subnet in the range - //subnet, _ = cidr.NextSubnet(subnet, cidrRange) - subnet, _ = cidr.NextSubnet(&route.DestNet, cidrRange) + subnet, _ = cidr.NextSubnet(subnet, cidrRange) } }