From b88bc8ddf766b237b45aaa734d38294ec8f3b3cb Mon Sep 17 00:00:00 2001 From: Diamon Wiggins <38189728+diamonwiggins@users.noreply.github.com> Date: Mon, 21 Oct 2024 17:45:50 -0400 Subject: [PATCH] Refactor Multi Node Analyzers (#1646) * initial refactor of host os analyzer * refactor remote collect analysis --------- Signed-off-by: Evans Mungai Co-authored-by: Gerard Nguyen Co-authored-by: Evans Mungai --- pkg/analyze/collected_contents.go | 58 +++ pkg/analyze/collected_contents_test.go | 138 +++++++ pkg/analyze/host_analyzer.go | 106 ++++- pkg/analyze/host_analyzer_test.go | 299 ++++++++++++++ pkg/analyze/host_memory.go | 122 ++---- pkg/analyze/host_memory_test.go | 271 ++++++++----- pkg/analyze/host_os_info.go | 228 ++++------- pkg/analyze/host_os_info_test.go | 534 ++++++++++--------------- pkg/collect/host_memory.go | 1 + pkg/supportbundle/collect.go | 91 ++--- pkg/supportbundle/collect_test.go | 45 --- 11 files changed, 1128 insertions(+), 765 deletions(-) create mode 100644 pkg/analyze/collected_contents.go create mode 100644 pkg/analyze/collected_contents_test.go create mode 100644 pkg/analyze/host_analyzer_test.go delete mode 100644 pkg/supportbundle/collect_test.go diff --git a/pkg/analyze/collected_contents.go b/pkg/analyze/collected_contents.go new file mode 100644 index 000000000..e8abe7ee5 --- /dev/null +++ b/pkg/analyze/collected_contents.go @@ -0,0 +1,58 @@ +package analyzer + +import ( + "encoding/json" + "fmt" + + "github.com/pkg/errors" + "github.com/replicatedhq/troubleshoot/pkg/constants" +) + +type collectedContent struct { + NodeName string + Data collectorData +} + +type collectorData interface{} + +type nodeNames struct { + Nodes []string `json:"nodes"` +} + +func retrieveCollectedContents( + getCollectedFileContents func(string) ([]byte, error), + localPath string, remoteNodeBaseDir string, remoteFileName string, +) ([]collectedContent, error) { + var collectedContents []collectedContent + + // Try to retrieve local data first + if contents, err := getCollectedFileContents(localPath); err == nil { + collectedContents = append(collectedContents, collectedContent{NodeName: "", Data: contents}) + // Return immediately if local content is available + return collectedContents, nil + } + + // Local data not available, move to remote collection + nodeListContents, err := getCollectedFileContents(constants.NODE_LIST_FILE) + if err != nil { + return nil, errors.Wrap(err, "failed to get node list") + } + + var nodeNames nodeNames + if err := json.Unmarshal(nodeListContents, &nodeNames); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal node names") + } + + // Collect data for each node + for _, node := range nodeNames.Nodes { + nodeFilePath := fmt.Sprintf("%s/%s/%s", remoteNodeBaseDir, node, remoteFileName) + nodeContents, err := getCollectedFileContents(nodeFilePath) + if err != nil { + return nil, errors.Wrapf(err, "failed to retrieve content for node %s", node) + } + + collectedContents = append(collectedContents, collectedContent{NodeName: node, Data: nodeContents}) + } + + return collectedContents, nil +} diff --git a/pkg/analyze/collected_contents_test.go b/pkg/analyze/collected_contents_test.go new file mode 100644 index 000000000..c7200b4db --- /dev/null +++ b/pkg/analyze/collected_contents_test.go @@ -0,0 +1,138 @@ +package analyzer + +import ( + "encoding/json" + "testing" + + "github.com/replicatedhq/troubleshoot/pkg/constants" + "github.com/replicatedhq/troubleshoot/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRetrieveCollectedContents(t *testing.T) { + tests := []struct { + name string + getCollectedFileContents func(string) ([]byte, error) // Mock function + localPath string + remoteNodeBaseDir string + remoteFileName string + expectedResult []collectedContent + expectedError string + }{ + { + name: "successfully retrieve local content", + getCollectedFileContents: func(path string) ([]byte, error) { + if path == "localPath" { + return []byte("localContent"), nil + } + return nil, &types.NotFoundError{Name: path} + }, + localPath: "localPath", + remoteNodeBaseDir: "remoteBaseDir", + remoteFileName: "remoteFileName", + expectedResult: []collectedContent{ + { + NodeName: "", + Data: []byte("localContent"), + }, + }, + expectedError: "", + }, + { + name: "local content not found, retrieve remote node content successfully", + getCollectedFileContents: func(path string) ([]byte, error) { + if path == constants.NODE_LIST_FILE { + nodeNames := nodeNames{Nodes: []string{"node1", "node2"}} + return json.Marshal(nodeNames) + } + if path == "remoteBaseDir/node1/remoteFileName" { + return []byte("remoteContent1"), nil + } + if path == "remoteBaseDir/node2/remoteFileName" { + return []byte("remoteContent2"), nil + } + return nil, &types.NotFoundError{Name: path} + }, + localPath: "localPath", + remoteNodeBaseDir: "remoteBaseDir", + remoteFileName: "remoteFileName", + expectedResult: []collectedContent{ + { + NodeName: "node1", + Data: []byte("remoteContent1"), + }, + { + NodeName: "node2", + Data: []byte("remoteContent2"), + }, + }, + expectedError: "", + }, + { + name: "fail to retrieve local content and node list", + getCollectedFileContents: func(path string) ([]byte, error) { + return nil, &types.NotFoundError{Name: path} + }, + localPath: "localPath", + remoteNodeBaseDir: "remoteBaseDir", + remoteFileName: "remoteFileName", + expectedResult: nil, + expectedError: "failed to get node list", + }, + { + name: "fail to retrieve content for one of the nodes", + getCollectedFileContents: func(path string) ([]byte, error) { + if path == constants.NODE_LIST_FILE { + nodeNames := nodeNames{Nodes: []string{"node1", "node2"}} + return json.Marshal(nodeNames) + } + if path == "remoteBaseDir/node1/remoteFileName" { + return []byte("remoteContent1"), nil + } + if path == "remoteBaseDir/node2/remoteFileName" { + return nil, &types.NotFoundError{Name: path} + } + return nil, &types.NotFoundError{Name: path} + }, + localPath: "localPath", + remoteNodeBaseDir: "remoteBaseDir", + remoteFileName: "remoteFileName", + expectedResult: nil, + expectedError: "failed to retrieve content for node node2", + }, + { + name: "fail to unmarshal node list", + getCollectedFileContents: func(path string) ([]byte, error) { + if path == constants.NODE_LIST_FILE { + return []byte("invalidJSON"), nil + } + return nil, &types.NotFoundError{Name: path} + }, + localPath: "localPath", + remoteNodeBaseDir: "remoteBaseDir", + remoteFileName: "remoteFileName", + expectedResult: nil, + expectedError: "failed to unmarshal node names", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := retrieveCollectedContents( + test.getCollectedFileContents, + test.localPath, + test.remoteNodeBaseDir, + test.remoteFileName, + ) + + if test.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), test.expectedError) + } else { + require.NoError(t, err) + assert.Equal(t, test.expectedResult, result) + } + }) + } +} diff --git a/pkg/analyze/host_analyzer.go b/pkg/analyze/host_analyzer.go index cae6a26b3..6edc709d7 100644 --- a/pkg/analyze/host_analyzer.go +++ b/pkg/analyze/host_analyzer.go @@ -1,6 +1,11 @@ package analyzer -import troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" +import ( + "fmt" + + "github.com/pkg/errors" + troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" +) type HostAnalyzer interface { Title() string @@ -83,3 +88,102 @@ func (c *resultCollector) get(title string) []*AnalyzeResult { } return []*AnalyzeResult{{Title: title, IsWarn: true, Message: "no results"}} } + +func analyzeHostCollectorResults(collectedContent []collectedContent, outcomes []*troubleshootv1beta2.Outcome, checkCondition func(string, collectorData) (bool, error), title string) ([]*AnalyzeResult, error) { + var results []*AnalyzeResult + for _, content := range collectedContent { + currentTitle := title + if content.NodeName != "" { + currentTitle = fmt.Sprintf("%s - Node %s", title, content.NodeName) + } + + analyzeResult, err := evaluateOutcomes(outcomes, checkCondition, content.Data, currentTitle) + if err != nil { + return nil, errors.Wrap(err, "failed to evaluate outcomes") + } + if analyzeResult != nil { + results = append(results, analyzeResult...) + } + } + return results, nil +} + +func evaluateOutcomes(outcomes []*troubleshootv1beta2.Outcome, checkCondition func(string, collectorData) (bool, error), data collectorData, title string) ([]*AnalyzeResult, error) { + var results []*AnalyzeResult + + for _, outcome := range outcomes { + result := AnalyzeResult{ + Title: title, + } + + switch { + case outcome.Fail != nil: + if outcome.Fail.When == "" { + result.IsFail = true + result.Message = outcome.Fail.Message + result.URI = outcome.Fail.URI + results = append(results, &result) + return results, nil + } + + isMatch, err := checkCondition(outcome.Fail.When, data) + if err != nil { + return []*AnalyzeResult{&result}, errors.Wrapf(err, "failed to compare %s", outcome.Fail.When) + } + + if isMatch { + result.IsFail = true + result.Message = outcome.Fail.Message + result.URI = outcome.Fail.URI + results = append(results, &result) + return results, nil + } + + case outcome.Warn != nil: + if outcome.Warn.When == "" { + result.IsWarn = true + result.Message = outcome.Warn.Message + result.URI = outcome.Warn.URI + results = append(results, &result) + return results, nil + } + + isMatch, err := checkCondition(outcome.Warn.When, data) + if err != nil { + return []*AnalyzeResult{&result}, errors.Wrapf(err, "failed to compare %s", outcome.Warn.When) + } + + if isMatch { + result.IsWarn = true + result.Message = outcome.Warn.Message + result.URI = outcome.Warn.URI + results = append(results, &result) + return results, nil + } + + case outcome.Pass != nil: + if outcome.Pass.When == "" { + result.IsPass = true + result.Message = outcome.Pass.Message + result.URI = outcome.Pass.URI + results = append(results, &result) + return results, nil + } + + isMatch, err := checkCondition(outcome.Pass.When, data) + if err != nil { + return []*AnalyzeResult{&result}, errors.Wrapf(err, "failed to compare %s", outcome.Pass.When) + } + + if isMatch { + result.IsPass = true + result.Message = outcome.Pass.Message + result.URI = outcome.Pass.URI + results = append(results, &result) + return results, nil + } + } + } + + return nil, nil +} diff --git a/pkg/analyze/host_analyzer_test.go b/pkg/analyze/host_analyzer_test.go new file mode 100644 index 000000000..21eca0f71 --- /dev/null +++ b/pkg/analyze/host_analyzer_test.go @@ -0,0 +1,299 @@ +package analyzer + +import ( + "testing" + + "github.com/pkg/errors" + troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/replicatedhq/troubleshoot/pkg/collect" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAnalyzeHostCollectorResults(t *testing.T) { + tests := []struct { + name string + outcomes []*troubleshootv1beta2.Outcome + collectedContent []collectedContent + checkCondition func(string, collectorData) (bool, error) + expectResult []*AnalyzeResult + }{ + { + name: "pass if ubuntu >= 00.1.2", + collectedContent: []collectedContent{ + { + NodeName: "node1", + Data: collect.HostOSInfo{ + Name: "myhost", + KernelVersion: "5.4.0-1034-gcp", + PlatformVersion: "00.1.2", + Platform: "ubuntu", + }, + }, + }, + outcomes: []*troubleshootv1beta2.Outcome{ + { + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "ubuntu >= 00.1.2", + Message: "supported distribution matches ubuntu >= 00.1.2", + }, + }, + { + Fail: &troubleshootv1beta2.SingleOutcome{ + Message: "unsupported distribution", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + osInfo := data.(collect.HostOSInfo) + return osInfo.Platform == "ubuntu" && osInfo.PlatformVersion >= "00.1.2", nil + }, + expectResult: []*AnalyzeResult{ + { + Title: "Host OS Info - Node node1", + IsPass: true, + Message: "supported distribution matches ubuntu >= 00.1.2", + }, + }, + }, + { + name: "fail if ubuntu <= 11.04", + collectedContent: []collectedContent{ + { + NodeName: "node1", + Data: collect.HostOSInfo{ + Name: "myhost", + KernelVersion: "5.4.0-1034-gcp", + PlatformVersion: "11.04", + Platform: "ubuntu", + }, + }, + { + NodeName: "node2", + Data: collect.HostOSInfo{ + Name: "myhost", + KernelVersion: "5.4.0-1034-gcp", + PlatformVersion: "11.04", + Platform: "ubuntu", + }, + }, + }, + outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "ubuntu <= 11.04", + Message: "unsupported ubuntu version 11.04", + }, + }, + { + Pass: &troubleshootv1beta2.SingleOutcome{ + Message: "supported distribution", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + osInfo := data.(collect.HostOSInfo) + return osInfo.Platform == "ubuntu" && osInfo.PlatformVersion <= "11.04", nil + }, + expectResult: []*AnalyzeResult{ + { + Title: "Host OS Info - Node node1", + IsFail: true, + Message: "unsupported ubuntu version 11.04", + }, + { + Title: "Host OS Info - Node node2", + IsFail: true, + Message: "unsupported ubuntu version 11.04", + }, + }, + }, + { + name: "title does not include node name if empty", + collectedContent: []collectedContent{ + { + NodeName: "", + Data: collect.HostOSInfo{ + Name: "myhost", + KernelVersion: "5.4.0-1034-gcp", + PlatformVersion: "20.04", + Platform: "ubuntu", + }, + }, + }, + outcomes: []*troubleshootv1beta2.Outcome{ + { + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "ubuntu >= 20.04", + Message: "supported distribution matches ubuntu >= 20.04", + }, + }, + { + Fail: &troubleshootv1beta2.SingleOutcome{ + Message: "unsupported distribution", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + osInfo := data.(collect.HostOSInfo) + return osInfo.Platform == "ubuntu" && osInfo.PlatformVersion >= "20.04", nil + }, + expectResult: []*AnalyzeResult{ + { + Title: "Host OS Info", // Ensuring the title does not include node name if it's empty + IsPass: true, + Message: "supported distribution matches ubuntu >= 20.04", + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Call the new analyzeHostCollectorResults function with the test data + result, err := analyzeHostCollectorResults(test.collectedContent, test.outcomes, test.checkCondition, "Host OS Info") + require.NoError(t, err) + assert.Equal(t, test.expectResult, result) + }) + } +} + +func TestEvaluateOutcomes(t *testing.T) { + tests := []struct { + name string + outcomes []*troubleshootv1beta2.Outcome + checkCondition func(string, collectorData) (bool, error) + data collectorData + expectedResult []*AnalyzeResult + }{ + { + name: "fail condition matches", + outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "failCondition", + Message: "failure condition met", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + // Return true if the condition being checked matches "failCondition" + return when == "failCondition", nil + }, + data: "someData", + expectedResult: []*AnalyzeResult{ + { + Title: "Test Title", + IsFail: true, + Message: "failure condition met", + }, + }, + }, + { + name: "warn condition matches", + outcomes: []*troubleshootv1beta2.Outcome{ + { + Warn: &troubleshootv1beta2.SingleOutcome{ + When: "warnCondition", + Message: "warning condition met", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + // Return true if the condition being checked matches "warnCondition" + return when == "warnCondition", nil + }, + data: "someData", + expectedResult: []*AnalyzeResult{ + { + Title: "Test Title", + IsWarn: true, + Message: "warning condition met", + }, + }, + }, + { + name: "pass condition matches", + outcomes: []*troubleshootv1beta2.Outcome{ + { + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "passCondition", + Message: "pass condition met", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + // Return true if the condition being checked matches "passCondition" + return when == "passCondition", nil + }, + data: "someData", + expectedResult: []*AnalyzeResult{ + { + Title: "Test Title", + IsPass: true, + Message: "pass condition met", + }, + }, + }, + { + name: "no condition matches", + outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "failCondition", + Message: "failure condition met", + }, + Warn: &troubleshootv1beta2.SingleOutcome{ + When: "warnCondition", + Message: "warning condition met", + }, + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "passCondition", + Message: "pass condition met", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + // Always return false to simulate no condition matching + return false, nil + }, + data: "someData", + expectedResult: nil, // No condition matches, so we expect no results + }, + { + name: "error in checkCondition", + outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "failCondition", + Message: "failure condition met", + }, + }, + }, + checkCondition: func(when string, data collectorData) (bool, error) { + // Simulate an error occurring during condition evaluation + return false, errors.New("mock error") + }, + data: "someData", + expectedResult: []*AnalyzeResult{ + { + Title: "Test Title", + IsFail: false, // Error occurred, so no success flag + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := evaluateOutcomes(test.outcomes, test.checkCondition, test.data, "Test Title") + + if test.name == "error in checkCondition" { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, test.expectedResult, result) + } + }) + } +} diff --git a/pkg/analyze/host_memory.go b/pkg/analyze/host_memory.go index 8eb5f1566..92ed2f450 100644 --- a/pkg/analyze/host_memory.go +++ b/pkg/analyze/host_memory.go @@ -3,6 +3,7 @@ package analyzer import ( "encoding/json" "fmt" + "reflect" "strings" "github.com/pkg/errors" @@ -26,95 +27,40 @@ func (a *AnalyzeHostMemory) IsExcluded() (bool, error) { func (a *AnalyzeHostMemory) Analyze( getCollectedFileContents func(string) ([]byte, error), findFiles getChildCollectedFileContents, ) ([]*AnalyzeResult, error) { - hostAnalyzer := a.hostAnalyzer - - contents, err := getCollectedFileContents(collect.HostMemoryPath) + result := AnalyzeResult{Title: a.Title()} + + // Use the generic function to collect both local and remote data + collectedContents, err := retrieveCollectedContents( + getCollectedFileContents, + collect.HostMemoryPath, // Local path + collect.NodeInfoBaseDir, // Remote base directory + collect.HostMemoryFileName, // Remote file name + ) if err != nil { - return nil, errors.Wrap(err, "failed to get collected file") - } - - memoryInfo := collect.MemoryInfo{} - if err := json.Unmarshal(contents, &memoryInfo); err != nil { - return nil, errors.Wrap(err, "failed to unmarshal memory info") + return []*AnalyzeResult{&result}, err } - result := AnalyzeResult{ - Title: a.Title(), + results, err := analyzeHostCollectorResults(collectedContents, a.hostAnalyzer.Outcomes, a.CheckCondition, a.Title()) + if err != nil { + return nil, errors.Wrap(err, "failed to analyze OS version") } - 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 - } - - isMatch, err := compareHostMemoryConditionalToActual(outcome.Fail.When, memoryInfo.Total) - if err != nil { - return nil, errors.Wrapf(err, "failed to compare %s", outcome.Fail.When) - } - - if isMatch { - result.IsFail = true - result.Message = outcome.Fail.Message - result.URI = outcome.Fail.URI - - return []*AnalyzeResult{&result}, nil - } - } else if outcome.Warn != nil { - if outcome.Warn.When == "" { - result.IsWarn = true - result.Message = outcome.Warn.Message - result.URI = outcome.Warn.URI - - return []*AnalyzeResult{&result}, nil - } - - isMatch, err := compareHostMemoryConditionalToActual(outcome.Warn.When, memoryInfo.Total) - if err != nil { - return nil, errors.Wrapf(err, "failed to compare %s", outcome.Warn.When) - } - - if isMatch { - result.IsWarn = true - result.Message = outcome.Warn.Message - result.URI = outcome.Warn.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 - } - - isMatch, err := compareHostMemoryConditionalToActual(outcome.Pass.When, memoryInfo.Total) - if err != nil { - return nil, errors.Wrapf(err, "failed to compare %s", outcome.Pass.When) - } - - if isMatch { - result.IsPass = true - result.Message = outcome.Pass.Message - result.URI = outcome.Pass.URI + return results, nil +} - return []*AnalyzeResult{&result}, nil - } - } +// checkCondition checks the condition of the when clause +func (a *AnalyzeHostMemory) CheckCondition(when string, data collectorData) (bool, error) { + rawData, ok := data.([]byte) + if !ok { + return false, fmt.Errorf("expected data to be []uint8 (raw bytes), got: %v", reflect.TypeOf(data)) } - return []*AnalyzeResult{&result}, nil -} + var memInfo collect.MemoryInfo + if err := json.Unmarshal(rawData, &memInfo); err != nil { + return false, fmt.Errorf("failed to unmarshal data into MemoryInfo: %v", err) + } -func compareHostMemoryConditionalToActual(conditional string, total uint64) (res bool, err error) { - parts := strings.Split(conditional, " ") + parts := strings.Split(when, " ") if len(parts) != 2 { return false, fmt.Errorf("Expected 2 parts in conditional, got %d", len(parts)) } @@ -129,19 +75,23 @@ func compareHostMemoryConditionalToActual(conditional string, total uint64) (res if !ok { return false, fmt.Errorf("could not parse quantity %q", desired) } + if desiredInt < 0 { + return false, fmt.Errorf("desired value must be a positive integer, got %d", desiredInt) + } switch operator { case "<": - return total < uint64(desiredInt), nil + return memInfo.Total < uint64(desiredInt), nil case "<=": - return total <= uint64(desiredInt), nil + return memInfo.Total <= uint64(desiredInt), nil case ">": - return total > uint64(desiredInt), nil + return memInfo.Total > uint64(desiredInt), nil case ">=": - return total >= uint64(desiredInt), nil + return memInfo.Total >= uint64(desiredInt), nil case "=", "==", "===": - return total == uint64(desiredInt), nil + return memInfo.Total == uint64(desiredInt), nil + default: + return false, fmt.Errorf("unsupported operator: %q", operator) } - return false, errors.New("unknown operator") } diff --git a/pkg/analyze/host_memory_test.go b/pkg/analyze/host_memory_test.go index 363c4b738..3bcd09829 100644 --- a/pkg/analyze/host_memory_test.go +++ b/pkg/analyze/host_memory_test.go @@ -2,91 +2,110 @@ package analyzer import ( "encoding/json" + "fmt" "testing" + "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" "github.com/replicatedhq/troubleshoot/pkg/collect" + "github.com/replicatedhq/troubleshoot/pkg/constants" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func Test_doCompareHostMemory(t *testing.T) { +func TestAnalyzeHostMemoryCheckCondition(t *testing.T) { tests := []struct { name string conditional string actual uint64 expected bool + expectErr bool }{ { name: "< 16Gi when actual is 8Gi", conditional: "< 16Gi", - actual: 8 * 1024 * 1024 * 1024, + actual: 8 * 1024 * 1024 * 1024, // 8GiB expected: true, + expectErr: false, }, { name: "< 8Gi when actual is 8Gi", conditional: "< 8Gi", - actual: 8 * 1024 * 1024 * 1024, + actual: 8 * 1024 * 1024 * 1024, // 8GiB expected: false, + expectErr: false, }, { name: "<= 8Gi when actual is 8Gi", conditional: "<= 8Gi", - actual: 8 * 1024 * 1024 * 1024, + actual: 8 * 1024 * 1024 * 1024, // 8GiB expected: true, + expectErr: false, }, { name: "<= 8Gi when actual is 16Gi", conditional: "<= 8Gi", - actual: 16 * 1024 * 1024 * 1024, + actual: 16 * 1024 * 1024 * 1024, // 16GiB expected: false, + expectErr: false, }, { name: "== 8Gi when actual is 16Gi", conditional: "== 8Gi", - actual: 16 * 1024 * 1024 * 1024, + actual: 16 * 1024 * 1024 * 1024, // 16GiB expected: false, + expectErr: false, }, { name: "== 8Gi when actual is 8Gi", conditional: "== 8Gi", - actual: 8 * 1024 * 1024 * 1024, + actual: 8 * 1024 * 1024 * 1024, // 8GiB expected: true, + expectErr: false, }, { name: "== 8000000000 when actual is 8000000000", conditional: "== 8000000000", - actual: 8 * 1000 * 1000 * 1000, + actual: 8 * 1000 * 1000 * 1000, // 8GB in decimal expected: true, + expectErr: false, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - req := require.New(t) + // Create the AnalyzeHostMemory object + analyzeHostMemory := AnalyzeHostMemory{} - actual, err := compareHostMemoryConditionalToActual(test.conditional, test.actual) - req.NoError(err) - - assert.Equal(t, test.expected, actual) + // Simulate the memory info as JSON-encoded data + memInfo := collect.MemoryInfo{ + Total: test.actual, + } + rawData, err := json.Marshal(memInfo) + require.NoError(t, err) + // Call the CheckCondition method + result, err := analyzeHostMemory.CheckCondition(test.conditional, rawData) + if test.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, test.expected, result) + } }) } } func TestAnalyzeHostMemory(t *testing.T) { tests := []struct { - name string - memoryInfo *collect.MemoryInfo - hostAnalyzer *troubleshootv1beta2.MemoryAnalyze - result []*AnalyzeResult - expectErr bool + name string + hostAnalyzer *troubleshootv1beta2.MemoryAnalyze + getCollectedFileContents func(string) ([]byte, error) + expectedResults []*AnalyzeResult + expectedError string }{ { - name: "Pass on memory available", - memoryInfo: &collect.MemoryInfo{ - Total: 8 * 1024 * 1024 * 1024, - }, + name: "Pass on memory available (local)", hostAnalyzer: &troubleshootv1beta2.MemoryAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ { @@ -97,19 +116,27 @@ func TestAnalyzeHostMemory(t *testing.T) { }, }, }, - result: []*AnalyzeResult{ + getCollectedFileContents: func(path string) ([]byte, error) { + // Simulate local memory content retrieval + if path == collect.HostMemoryPath { + memoryInfo := collect.MemoryInfo{ + Total: 8 * 1024 * 1024 * 1024, // 8GiB + } + return json.Marshal(memoryInfo) + } + return nil, errors.New("file not found") + }, + expectedResults: []*AnalyzeResult{ { Title: "Amount of Memory", IsPass: true, Message: "System has at least 4Gi of memory", }, }, + expectedError: "", }, { - name: "Fail on memory available", - memoryInfo: &collect.MemoryInfo{ - Total: 8 * 1024 * 1024 * 1024, - }, + name: "Fail on memory available (remote node)", hostAnalyzer: &troubleshootv1beta2.MemoryAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ { @@ -120,27 +147,33 @@ func TestAnalyzeHostMemory(t *testing.T) { }, }, }, - result: []*AnalyzeResult{ + getCollectedFileContents: func(path string) ([]byte, error) { + // Simulate remote node list and memory content retrieval + if path == constants.NODE_LIST_FILE { + nodeNames := nodeNames{Nodes: []string{"node1"}} + return json.Marshal(nodeNames) + } + if path == fmt.Sprintf("%s/node1/%s", collect.NodeInfoBaseDir, collect.HostMemoryFileName) { + memoryInfo := collect.MemoryInfo{ + Total: 8 * 1024 * 1024 * 1024, // 8GiB for remote node + } + return json.Marshal(memoryInfo) + } + return nil, errors.New("file not found") + }, + expectedResults: []*AnalyzeResult{ { - Title: "Amount of Memory", + Title: "Amount of Memory - Node node1", IsFail: true, Message: "System requires at least 16Gi of memory", }, }, + expectedError: "", }, { - name: "Warn on memory available", - memoryInfo: &collect.MemoryInfo{ - Total: 8 * 1024 * 1024 * 1024, - }, + name: "Warn on memory available (remote node)", hostAnalyzer: &troubleshootv1beta2.MemoryAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ - { - Fail: &troubleshootv1beta2.SingleOutcome{ - When: "< 4Gi", - Message: "System requires at least 4Gi of memory", - }, - }, { Warn: &troubleshootv1beta2.SingleOutcome{ When: "<= 8Gi", @@ -149,97 +182,123 @@ func TestAnalyzeHostMemory(t *testing.T) { }, }, }, - result: []*AnalyzeResult{ + getCollectedFileContents: func(path string) ([]byte, error) { + // Simulate remote node list and memory content retrieval + if path == constants.NODE_LIST_FILE { + nodeNames := nodeNames{Nodes: []string{"node1"}} + return json.Marshal(nodeNames) + } + if path == fmt.Sprintf("%s/node1/%s", collect.NodeInfoBaseDir, collect.HostMemoryFileName) { + memoryInfo := collect.MemoryInfo{ + Total: 8 * 1024 * 1024 * 1024, // 8GiB for remote node + } + return json.Marshal(memoryInfo) + } + return nil, errors.New("file not found") + }, + expectedResults: []*AnalyzeResult{ { - Title: "Amount of Memory", + Title: "Amount of Memory - Node node1", IsWarn: true, Message: "System performs best with more than 8Gi of memory", }, }, + expectedError: "", }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - req := require.New(t) - b, err := json.Marshal(test.memoryInfo) - if err != nil { - t.Fatal(err) - } - - getCollectedFileContents := func(filename string) ([]byte, error) { - return b, nil - } - - result, err := (&AnalyzeHostMemory{test.hostAnalyzer}).Analyze(getCollectedFileContents, nil) - if test.expectErr { - req.Error(err) - } else { - req.NoError(err) - } - - assert.Equal(t, test.result, result) - }) - } -} - -func TestHostMemoryAnalyze(t *testing.T) { - tt := []struct { - name string - memoryInfo collect.MemoryInfo - outcomes []*troubleshootv1beta2.Outcome - results []*AnalyzeResult - wantErr bool - }{ { - name: "fix for empty pass predicate", - memoryInfo: collect.MemoryInfo{ - Total: 16 * 1024 * 1024 * 1024, - }, - outcomes: []*troubleshootv1beta2.Outcome{ - { - Fail: &troubleshootv1beta2.SingleOutcome{ - When: "< 8Gi", - Message: "oops", + name: "Pass on empty pass predicate (local)", + hostAnalyzer: &troubleshootv1beta2.MemoryAnalyze{ + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "< 8Gi", + Message: "System requires at least 8Gi of memory", + }, + }, + { + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "", + Message: "Memory is sufficient", + }, }, }, + }, + getCollectedFileContents: func(path string) ([]byte, error) { + // Simulate local memory content retrieval + if path == collect.HostMemoryPath { + memoryInfo := collect.MemoryInfo{ + Total: 16 * 1024 * 1024 * 1024, // 16GiB + } + return json.Marshal(memoryInfo) + } + return nil, errors.New("file not found") + }, + expectedResults: []*AnalyzeResult{ { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "", - Message: "it passed", + Title: "Amount of Memory", + IsPass: true, + Message: "Memory is sufficient", + }, + }, + expectedError: "", + }, + { + name: "Fix for empty pass predicate", + hostAnalyzer: &troubleshootv1beta2.MemoryAnalyze{ + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "< 8Gi", + Message: "oops", + }, + }, + { + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "", + Message: "it passed", + }, }, }, }, - results: []*AnalyzeResult{ + getCollectedFileContents: func(path string) ([]byte, error) { + // Simulate local memory content retrieval + if path == collect.HostMemoryPath { + memoryInfo := collect.MemoryInfo{ + Total: 16 * 1024 * 1024 * 1024, // 16GiB + } + return json.Marshal(memoryInfo) + } + return nil, errors.New("file not found") + }, + expectedResults: []*AnalyzeResult{ { + Title: "Amount of Memory", IsPass: true, Message: "it passed", - Title: "Memory Test", }, }, + expectedError: "", }, } - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - fn := func(_ string) ([]byte, error) { - return json.Marshal(&tc.memoryInfo) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Set up the AnalyzeHostMemory object + analyzeHostMemory := AnalyzeHostMemory{ + hostAnalyzer: test.hostAnalyzer, } - analyzer := AnalyzeHostMemory{ - hostAnalyzer: &troubleshootv1beta2.MemoryAnalyze{ - AnalyzeMeta: troubleshootv1beta2.AnalyzeMeta{ - CheckName: "Memory Test", - }, - Outcomes: tc.outcomes, - }, - } - results, err := analyzer.Analyze(fn, nil) - if tc.wantErr { - require.NotNil(t, err) - return + // Call the Analyze function + results, err := analyzeHostMemory.Analyze(test.getCollectedFileContents, nil) + + // Check for errors and compare results + if test.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), test.expectedError) + } else { + require.NoError(t, err) + assert.Equal(t, test.expectedResults, results) } - require.Nil(t, err) - require.Equal(t, tc.results, results) }) } } diff --git a/pkg/analyze/host_os_info.go b/pkg/analyze/host_os_info.go index d82a62e50..917a0c437 100644 --- a/pkg/analyze/host_os_info.go +++ b/pkg/analyze/host_os_info.go @@ -3,6 +3,7 @@ package analyzer import ( "encoding/json" "fmt" + "reflect" "regexp" "strings" @@ -10,18 +11,12 @@ import ( "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" "github.com/replicatedhq/troubleshoot/pkg/collect" - "github.com/replicatedhq/troubleshoot/pkg/constants" ) type AnalyzeHostOS struct { hostAnalyzer *troubleshootv1beta2.HostOSAnalyze } -type NodeOSInfo struct { - NodeName string - collect.HostOSInfo -} - func (a *AnalyzeHostOS) Title() string { return hostAnalyzerTitleOrDefault(a.hostAnalyzer.AnalyzeMeta, "Host OS Info") } @@ -33,69 +28,91 @@ func (a *AnalyzeHostOS) IsExcluded() (bool, error) { func (a *AnalyzeHostOS) Analyze( getCollectedFileContents func(string) ([]byte, error), findFiles getChildCollectedFileContents, ) ([]*AnalyzeResult, error) { - var nodesOSInfo []NodeOSInfo - result := AnalyzeResult{} - result.Title = a.Title() + result := AnalyzeResult{Title: a.Title()} + + // Use the generic function to collect both local and remote data + collectedContents, err := retrieveCollectedContents( + getCollectedFileContents, + collect.HostOSInfoPath, // Local path + collect.NodeInfoBaseDir, // Remote base directory + collect.HostInfoFileName, // Remote file name + ) + if err != nil { + return []*AnalyzeResult{&result}, err + } - // check if the host os info file exists (local mode) - contents, err := getCollectedFileContents(collect.HostOSInfoPath) + results, err := analyzeHostCollectorResults(collectedContents, a.hostAnalyzer.Outcomes, a.CheckCondition, a.Title()) if err != nil { - // check if the node list file exists (remote mode) - contents, err := getCollectedFileContents(constants.NODE_LIST_FILE) - if err != nil { - return []*AnalyzeResult{&result}, errors.Wrap(err, "failed to get collected file") - } + return nil, errors.Wrap(err, "failed to analyze OS version") + } - var nodes collect.HostOSInfoNodes - if err := json.Unmarshal(contents, &nodes); err != nil { - return []*AnalyzeResult{&result}, errors.Wrap(err, "failed to unmarshal host os info nodes") - } + return results, nil +} - // iterate over each node and analyze the host os info - for _, node := range nodes.Nodes { - contents, err := getCollectedFileContents(collect.NodeInfoBaseDir + "/" + node + "/" + collect.HostInfoFileName) - if err != nil { - return []*AnalyzeResult{&result}, errors.Wrap(err, "failed to get collected file") - } +// checkCondition checks the condition of the when clause +func (a *AnalyzeHostOS) CheckCondition(when string, data collectorData) (bool, error) { + rawData, ok := data.([]byte) + if !ok { + return false, fmt.Errorf("expected data to be []uint8 (raw bytes), got: %v", reflect.TypeOf(data)) + } - var osInfo collect.HostOSInfo - if err := json.Unmarshal(contents, &osInfo); err != nil { - return []*AnalyzeResult{&result}, errors.Wrap(err, "failed to unmarshal host os info") - } + var osInfo collect.HostOSInfo + if err := json.Unmarshal(rawData, &osInfo); err != nil { + return false, fmt.Errorf("failed to unmarshal data into HostOSInfo: %v", err) + } - nodesOSInfo = append(nodesOSInfo, NodeOSInfo{NodeName: node, HostOSInfo: osInfo}) - } + parts := strings.Split(when, " ") + if len(parts) < 3 { + return false, errors.New("when condition must have at least 3 parts") + } + expectedVer := fixVersion(parts[2]) + toleratedVer, err := semver.ParseTolerant(expectedVer) + if err != nil { + return false, errors.Wrapf(err, "failed to parse version: %s", expectedVer) + } + when = fmt.Sprintf("%s %v", parts[1], toleratedVer) + whenRange, err := semver.ParseRange(when) + if err != nil { + return false, errors.Wrapf(err, "failed to parse version range: %s", when) + } - results, err := analyzeOSVersionResult(nodesOSInfo, a.hostAnalyzer.Outcomes, a.Title()) + // Match the kernel version regardless of the platform + if parts[0] == "kernelVersion" { + fixedKernelVer := fixVersion(osInfo.KernelVersion) + toleratedKernelVer, err := semver.ParseTolerant(fixedKernelVer) if err != nil { - return []*AnalyzeResult{&result}, errors.Wrap(err, "failed to analyze os version result") + return false, errors.Wrapf(err, "failed to parse tolerant: %v", fixedKernelVer) + } + if whenRange(toleratedKernelVer) { + return true, nil } - return results, nil } - var osInfo collect.HostOSInfo - if err := json.Unmarshal(contents, &osInfo); err != nil { - return []*AnalyzeResult{&result}, errors.Wrap(err, "failed to unmarshal host os info") - } - nodesOSInfo = append(nodesOSInfo, NodeOSInfo{NodeName: "", HostOSInfo: osInfo}) - return analyzeOSVersionResult(nodesOSInfo, a.hostAnalyzer.Outcomes, a.Title()) -} - -func analyzeOSVersionResult(nodesOSInfo []NodeOSInfo, outcomes []*troubleshootv1beta2.Outcome, title string) ([]*AnalyzeResult, error) { - var results []*AnalyzeResult - for _, osInfo := range nodesOSInfo { - if title == "" { - title = "Host OS Info" + // Match platform version and kernel version, such as "centos-8.2-kernel == 8.2" + platform := parts[0] + kernelInfo := fmt.Sprintf("%s-%s-kernel", osInfo.Platform, osInfo.PlatformVersion) + if len(strings.Split(platform, "-")) == 3 && strings.Split(platform, "-")[2] == "kernel" { + if platform == kernelInfo { + fixedKernelVer := fixVersion(osInfo.KernelVersion) + toleratedKernelVer, err := semver.ParseTolerant(fixedKernelVer) + if err != nil { + return false, errors.Wrapf(err, "failed to parse tolerant: %v", fixedKernelVer) + } + if whenRange(toleratedKernelVer) { + return true, nil + } } - - analyzeResult, err := analyzeByOutcomes(outcomes, osInfo, title) + } else if platform == osInfo.Platform { + fixedDistVer := fixVersion(osInfo.PlatformVersion) + toleratedDistVer, err := semver.ParseTolerant(fixedDistVer) if err != nil { - return nil, errors.Wrap(err, "failed to analyze condition") + return false, errors.Wrapf(err, "failed to parse tolerant: %v", fixedDistVer) + } + if whenRange(toleratedDistVer) { + return true, nil } - results = append(results, analyzeResult...) } - - return results, nil + return false, nil } var rx = regexp.MustCompile(`^[0-9]+\.?[0-9]*\.?[0-9]*`) @@ -113,106 +130,3 @@ func fixVersion(versionStr string) string { version = strings.TrimRight(version, ".") return version } - -func analyzeByOutcomes(outcomes []*troubleshootv1beta2.Outcome, osInfo NodeOSInfo, title string) ([]*AnalyzeResult, error) { - var results []*AnalyzeResult - for _, outcome := range outcomes { - if osInfo.NodeName != "" { - title = fmt.Sprintf("%s - Node %s", title, osInfo.NodeName) - } - - result := AnalyzeResult{ - Title: title, - } - when := "" - message := "" - uri := "" - - if outcome.Fail != nil { - result.IsFail = true - when = outcome.Fail.When - message = outcome.Fail.Message - uri = outcome.Fail.URI - } else if outcome.Warn != nil { - result.IsWarn = true - when = outcome.Warn.When - message = outcome.Warn.Message - uri = outcome.Warn.URI - } else if outcome.Pass != nil { - result.IsPass = true - when = outcome.Pass.When - message = outcome.Pass.Message - uri = outcome.Pass.URI - } else { - return nil, errors.New("empty outcome") - } - - result.Message = message - result.URI = uri - // When is usually empty as the final case and should be treated as true - if when == "" { - results = append(results, &result) - return results, nil - } - - parts := strings.Split(when, " ") - if len(parts) < 3 { - return []*AnalyzeResult{&result}, errors.New("when condition must have at least 3 parts") - } - expectedVer := fixVersion(parts[2]) - toleratedVer, err := semver.ParseTolerant(expectedVer) - if err != nil { - return []*AnalyzeResult{&result}, errors.Wrapf(err, "failed to parse version: %s", expectedVer) - } - when = fmt.Sprintf("%s %v", parts[1], toleratedVer) - whenRange, err := semver.ParseRange(when) - if err != nil { - return []*AnalyzeResult{&result}, errors.Wrapf(err, "failed to parse version range: %s", when) - } - - // Match the kernel version regardless of the platform - // e.g "kernelVersion == 4.15" - if parts[0] == "kernelVersion" { - fixedKernelVer := fixVersion(osInfo.KernelVersion) - toleratedKernelVer, err := semver.ParseTolerant(fixedKernelVer) - if err != nil { - return []*AnalyzeResult{}, errors.Wrapf(err, "failed to parse tolerant: %v", fixedKernelVer) - } - if whenRange(toleratedKernelVer) { - results = append(results, &result) - return results, nil - } - } - - // Match the platform version and and kernel version passed in as - // "--kernel" e.g "centos-8.2-kernel == 8.2" - platform := parts[0] - kernelInfo := fmt.Sprintf("%s-%s-kernel", osInfo.Platform, osInfo.PlatformVersion) - if len(strings.Split(platform, "-")) == 3 && strings.Split(platform, "-")[2] == "kernel" { - if platform == kernelInfo { - fixedKernelVer := fixVersion(osInfo.KernelVersion) - toleratedKernelVer, err := semver.ParseTolerant(fixedKernelVer) - if err != nil { - return []*AnalyzeResult{&result}, errors.Wrapf(err, "failed to parse tolerant: %v", fixedKernelVer) - } - if whenRange(toleratedKernelVer) { - results = append(results, &result) - return results, nil - } - } - // Match the platform version - // e.g "centos == 8.2" - } else if platform == osInfo.Platform { - fixedDistVer := fixVersion(osInfo.PlatformVersion) - toleratedDistVer, err := semver.ParseTolerant(fixedDistVer) - if err != nil { - return []*AnalyzeResult{&result}, errors.Wrapf(err, "failed to parse tolerant: %v", fixedDistVer) - } - if whenRange(toleratedDistVer) { - results = append(results, &result) - return results, nil - } - } - } - return results, nil -} diff --git a/pkg/analyze/host_os_info_test.go b/pkg/analyze/host_os_info_test.go index 78511886a..00422df15 100644 --- a/pkg/analyze/host_os_info_test.go +++ b/pkg/analyze/host_os_info_test.go @@ -2,30 +2,106 @@ package analyzer import ( "encoding/json" + "fmt" "testing" + "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" "github.com/replicatedhq/troubleshoot/pkg/collect" + "github.com/replicatedhq/troubleshoot/pkg/constants" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestAnalyzeHostOS(t *testing.T) { +func TestAnalyzeHostOSCheckCondition(t *testing.T) { tests := []struct { - name string - hostInfo collect.HostOSInfo - hostAnalyzer *troubleshootv1beta2.HostOSAnalyze - result []*AnalyzeResult - expectErr bool + name string + conditional string + osInfo collect.HostOSInfo + expected bool + expectErr bool }{ { - name: "pass if ubuntu >= 0.1.2", - hostInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "5.4.0-1034-gcp", - PlatformVersion: "00.1.2", + name: "kernelVersion == 4.15 when actual is 4.15", + conditional: "kernelVersion == 4.15", + osInfo: collect.HostOSInfo{ + KernelVersion: "4.15.0", + }, + expected: true, + expectErr: false, + }, + { + name: "kernelVersion < 4.15 when actual is 4.16", + conditional: "kernelVersion < 4.15", + osInfo: collect.HostOSInfo{ + KernelVersion: "4.16.0", + }, + expected: false, + expectErr: false, + }, + { + name: "centos == 8.2 when actual is 8.2", + conditional: "centos == 8.2", + osInfo: collect.HostOSInfo{ + Platform: "centos", + PlatformVersion: "8.2", + }, + expected: true, + expectErr: false, + }, + { + name: "ubuntu == 20.04 when actual is 18.04", + conditional: "ubuntu == 20.04", + osInfo: collect.HostOSInfo{ Platform: "ubuntu", + PlatformVersion: "18.04", }, + expected: false, + expectErr: false, + }, + { + name: "invalid conditional format", + conditional: "invalid conditional", + osInfo: collect.HostOSInfo{ + Platform: "ubuntu", + PlatformVersion: "18.04", + }, + expected: false, + expectErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Create the AnalyzeHostOS object + analyzeHostOS := AnalyzeHostOS{} + + // Simulate the OS info as JSON-encoded data + rawData, err := json.Marshal(test.osInfo) + require.NoError(t, err) + + // Call the CheckCondition method + result, err := analyzeHostOS.CheckCondition(test.conditional, rawData) + if test.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, test.expected, result) + } + }) + } +} + +func TestAnalyzeHostOS(t *testing.T) { + tests := []struct { + name string + hostAnalyzer *troubleshootv1beta2.HostOSAnalyze + getCollectedFileContents func(string) ([]byte, error) + result []*AnalyzeResult + expectErr bool + }{ + { + name: "successfully retrieve local content and analyze", hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ { @@ -41,6 +117,17 @@ func TestAnalyzeHostOS(t *testing.T) { }, }, }, + getCollectedFileContents: func(path string) ([]byte, error) { + if path == collect.HostOSInfoPath { + return json.Marshal(collect.HostOSInfo{ + Name: "myhost", + KernelVersion: "5.4.0-1034-gcp", + PlatformVersion: "00.1.2", + Platform: "ubuntu", + }) + } + return nil, errors.New("file not found") + }, result: []*AnalyzeResult{ { Title: "Host OS Info", @@ -48,22 +135,16 @@ func TestAnalyzeHostOS(t *testing.T) { Message: "supported distribution matches ubuntu >= 00.1.2", }, }, + expectErr: false, }, - { - name: "pass if ubuntu >= 1.0.2", - hostInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "5.4.0-1034-gcp", - PlatformVersion: "1.0.2", - Platform: "ubuntu", - }, + name: "local content not found, retrieve and analyze remote content", hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ { Pass: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu >= 1.0.2", - Message: "supported distribution matches ubuntu >= 1.0.2", + When: "ubuntu == 18.04", + Message: "supported remote ubuntu 18.04", }, }, { @@ -73,90 +154,81 @@ func TestAnalyzeHostOS(t *testing.T) { }, }, }, + getCollectedFileContents: func(path string) ([]byte, error) { + if path == constants.NODE_LIST_FILE { + return json.Marshal(nodeNames{Nodes: []string{"node1"}}) + } + if path == fmt.Sprintf("%s/node1/%s", collect.NodeInfoBaseDir, collect.HostInfoFileName) { + return json.Marshal(collect.HostOSInfo{ + Name: "nodehost", + KernelVersion: "4.15.0-1034-aws", + PlatformVersion: "18.04", + Platform: "ubuntu", + }) + } + return nil, errors.New("file not found") + }, result: []*AnalyzeResult{ { - Title: "Host OS Info", + Title: "Host OS Info - Node node1", IsPass: true, - Message: "supported distribution matches ubuntu >= 1.0.2", + Message: "supported remote ubuntu 18.04", }, }, + expectErr: false, }, { - name: "pass if ubuntu >= 1.2.0", - hostInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "5.4.0-1034-gcp", - PlatformVersion: "1.2.0", - Platform: "ubuntu", - }, + name: "fail to retrieve both local and remote content", hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu >= 1.0.2", - Message: "supported distribution matches ubuntu >= 1.2.0", - }, - }, { Fail: &troubleshootv1beta2.SingleOutcome{ - Message: "unsupported distribution", + Message: "failed analysis", }, }, }, }, + getCollectedFileContents: func(path string) ([]byte, error) { + return nil, errors.New("file not found") + }, result: []*AnalyzeResult{ { - Title: "Host OS Info", - IsPass: true, - Message: "supported distribution matches ubuntu >= 1.2.0", + Title: "Host OS Info", }, }, + expectErr: true, }, { - name: "pass if ubuntu-1.2.0-kernel >= 1.2.0", - hostInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "1.2.0-1034-gcp", - PlatformVersion: "1.2.0", - Platform: "centos", - }, + name: "error during remote content analysis", hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "centos-1.2.0-kernel >= 1.2.0", - Message: "supported kernel matches centos-1.2.0-kernel >= 1.2.0", - }, - }, { Fail: &troubleshootv1beta2.SingleOutcome{ - Message: "unsupported distribution", + Message: "analysis failed", }, }, }, }, - result: []*AnalyzeResult{ - { - Title: "Host OS Info", - IsPass: true, - Message: "supported kernel matches centos-1.2.0-kernel >= 1.2.0", - }, - }, + getCollectedFileContents: func(path string) ([]byte, error) { + if path == constants.NODE_LIST_FILE { + return json.Marshal(nodeNames{Nodes: []string{"node1"}}) + } + if path == fmt.Sprintf("%s/node1/%s", collect.NodeInfoBaseDir, collect.HostInfoFileName) { + return nil, errors.New("file not found") + } + return nil, errors.New("file not found") + }, + result: nil, + expectErr: true, }, { - name: "pass if ubuntu-0.1.2-kernel >= 0.1.2", - hostInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "0.01.2-1034-gcp", - PlatformVersion: "8.2", - Platform: "centos", - }, + name: "pass if centos-1.2.0-kernel >= 1.2.0", hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ { Pass: &troubleshootv1beta2.SingleOutcome{ - When: "centos-8.2-kernel >= 0.01.2", - Message: "supported kernel matches centos-8.2-kernel >= 0.01.2", + When: "centos-1.2.0-kernel >= 1.2.0", + Message: "supported kernel matches centos-1.2.0-kernel >= 1.2.0", }, }, { @@ -166,307 +238,139 @@ func TestAnalyzeHostOS(t *testing.T) { }, }, }, + getCollectedFileContents: func(path string) ([]byte, error) { + if path == constants.NODE_LIST_FILE { + return json.Marshal(nodeNames{Nodes: []string{"node1"}}) + } + if path == fmt.Sprintf("%s/node1/%s", collect.NodeInfoBaseDir, collect.HostInfoFileName) { + return json.Marshal(collect.HostOSInfo{ + Name: "nodehost", + KernelVersion: "1.2.0-1034-aws", + PlatformVersion: "1.2.0", + Platform: "centos", + }) + } + return nil, errors.New("file not found") + }, result: []*AnalyzeResult{ { - Title: "Host OS Info", + Title: "Host OS Info - Node node1", IsPass: true, - Message: "supported kernel matches centos-8.2-kernel >= 0.01.2", + Message: "supported kernel matches centos-1.2.0-kernel >= 1.2.0", }, }, + expectErr: false, }, - { - name: "fail if ubuntu <= 11.04", - hostInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "5.4.0-1034-gcp", - PlatformVersion: "11.04", - Platform: "ubuntu", - }, + name: "warn if ubuntu <= 16.04", hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ { - Fail: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu <= 11.04", - Message: "unsupported ubuntu version 11.04", + Warn: &troubleshootv1beta2.SingleOutcome{ + When: "ubuntu <= 16.04", + Message: "System performs best with Ubuntu version higher than 16.04", }, }, { Pass: &troubleshootv1beta2.SingleOutcome{ - Message: "supported distribution", + When: "ubuntu > 16.04", + Message: "Ubuntu version is sufficient", }, }, }, }, - result: []*AnalyzeResult{ - { - Title: "Host OS Info", - IsFail: true, - Message: "unsupported ubuntu version 11.04", - }, - }, - }, - { - name: "fail if none of the kernel distribution versions match", - hostInfo: collect.HostOSInfo{ - Name: "my-host", - KernelVersion: "4.4", - PlatformVersion: "18.04", - Platform: "ubuntu", - }, - hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ - Outcomes: []*troubleshootv1beta2.Outcome{ - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "centos-18.04-kernel > 4.15", - Message: "supported distribution matches centos-18.04-kernel >= 4.15", - }, - }, - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu-18.04-kernel > 4.15", - Message: "supported distribution matches ubuntu-18.04-kernel >= 4.15", - }, - }, - { - Warn: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu-16.04-kernel == 4.15", - Message: "supported distribution matches ubuntu-16.04-kernel == 4.15 ", - }, - }, - { - Fail: &troubleshootv1beta2.SingleOutcome{ - Message: "None matched, centos-18.04-kernel >= 4.15,ubuntu-18.04-kernel >= 4.15, supported distribution", - }, - }, - }, + getCollectedFileContents: func(path string) ([]byte, error) { + if path == collect.HostOSInfoPath { + return json.Marshal(collect.HostOSInfo{ + Name: "myhost", + KernelVersion: "4.15.0-1234-gcp", + PlatformVersion: "16.04", + Platform: "ubuntu", + }) + } + return nil, errors.New("file not found") }, result: []*AnalyzeResult{ { Title: "Host OS Info", - IsFail: true, - Message: "None matched, centos-18.04-kernel >= 4.15,ubuntu-18.04-kernel >= 4.15, supported distribution", + IsWarn: true, + Message: "System performs best with Ubuntu version higher than 16.04", }, }, + expectErr: false, }, { - name: "test if centos kernel > 4.15", - hostInfo: collect.HostOSInfo{ - Name: "my-host", - KernelVersion: "4.15", - PlatformVersion: "18.04", - Platform: "centos", - }, + name: "analyze multiple nodes with different OS info", hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ Outcomes: []*troubleshootv1beta2.Outcome{ { Pass: &troubleshootv1beta2.SingleOutcome{ - When: "centos-18.04-kernel >= 4.15", - Message: "supported distribution matches centos-18.04-kernel >= 4.15", - }, - }, - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu-18.04-kernel > 4.15", - Message: "supported distribution matches ubuntu-18.04-kernel >= 4.15", - }, - }, - { - Warn: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu-16.04-kernel == 4.15", - Message: "supported distribution matches ubuntu-16.04-kernel == 4.15 ", + When: "ubuntu == 18.04", + Message: "supported ubuntu version", }, }, { Fail: &troubleshootv1beta2.SingleOutcome{ - Message: "None matched, centos-18.04-kernel >= 4.15,ubuntu-18.04-kernel >= 4.15, supported distribution", + Message: "unsupported ubuntu version", }, }, }, }, - result: []*AnalyzeResult{ - { - Title: "Host OS Info", - IsPass: true, - Message: "supported distribution matches centos-18.04-kernel >= 4.15", - }, - }, - }, - { - name: "test ubuntu 16 kernel >= 4.15-abc", - hostInfo: collect.HostOSInfo{ - Name: "my-host", - KernelVersion: "4.14-abc", - PlatformVersion: "16.04", - Platform: "ubuntu", - }, - hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ - Outcomes: []*troubleshootv1beta2.Outcome{ - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu-16.04-kernel >= 4.14", - Message: "supported distribution match 4.14", - }, - }, - }, + getCollectedFileContents: func(path string) ([]byte, error) { + if path == constants.NODE_LIST_FILE { + return json.Marshal(nodeNames{Nodes: []string{"node1", "node2"}}) + } + if path == fmt.Sprintf("%s/node1/%s", collect.NodeInfoBaseDir, collect.HostInfoFileName) { + return json.Marshal(collect.HostOSInfo{ + Name: "nodehost", + KernelVersion: "4.15.0-1034-aws", + PlatformVersion: "18.04", + Platform: "ubuntu", + }) + } + if path == fmt.Sprintf("%s/node2/%s", collect.NodeInfoBaseDir, collect.HostInfoFileName) { + return json.Marshal(collect.HostOSInfo{ + Name: "nodehost", + KernelVersion: "4.15.0-1034-aws", + PlatformVersion: "16.04", + Platform: "ubuntu", + }) + } + return nil, errors.New("file not found") }, - result: []*AnalyzeResult{ { - Title: "Host OS Info", + Title: "Host OS Info - Node node1", IsPass: true, - Message: "supported distribution match 4.14", - }, - }, - }, - - { - name: "test kernelVersion >= 6.4.9-abc", - hostInfo: collect.HostOSInfo{ - Name: "my-host", - KernelVersion: "6.5.0-1024-gcp", - PlatformVersion: "22.04", - Platform: "ubuntu", - }, - hostAnalyzer: &troubleshootv1beta2.HostOSAnalyze{ - Outcomes: []*troubleshootv1beta2.Outcome{ - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "kernelVersion >= 6.4.9-abc", - Message: "supported kernel version >= 6.4.9-abc", - }, - }, + Message: "supported ubuntu version", }, - }, - - result: []*AnalyzeResult{ { - Title: "Host OS Info", - IsPass: true, - Message: "supported kernel version >= 6.4.9-abc", + Title: "Host OS Info - Node node2", + IsFail: true, + Message: "unsupported ubuntu version", }, }, + expectErr: false, }, } + for _, test := range tests { t.Run(test.name, func(t *testing.T) { - req := require.New(t) - b, err := json.Marshal(test.hostInfo) - if err != nil { - t.Fatal(err) + // Set up the AnalyzeHostOS object with the custom hostAnalyzer per test + analyzeHostOS := AnalyzeHostOS{ + hostAnalyzer: test.hostAnalyzer, } - getCollectedFileContents := func(filename string) ([]byte, error) { - return b, nil - } + // Call the Analyze function + results, err := analyzeHostOS.Analyze(test.getCollectedFileContents, nil) - result, err := (&AnalyzeHostOS{test.hostAnalyzer}).Analyze(getCollectedFileContents, nil) + // Check for errors and compare results if test.expectErr { - req.Error(err) + require.Error(t, err) } else { - req.NoError(err) + require.NoError(t, err) + assert.Equal(t, test.result, results) } - - assert.Equal(t, test.result, result) - }) - } -} - -func TestAnalyzeOSVersionResult(t *testing.T) { - tests := []struct { - name string - outcomes []*troubleshootv1beta2.Outcome - nodeOSInfo []NodeOSInfo - expectResult []*AnalyzeResult - }{ - { - name: "pass if ubuntu >= 0.1.2", - nodeOSInfo: []NodeOSInfo{ - { - NodeName: "node1", - HostOSInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "5.4.0-1034-gcp", - PlatformVersion: "00.1.2", - Platform: "ubuntu", - }, - }, - }, - outcomes: []*troubleshootv1beta2.Outcome{ - { - Pass: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu >= 00.1.2", - Message: "supported distribution matches ubuntu >= 00.1.2", - }, - }, - { - Fail: &troubleshootv1beta2.SingleOutcome{ - Message: "unsupported distribution", - }, - }, - }, - expectResult: []*AnalyzeResult{ - { - Title: "Host OS Info - Node node1", - IsPass: true, - Message: "supported distribution matches ubuntu >= 00.1.2", - }, - }, - }, - { - name: "fail if ubuntu <= 11.04", - nodeOSInfo: []NodeOSInfo{ - { - NodeName: "node1", - HostOSInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "5.4.0-1034-gcp", - PlatformVersion: "11.04", - Platform: "ubuntu", - }, - }, - { - NodeName: "node2", - HostOSInfo: collect.HostOSInfo{ - Name: "myhost", - KernelVersion: "5.4.0-1034-gcp", - PlatformVersion: "11.04", - Platform: "ubuntu", - }, - }, - }, - outcomes: []*troubleshootv1beta2.Outcome{ - { - Fail: &troubleshootv1beta2.SingleOutcome{ - When: "ubuntu <= 11.04", - Message: "unsupported ubuntu version 11.04", - }, - }, - { - Pass: &troubleshootv1beta2.SingleOutcome{ - Message: "supported distribution", - }, - }, - }, - expectResult: []*AnalyzeResult{ - { - Title: "Host OS Info - Node node1", - IsFail: true, - Message: "unsupported ubuntu version 11.04", - }, - { - Title: "Host OS Info - Node node2", - IsFail: true, - Message: "unsupported ubuntu version 11.04", - }, - }, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - result, err := analyzeOSVersionResult(test.nodeOSInfo, test.outcomes, "Host OS Info") - require.NoError(t, err) - assert.Equal(t, test.expectResult, result) }) } } diff --git a/pkg/collect/host_memory.go b/pkg/collect/host_memory.go index f7e4ad281..88273c8c9 100644 --- a/pkg/collect/host_memory.go +++ b/pkg/collect/host_memory.go @@ -14,6 +14,7 @@ type MemoryInfo struct { } const HostMemoryPath = `host-collectors/system/memory.json` +const HostMemoryFileName = `memory.json` type CollectHostMemory struct { hostCollector *troubleshootv1beta2.Memory diff --git a/pkg/supportbundle/collect.go b/pkg/supportbundle/collect.go index 303b68bcc..ec8e680c2 100644 --- a/pkg/supportbundle/collect.go +++ b/pkg/supportbundle/collect.go @@ -23,21 +23,10 @@ import ( "k8s.io/client-go/kubernetes" ) -type FilteredCollector struct { - Spec troubleshootv1beta2.HostCollect - Collector collect.HostCollector -} - func runHostCollectors(ctx context.Context, hostCollectors []*troubleshootv1beta2.HostCollect, additionalRedactors *troubleshootv1beta2.Redactor, bundlePath string, opts SupportBundleCreateOpts) (collect.CollectorResult, error) { collectSpecs := append([]*troubleshootv1beta2.HostCollect{}, hostCollectors...) collectedData := make(map[string][]byte) - // Filter out excluded collectors - filteredCollectors, err := filterHostCollectors(ctx, collectSpecs, bundlePath, opts) - if err != nil { - return nil, err - } - if opts.RunHostCollectorsInPod { if err := checkRemoteCollectorRBAC(ctx, opts.KubernetesRestConfig, "Remote Host Collectors", opts.Namespace); err != nil { if rbacErr, ok := err.(*RBACPermissionError); ok { @@ -52,11 +41,11 @@ func runHostCollectors(ctx context.Context, hostCollectors []*troubleshootv1beta return nil, err } } - if err := collectRemoteHost(ctx, filteredCollectors, bundlePath, opts, collectedData); err != nil { + if err := collectRemoteHost(ctx, collectSpecs, bundlePath, opts, collectedData); err != nil { return nil, err } } else { - if err := collectHost(ctx, filteredCollectors, opts, collectedData); err != nil { + if err := collectHost(ctx, collectSpecs, bundlePath, opts, collectedData); err != nil { return nil, err } } @@ -219,27 +208,38 @@ func getAnalysisFile(analyzeResults []*analyze.AnalyzeResult) (io.Reader, error) } // collectRemoteHost runs remote host collectors sequentially -func collectRemoteHost(ctx context.Context, filteredCollectors []FilteredCollector, bundlePath string, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { +func collectRemoteHost(ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { opts.KubernetesRestConfig.QPS = constants.DEFAULT_CLIENT_QPS opts.KubernetesRestConfig.Burst = constants.DEFAULT_CLIENT_BURST opts.KubernetesRestConfig.UserAgent = fmt.Sprintf("%s/%s", constants.DEFAULT_CLIENT_USER_AGENT, version.Version()) // Run remote collectors sequentially - for _, c := range filteredCollectors { - collector := c.Collector - spec := c.Spec - - // Send progress event: starting the collector - opts.ProgressChan <- fmt.Sprintf("[%s] Running host collector...", collector.Title()) + for _, spec := range collectSpecs { + collector, ok := collect.GetHostCollector(spec, bundlePath) + if !ok { + opts.ProgressChan <- "Host collector not found" + continue + } // Start a span for tracing _, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title()) span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String())) + isExcluded, _ := collector.IsExcluded() + if isExcluded { + opts.ProgressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title()) + span.SetAttributes(attribute.Bool(constants.EXCLUDED, true)) + span.End() + continue + } + + // Send progress event: starting the collector + opts.ProgressChan <- fmt.Sprintf("[%s] Running host collector...", collector.Title()) + // Parameters for remote collection params := &collect.RemoteCollectParams{ ProgressChan: opts.ProgressChan, - HostCollector: &spec, + HostCollector: spec, BundlePath: bundlePath, ClientConfig: opts.KubernetesRestConfig, Image: "replicated/troubleshoot:latest", @@ -273,10 +273,14 @@ func collectRemoteHost(ctx context.Context, filteredCollectors []FilteredCollect } // collectHost runs host collectors sequentially -func collectHost(ctx context.Context, filteredCollectors []FilteredCollector, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { +func collectHost(ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string, opts SupportBundleCreateOpts, collectedData map[string][]byte) error { // Run local collectors sequentially - for _, c := range filteredCollectors { - collector := c.Collector + for _, spec := range collectSpecs { + collector, ok := collect.GetHostCollector(spec, bundlePath) + if !ok { + opts.ProgressChan <- "Host collector not found" + continue + } // Send progress event: starting the collector opts.ProgressChan <- fmt.Sprintf("[%s] Running host collector...", collector.Title()) @@ -285,6 +289,14 @@ func collectHost(ctx context.Context, filteredCollectors []FilteredCollector, op _, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title()) span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String())) + isExcluded, _ := collector.IsExcluded() + if isExcluded { + opts.ProgressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title()) + span.SetAttributes(attribute.Bool(constants.EXCLUDED, true)) + span.End() + continue + } + // Run local collector sequentially result, err := collector.Collect(opts.ProgressChan) if err != nil { @@ -324,34 +336,3 @@ func getGlobalRedactors(additionalRedactors *troubleshootv1beta2.Redactor) []*tr } return []*troubleshootv1beta2.Redact{} } - -// filterHostCollectors filters out excluded collectors and returns a list of collectors to run -func filterHostCollectors(ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string, opts SupportBundleCreateOpts) ([]FilteredCollector, error) { - var filteredCollectors []FilteredCollector - - for _, desiredCollector := range collectSpecs { - collector, ok := collect.GetHostCollector(desiredCollector, bundlePath) - if !ok { - opts.ProgressChan <- "Host collector not found" - continue - } - - _, span := otel.Tracer(constants.LIB_TRACER_NAME).Start(ctx, collector.Title()) - span.SetAttributes(attribute.String("type", reflect.TypeOf(collector).String())) - - isExcluded, _ := collector.IsExcluded() - if isExcluded { - opts.ProgressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title()) - span.SetAttributes(attribute.Bool(constants.EXCLUDED, true)) - span.End() - continue - } - - filteredCollectors = append(filteredCollectors, FilteredCollector{ - Spec: *desiredCollector, - Collector: collector, - }) - } - - return filteredCollectors, nil -} diff --git a/pkg/supportbundle/collect_test.go b/pkg/supportbundle/collect_test.go deleted file mode 100644 index 0dfda3e68..000000000 --- a/pkg/supportbundle/collect_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package supportbundle - -import ( - "context" - "testing" - - v1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/trace/noop" -) - -func Test_filterHostCollectors(t *testing.T) { - otel.SetTracerProvider(noop.NewTracerProvider()) - - testCases := []struct { - name string - collectSpecs []*v1beta2.HostCollect - bundlePath string - opts SupportBundleCreateOpts - expectedResult []FilteredCollector - expectedError error - }{ - { - name: "nil host collectors spec", - collectSpecs: []*v1beta2.HostCollect{}, - bundlePath: "/tmp", - opts: SupportBundleCreateOpts{ - ProgressChan: make(chan interface{}, 10), - }, - expectedResult: []FilteredCollector{}, - expectedError: nil, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - filtered, err := filterHostCollectors(context.TODO(), tc.collectSpecs, tc.bundlePath, tc.opts) - if err != tc.expectedError { - t.Fatalf("expected error %v, got %v", tc.expectedError, err) - } - if len(filtered) != len(tc.expectedResult) { - t.Fatalf("expected %d filtered collectors, got %d", len(tc.expectedResult), len(filtered)) - } - }) - } -}