mirror of
https://github.com/replicatedhq/troubleshoot.git
synced 2026-08-27 00:37:20 +00:00
feat(analyze): warn when nodeResources has no node data, add ignoreIfNoFiles (#2035)
* feat(analyze): warn when nodeResources has no node data, add ignoreIfNoFiles The nodeResources analyzer previously failed silently when cluster-resources/nodes.json was not collected (e.g. when the clusterResources collector was excluded or could not list nodes). It now emits a warn outcome per nodeResources entry in the spec. Add an ignoreIfNoFiles top-level field to nodeResources, mirroring textAnalyze, so users can opt out of the new warning when the analyzer is intentionally optional. - Add IgnoreIfNoFiles to v1beta2.NodeResources - Update CRDs and JSON schemas - Unit test the warn / ignore paths - Add an e2e fixture and test that excludes clusterResources and asserts the analyze output Signed-off-by: Evans Mungai <evans@replicated.com> * Fix review comment Signed-off-by: Evans Mungai <evans@replicated.com> --------- Signed-off-by: Evans Mungai <evans@replicated.com>
This commit is contained in:
@@ -1360,6 +1360,8 @@ spec:
|
||||
- key
|
||||
type: object
|
||||
type: object
|
||||
ignoreIfNoFiles:
|
||||
type: boolean
|
||||
outcomes:
|
||||
items:
|
||||
properties:
|
||||
|
||||
@@ -1360,6 +1360,8 @@ spec:
|
||||
- key
|
||||
type: object
|
||||
type: object
|
||||
ignoreIfNoFiles:
|
||||
type: boolean
|
||||
outcomes:
|
||||
items:
|
||||
properties:
|
||||
|
||||
@@ -1391,6 +1391,8 @@ spec:
|
||||
- key
|
||||
type: object
|
||||
type: object
|
||||
ignoreIfNoFiles:
|
||||
type: boolean
|
||||
outcomes:
|
||||
items:
|
||||
properties:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -1836,6 +1836,9 @@
|
||||
"exclude": {
|
||||
"oneOf": [{"type": "string"},{"type": "boolean"}]
|
||||
},
|
||||
"ignoreIfNoFiles": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"filters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
@@ -1836,6 +1836,9 @@
|
||||
"exclude": {
|
||||
"oneOf": [{"type": "string"},{"type": "boolean"}]
|
||||
},
|
||||
"ignoreIfNoFiles": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"filters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
@@ -1882,6 +1882,9 @@
|
||||
"exclude": {
|
||||
"oneOf": [{"type": "string"},{"type": "boolean"}]
|
||||
},
|
||||
"ignoreIfNoFiles": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"filters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user