diff --git a/config/crds/troubleshoot.sh_analyzers.yaml b/config/crds/troubleshoot.sh_analyzers.yaml index 8ae4aa0c..67518283 100644 --- a/config/crds/troubleshoot.sh_analyzers.yaml +++ b/config/crds/troubleshoot.sh_analyzers.yaml @@ -1360,6 +1360,8 @@ spec: - key type: object type: object + ignoreIfNoFiles: + type: boolean outcomes: items: properties: diff --git a/config/crds/troubleshoot.sh_preflights.yaml b/config/crds/troubleshoot.sh_preflights.yaml index 3181ffba..3ac13987 100644 --- a/config/crds/troubleshoot.sh_preflights.yaml +++ b/config/crds/troubleshoot.sh_preflights.yaml @@ -1360,6 +1360,8 @@ spec: - key type: object type: object + ignoreIfNoFiles: + type: boolean outcomes: items: properties: diff --git a/config/crds/troubleshoot.sh_supportbundles.yaml b/config/crds/troubleshoot.sh_supportbundles.yaml index cac0efc4..4f080d91 100644 --- a/config/crds/troubleshoot.sh_supportbundles.yaml +++ b/config/crds/troubleshoot.sh_supportbundles.yaml @@ -1391,6 +1391,8 @@ spec: - key type: object type: object + ignoreIfNoFiles: + type: boolean outcomes: items: properties: diff --git a/pkg/analyze/node_resources.go b/pkg/analyze/node_resources.go index 5e676a67..e549d2df 100644 --- a/pkg/analyze/node_resources.go +++ b/pkg/analyze/node_resources.go @@ -17,6 +17,7 @@ import ( troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" "github.com/replicatedhq/troubleshoot/pkg/constants" "github.com/replicatedhq/troubleshoot/pkg/k8sutil" + "github.com/replicatedhq/troubleshoot/pkg/types" ) type AnalyzeNodeResources struct { @@ -45,6 +46,9 @@ func (a *AnalyzeNodeResources) Analyze(getFile getCollectedFileContents, findFil if err != nil { return nil, err } + if result == nil { + return nil, nil + } result.Strict = a.analyzer.Strict.BoolOrDefaultFalse() return []*AnalyzeResult{result}, nil } @@ -53,7 +57,19 @@ func (a *AnalyzeNodeResources) analyzeNodeResources(analyzer *troubleshootv1beta collected, err := getCollectedFileContents(fmt.Sprintf("%s/%s.json", constants.CLUSTER_RESOURCES_DIR, constants.CLUSTER_RESOURCES_NODES)) if err != nil { - return nil, errors.Wrap(err, "failed to get contents of nodes.json") + if _, ok := err.(*types.NotFoundError); !ok { + return nil, errors.Wrap(err, "failed to get contents of nodes.json") + } + if analyzer.IgnoreIfNoFiles { + return nil, nil + } + return &AnalyzeResult{ + Title: a.Title(), + IconKey: "kubernetes_node_resources", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/node-resources.svg?w=16&h=18", + IsWarn: true, + Message: "No node resources were collected, unable to analyze node resources", + }, nil } var nodes corev1.NodeList diff --git a/pkg/analyze/node_resources_test.go b/pkg/analyze/node_resources_test.go index e4c87868..cae6c892 100644 --- a/pkg/analyze/node_resources_test.go +++ b/pkg/analyze/node_resources_test.go @@ -1,6 +1,7 @@ package analyzer import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -10,6 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/replicatedhq/troubleshoot/pkg/types" ) func Test_compareNodeResourceConditionalToActual(t *testing.T) { @@ -1684,3 +1686,56 @@ func Test_analyzeNodeResources(t *testing.T) { }) } } + +func Test_analyzeNodeResources_NoFiles(t *testing.T) { + missingFile := func(name string) ([]byte, error) { + return nil, &types.NotFoundError{Name: name} + } + + t.Run("emits warning when nodes.json is not collected", func(t *testing.T) { + req := require.New(t) + analyzer := &troubleshootv1beta2.NodeResources{ + Outcomes: []*troubleshootv1beta2.Outcome{ + {Pass: &troubleshootv1beta2.SingleOutcome{Message: "ok"}}, + }, + } + a := AnalyzeNodeResources{analyzer: analyzer} + got, err := a.Analyze(missingFile, nil) + req.NoError(err) + req.Len(got, 1) + req.True(got[0].IsWarn) + req.Equal("Node Resources", got[0].Title) + }) + + t.Run("ignoreIfNoFiles suppresses the warning", func(t *testing.T) { + req := require.New(t) + analyzer := &troubleshootv1beta2.NodeResources{ + IgnoreIfNoFiles: true, + Outcomes: []*troubleshootv1beta2.Outcome{ + {Pass: &troubleshootv1beta2.SingleOutcome{Message: "ok"}}, + }, + } + a := AnalyzeNodeResources{analyzer: analyzer} + got, err := a.Analyze(missingFile, nil) + req.NoError(err) + req.Empty(got) + }) + + t.Run("non-NotFound errors are propagated", func(t *testing.T) { + req := require.New(t) + analyzer := &troubleshootv1beta2.NodeResources{ + IgnoreIfNoFiles: true, + Outcomes: []*troubleshootv1beta2.Outcome{ + {Pass: &troubleshootv1beta2.SingleOutcome{Message: "ok"}}, + }, + } + ioErr := func(string) ([]byte, error) { + return nil, fmt.Errorf("permission denied") + } + a := AnalyzeNodeResources{analyzer: analyzer} + got, err := a.Analyze(ioErr, nil) + req.Error(err) + req.Nil(got) + req.Contains(err.Error(), "permission denied") + }) +} diff --git a/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go b/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go index ffaf87ff..3eac350b 100644 --- a/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/analyzer_shared.go @@ -130,9 +130,10 @@ type Distribution struct { } type NodeResources struct { - AnalyzeMeta `json:",inline" yaml:",inline"` - Outcomes []*Outcome `json:"outcomes" yaml:"outcomes"` - Filters *NodeResourceFilters `json:"filters,omitempty" yaml:"filters,omitempty"` + AnalyzeMeta `json:",inline" yaml:",inline"` + Outcomes []*Outcome `json:"outcomes" yaml:"outcomes"` + Filters *NodeResourceFilters `json:"filters,omitempty" yaml:"filters,omitempty"` + IgnoreIfNoFiles bool `json:"ignoreIfNoFiles,omitempty" yaml:"ignoreIfNoFiles,omitempty"` } type NodeResourceFilters struct { diff --git a/schemas/analyzer-troubleshoot-v1beta2.json b/schemas/analyzer-troubleshoot-v1beta2.json index c2c5c311..84f45436 100644 --- a/schemas/analyzer-troubleshoot-v1beta2.json +++ b/schemas/analyzer-troubleshoot-v1beta2.json @@ -1836,6 +1836,9 @@ "exclude": { "oneOf": [{"type": "string"},{"type": "boolean"}] }, + "ignoreIfNoFiles": { + "type": "boolean" + }, "filters": { "type": "object", "properties": { diff --git a/schemas/preflight-troubleshoot-v1beta2.json b/schemas/preflight-troubleshoot-v1beta2.json index 64f77629..8d284d33 100644 --- a/schemas/preflight-troubleshoot-v1beta2.json +++ b/schemas/preflight-troubleshoot-v1beta2.json @@ -1836,6 +1836,9 @@ "exclude": { "oneOf": [{"type": "string"},{"type": "boolean"}] }, + "ignoreIfNoFiles": { + "type": "boolean" + }, "filters": { "type": "object", "properties": { diff --git a/schemas/supportbundle-troubleshoot-v1beta2.json b/schemas/supportbundle-troubleshoot-v1beta2.json index e19155f9..fc078cbe 100644 --- a/schemas/supportbundle-troubleshoot-v1beta2.json +++ b/schemas/supportbundle-troubleshoot-v1beta2.json @@ -1882,6 +1882,9 @@ "exclude": { "oneOf": [{"type": "string"},{"type": "boolean"}] }, + "ignoreIfNoFiles": { + "type": "boolean" + }, "filters": { "type": "object", "properties": { diff --git a/test/e2e/support-bundle/node_resources_no_files_e2e_test.go b/test/e2e/support-bundle/node_resources_no_files_e2e_test.go new file mode 100644 index 00000000..6650b0b5 --- /dev/null +++ b/test/e2e/support-bundle/node_resources_no_files_e2e_test.go @@ -0,0 +1,101 @@ +package e2e + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/e2e-framework/pkg/envconf" + "sigs.k8s.io/e2e-framework/pkg/features" +) + +// TestNodeResourcesNoFiles verifies the behavior of the nodeResources +// analyzer when cluster-resources/nodes.json is not present in the +// bundle. The spec excludes the clusterResources collector so nodes.json +// is never written, then runs `support-bundle analyze` against the +// generated bundle. +// +// Expected outcomes: +// - "warn-default": warn outcome (default behavior) +// - "warn-explicit-false": warn outcome (ignoreIfNoFiles: false) +// - "ignored": no result (ignoreIfNoFiles: true) +func TestNodeResourcesNoFiles(t *testing.T) { + feature := features.New("Node Resources No Files"). + Assess("warns per analyzer when nodes.json is missing and respects ignoreIfNoFiles", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + supportBundleName := "node-resources-no-files-test" + tarPath := fmt.Sprintf("%s.tar.gz", supportBundleName) + specPath := "spec/nodeResourcesNoFiles.yaml" + + var collectOut bytes.Buffer + collectCmd := exec.CommandContext(ctx, sbBinary(), specPath, + "--interactive=false", + fmt.Sprintf("-o=%s", supportBundleName), + ) + collectCmd.Stdout = &collectOut + collectCmd.Stderr = &collectOut + require.NoErrorf(t, collectCmd.Run(), "support-bundle collect failed: %s", collectOut.String()) + + defer func() { + if err := os.Remove(tarPath); err != nil { + t.Logf("Error removing %s: %v", tarPath, err) + } + }() + + // Sanity check: nodes.json should NOT have been collected. + _, err := readFileFromTar(tarPath, fmt.Sprintf("%s/cluster-resources/nodes.json", supportBundleName)) + require.Error(t, err, "nodes.json should not exist in the bundle") + + var analyzeOut bytes.Buffer + var analyzeErr bytes.Buffer + analyzeCmd := exec.CommandContext(ctx, sbBinary(), "analyze", + "--bundle", tarPath, + "--output", "json", + specPath, + ) + analyzeCmd.Stdout = &analyzeOut + analyzeCmd.Stderr = &analyzeErr + require.NoErrorf(t, analyzeCmd.Run(), "support-bundle analyze failed: %s", analyzeErr.String()) + + type analyzeResult struct { + IsPass bool `json:"IsPass"` + IsFail bool `json:"IsFail"` + IsWarn bool `json:"IsWarn"` + Title string `json:"Title"` + Message string `json:"Message"` + } + var results []analyzeResult + require.NoError(t, json.Unmarshal(analyzeOut.Bytes(), &results), "analyzer JSON output: %s", analyzeOut.String()) + + byTitle := map[string]analyzeResult{} + for _, r := range results { + byTitle[r.Title] = r + } + + // Two warns, no result for the suppressed entry. + assert.Len(t, results, 2, "expected exactly two analyzer results, got %d: %s", len(results), analyzeOut.String()) + + for _, title := range []string{"warn-default", "warn-explicit-false"} { + r, ok := byTitle[title] + if !assert.Truef(t, ok, "expected an analyzer result with title %q", title) { + continue + } + assert.Truef(t, r.IsWarn, "%q: expected IsWarn=true", title) + assert.Falsef(t, r.IsFail, "%q: expected IsFail=false", title) + assert.Falsef(t, r.IsPass, "%q: expected IsPass=false", title) + assert.Containsf(t, r.Message, "No node resources were collected", "%q: unexpected message %q", title, r.Message) + } + + _, ignored := byTitle["ignored"] + assert.Falsef(t, ignored, "analyzer with ignoreIfNoFiles: true should have produced no result") + + return ctx + }).Feature() + + testenv.Test(t, feature) +} diff --git a/test/e2e/support-bundle/spec/nodeResourcesNoFiles.yaml b/test/e2e/support-bundle/spec/nodeResourcesNoFiles.yaml new file mode 100644 index 00000000..78fd431c --- /dev/null +++ b/test/e2e/support-bundle/spec/nodeResourcesNoFiles.yaml @@ -0,0 +1,37 @@ +apiVersion: troubleshoot.sh/v1beta2 +kind: SupportBundle +metadata: + name: node-resources-no-files-test +spec: + collectors: + - clusterResources: + exclude: true + - clusterInfo: + exclude: true + analyzers: + - nodeResources: + checkName: warn-default + outcomes: + - fail: + when: "count() < 3" + message: This application requires at least 3 nodes + - pass: + message: This cluster has enough nodes + - nodeResources: + checkName: warn-explicit-false + ignoreIfNoFiles: false + outcomes: + - fail: + when: "min(memoryCapacity) < 16Gi" + message: All nodes must have at least 16Gi of memory + - pass: + message: All nodes have at least 16Gi of memory + - nodeResources: + checkName: ignored + ignoreIfNoFiles: true + outcomes: + - fail: + when: "count() < 3" + message: This application requires at least 3 nodes + - pass: + message: This cluster has enough nodes