Repository: replicatedhq/troubleshoot
Pull request #943 (2023-01-04 01:51:02)

feat(support-bundle): silent result if ceph or longhorn addon not enabled
## Description, Motivation and Context

- if `ceph` is not enabled as add-on, return empty analyzer result silently without returning error 
- if `longhorn` is not enabled as add-on, return empty analyzer result silently without returning error 
- do not save those empty results into `analysis.json`
- if  `--debug` is supplied, error will be logged

Fixes: #659

<!--- If it relates to an open issue, please link to the issue here.
e.g.
Fixes: #414
-->

## Checklist

- [x] New and existing tests pass locally with introduced changes.
- [ ] Tests for the changes have been added (for bug fixes / features)
- [x] The commit message(s) are informative and highlight any breaking changes
- [ ] Any documentation required has been added/updated. For changes to https://troubleshoot.sh/ create a PR [here](https://github.com/replicatedhq/troubleshoot.sh/pulls)

## Does this PR introduce a breaking change?
- [ ] Yes
- [x] No

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->


DexterYan: You can run `./bin/support-bundle spec.yaml` to verify the result

`spec.yaml`
```
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: collector-sample
spec:
  collectors:
    - ceph: {}
    - longhorn: {}

  analyzers:
    - clusterVersion:
        outcomes:
          - fail:
              when: "< 1.16.0"
              message: The Admin Console requires at least Kubernetes 1.16.0
          - pass:
              message: Your cluster meets the recommended and required versions of Kubernetes
    - cephStatus: {}
    - longhorn: {}
```

Patch:
diff --git a/cmd/troubleshoot/cli/root.go b/cmd/troubleshoot/cli/root.go
index 6c72e7ab..3e4f2a71 100644
--- a/cmd/troubleshoot/cli/root.go
+++ b/cmd/troubleshoot/cli/root.go
@@ -35,7 +35,7 @@ from a server that can be used to assist when troubleshooting a Kubernetes clust
 			if !v.GetBool("debug") {
 				klog.SetLogger(logr.Discard())
 			}
-			logger.SetQuiet(v.GetBool("quiet"))
+			logger.SetQuiet(!v.GetBool("debug"))
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			v := viper.GetViper()
diff --git a/go.mod b/go.mod
index af0e9a9b..409e9ae5 100644
--- a/go.mod
+++ b/go.mod
@@ -181,11 +181,11 @@ require (
 	go.opencensus.io v0.23.0 // indirect
 	go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
 	golang.org/x/crypto v0.1.0 // indirect
-	golang.org/x/net v0.3.1-0.20221206200815-1e63c2f08a10 // indirect
+	golang.org/x/net v0.5.0 // indirect
 	golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783 // indirect
-	golang.org/x/sys v0.3.0 // indirect
-	golang.org/x/term v0.3.0 // indirect
-	golang.org/x/text v0.5.0 // indirect
+	golang.org/x/sys v0.4.0 // indirect
+	golang.org/x/term v0.4.0 // indirect
+	golang.org/x/text v0.6.0 // indirect
 	golang.org/x/time v0.0.0-20220609170525-579cf78fd858 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	google.golang.org/api v0.102.0 // indirect
diff --git a/go.sum b/go.sum
index d753bdfe..4c64eb85 100644
--- a/go.sum
+++ b/go.sum
@@ -1201,8 +1201,8 @@ golang.org/x/net v0.0.0-20220412020605-290c469a71a5/go.mod h1:CfG3xpIq0wQ8r1q4Su
 golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
 golang.org/x/net v0.0.0-20220607020251-c690dde0001d/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
 golang.org/x/net v0.0.0-20220617184016-355a448f1bc9/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
-golang.org/x/net v0.3.1-0.20221206200815-1e63c2f08a10 h1:Frnccbp+ok2GkUS2tC84yAq/U9Vg+0sIO7aRL3T4Xnc=
-golang.org/x/net v0.3.1-0.20221206200815-1e63c2f08a10/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE=
+golang.org/x/net v0.5.0 h1:GyT4nK/YDHSqa1c4753ouYCDajOYKTja9Xb/OHtgvSw=
+golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -1359,12 +1359,12 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=
-golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18=
+golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
-golang.org/x/term v0.3.0 h1:qoo4akIqOcDME5bhc/NgxUdovd6BSS2uMsVjB56q1xI=
-golang.org/x/term v0.3.0/go.mod h1:q750SLmJuPmVoN1blW3UFBPREJfb1KmY3vwxfr+nFDA=
+golang.org/x/term v0.4.0 h1:O7UWfv5+A2qiuulQk30kVinPoMtoIPeVaKLEgLpVkvg=
+golang.org/x/term v0.4.0/go.mod h1:9P2UbLfCdcvo3p/nzKvsmas4TnlujnuoV9hGgYzW1lQ=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@@ -1374,8 +1374,8 @@ golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
-golang.org/x/text v0.5.0 h1:OLmvp0KP+FVG99Ct/qFiL/Fhk4zp4QQnZ7b2U+5piUM=
-golang.org/x/text v0.5.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
+golang.org/x/text v0.6.0 h1:3XmdazWV+ubf7QgHSTWeykHOci5oeekaGJBLkrkaw4k=
+golang.org/x/text v0.6.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
 golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go
index 82a76758..cb6e470a 100644
--- a/pkg/analyze/analyzer.go
+++ b/pkg/analyze/analyzer.go
@@ -441,7 +441,9 @@ func Analyze(analyzer *troubleshootv1beta2.Analyze, getFile getCollectedFileCont
 		if err != nil {
 			return nil, err
 		}
-		result.Strict = analyzer.CephStatus.Strict.BoolOrDefaultFalse()
+		if result != nil {
+			result.Strict = analyzer.CephStatus.Strict.BoolOrDefaultFalse()
+		}
 		return []*AnalyzeResult{result}, nil
 	}
 	if analyzer.Longhorn != nil {
diff --git a/pkg/analyze/ceph.go b/pkg/analyze/ceph.go
index 97852a1d..d055e4a5 100644
--- a/pkg/analyze/ceph.go
+++ b/pkg/analyze/ceph.go
@@ -9,6 +9,7 @@ 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/types"
 )
 
 type CephHealth string
@@ -102,7 +103,11 @@ type PgMap struct {
 func cephStatus(analyzer *troubleshootv1beta2.CephStatusAnalyze, getCollectedFileContents func(string) ([]byte, error)) (*AnalyzeResult, error) {
 	fileName := path.Join(collect.GetCephCollectorFilepath(analyzer.CollectorName, analyzer.Namespace), "status.json")
 	collected, err := getCollectedFileContents(fileName)
+
 	if err != nil {
+		if _, ok := err.(*types.NotFoundError); ok {
+			return nil, nil
+		}
 		return nil, errors.Wrap(err, "failed to read collected ceph status")
 	}
 
diff --git a/pkg/analyze/download.go b/pkg/analyze/download.go
index 2c0b112a..49594ac2 100644
--- a/pkg/analyze/download.go
+++ b/pkg/analyze/download.go
@@ -4,6 +4,7 @@ import (
 	"archive/tar"
 	"compress/gzip"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 
@@ -14,6 +15,7 @@ import (
 	"github.com/replicatedhq/troubleshoot/pkg/constants"
 	"github.com/replicatedhq/troubleshoot/pkg/docrewrite"
 	"github.com/replicatedhq/troubleshoot/pkg/logger"
+	"github.com/replicatedhq/troubleshoot/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
 )
 
@@ -295,7 +297,14 @@ func FindBundleRootDir(localBundlePath string) (string, error) {
 }
 
 func (f fileContentProvider) getFileContents(fileName string) ([]byte, error) {
-	return os.ReadFile(filepath.Join(f.rootDir, fileName))
+	contents, err := os.ReadFile(filepath.Join(f.rootDir, fileName))
+	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			return nil, &types.NotFoundError{Name: fileName}
+		}
+		return nil, err
+	}
+	return contents, nil
 }
 
 func excludeFilePaths(files, excludeFiles []string) []string {
diff --git a/pkg/collect/ceph.go b/pkg/collect/ceph.go
index bd463489..e2a4234d 100644
--- a/pkg/collect/ceph.go
+++ b/pkg/collect/ceph.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/pkg/errors"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/logger"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -137,15 +138,16 @@ func (c *CollectCeph) Collect(progressChan chan<- interface{}) (CollectorResult,
 	}
 
 	output := NewResult()
-	for _, command := range CephCommands {
-		err := cephCommandExec(ctx, progressChan, c, c.Collector, pod, command, output)
-		if err != nil {
-			pathPrefix := GetCephCollectorFilepath(c.Collector.CollectorName, c.Namespace)
-			dstFileName := path.Join(pathPrefix, fmt.Sprintf("%s.%s-error", command.ID, command.Format))
-			output.SaveResult(c.BundlePath, dstFileName, strings.NewReader(err.Error()))
+	if pod != nil {
+		for _, command := range CephCommands {
+			err := cephCommandExec(ctx, progressChan, c, c.Collector, pod, command, output)
+			if err != nil {
+				pathPrefix := GetCephCollectorFilepath(c.Collector.CollectorName, c.Namespace)
+				dstFileName := path.Join(pathPrefix, fmt.Sprintf("%s.%s-error", command.ID, command.Format))
+				output.SaveResult(c.BundlePath, dstFileName, strings.NewReader(err.Error()))
+			}
 		}
 	}
-
 	return output, nil
 }
 
@@ -196,6 +198,7 @@ func cephCommandExec(ctx context.Context, progressChan chan<- interface{}, c *Co
 
 func findRookCephToolsPod(ctx context.Context, c *CollectCeph, namespace string) (*corev1.Pod, error) {
 	client, err := kubernetes.NewForConfig(c.ClientConfig)
+
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create kubernetes client")
 	}
@@ -210,7 +213,9 @@ func findRookCephToolsPod(ctx context.Context, c *CollectCeph, namespace string)
 		return &pods[0], nil
 	}
 
-	return nil, errors.New("rook ceph tools pod not found")
+	logger.Printf("rook ceph tools pod not found")
+
+	return nil, nil
 }
 
 func GetCephCollectorFilepath(name, namespace string) string {
diff --git a/pkg/collect/longhorn.go b/pkg/collect/longhorn.go
index 1ea8abe2..ffc70134 100644
--- a/pkg/collect/longhorn.go
+++ b/pkg/collect/longhorn.go
@@ -5,6 +5,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"net/http"
 	"path/filepath"
 	"regexp"
 	"sync"
@@ -16,6 +17,7 @@ import (
 	longhornv1beta1 "github.com/replicatedhq/troubleshoot/pkg/longhorn/client/clientset/versioned/typed/longhorn/v1beta1"
 	longhorntypes "github.com/replicatedhq/troubleshoot/pkg/longhorn/types"
 	"gopkg.in/yaml.v2"
+	apiErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -65,6 +67,12 @@ func (c *CollectLonghorn) Collect(progressChan chan<- interface{}) (CollectorRes
 	// collect nodes.longhorn.io
 	nodes, err := client.Nodes(ns).List(ctx, metav1.ListOptions{})
 	if err != nil {
+		if apiErr, ok := err.(*apiErrors.StatusError); ok {
+			if apiErr.ErrStatus.Code == http.StatusNotFound {
+				logger.Printf("list nodes.longhorn.io not found")
+				return nil, nil
+			}
+		}
 		return nil, errors.Wrap(err, "list nodes.longhorn.io")
 	}
 	dir := GetLonghornNodesDirectory(ns)
diff --git a/pkg/types/types.go b/pkg/types/types.go
new file mode 100644
index 00000000..ac4542c0
--- /dev/null
+++ b/pkg/types/types.go
@@ -0,0 +1,9 @@
+package types
+
+type NotFoundError struct {
+	Name string
+}
+
+func (e *NotFoundError) Error() string {
+	return e.Name + ": not found"
+}


Test patch:
diff --git a/pkg/analyze/ceph_test.go b/pkg/analyze/ceph_test.go
index aa97d8bc..6ec31542 100644
--- a/pkg/analyze/ceph_test.go
+++ b/pkg/analyze/ceph_test.go
@@ -5,21 +5,25 @@ import (
 	"testing"
 
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+type getFile func(n string) ([]byte, error)
+
 func Test_cephStatus(t *testing.T) {
 	tests := []struct {
 		name           string
 		analyzer       troubleshootv1beta2.CephStatusAnalyze
-		expectResult   AnalyzeResult
+		expectResult   *AnalyzeResult
+		getFile        getFile
 		filePath, file string
 	}{
 		{
 			name:     "pass case",
 			analyzer: troubleshootv1beta2.CephStatusAnalyze{},
-			expectResult: AnalyzeResult{
+			expectResult: &AnalyzeResult{
 				IsPass:  true,
 				IsWarn:  false,
 				IsFail:  false,
@@ -51,7 +55,7 @@ func Test_cephStatus(t *testing.T) {
 		{
 			name:     "warn case",
 			analyzer: troubleshootv1beta2.CephStatusAnalyze{},
-			expectResult: AnalyzeResult{
+			expectResult: &AnalyzeResult{
 				IsPass:  false,
 				IsWarn:  true,
 				IsFail:  false,
@@ -84,7 +88,7 @@ func Test_cephStatus(t *testing.T) {
 		{
 			name:     "fail case",
 			analyzer: troubleshootv1beta2.CephStatusAnalyze{},
-			expectResult: AnalyzeResult{
+			expectResult: &AnalyzeResult{
 				IsPass:  false,
 				IsWarn:  false,
 				IsFail:  true,
@@ -120,7 +124,7 @@ func Test_cephStatus(t *testing.T) {
 				CollectorName: "custom-namespace",
 				Namespace:     "namespace",
 			},
-			expectResult: AnalyzeResult{
+			expectResult: &AnalyzeResult{
 				IsPass:  true,
 				IsWarn:  false,
 				IsFail:  false,
@@ -167,7 +171,7 @@ func Test_cephStatus(t *testing.T) {
 					},
 				},
 			},
-			expectResult: AnalyzeResult{
+			expectResult: &AnalyzeResult{
 				IsPass:  false,
 				IsWarn:  false,
 				IsFail:  true,
@@ -200,7 +204,7 @@ func Test_cephStatus(t *testing.T) {
 		{
 			name:     "warn case with missing osd/pg data",
 			analyzer: troubleshootv1beta2.CephStatusAnalyze{},
-			expectResult: AnalyzeResult{
+			expectResult: &AnalyzeResult{
 				IsPass:  false,
 				IsWarn:  true,
 				IsFail:  false,
@@ -221,7 +225,7 @@ func Test_cephStatus(t *testing.T) {
 		{
 			name:     "warn case with health status message and summary",
 			analyzer: troubleshootv1beta2.CephStatusAnalyze{},
-			expectResult: AnalyzeResult{
+			expectResult: &AnalyzeResult{
 				IsPass:  false,
 				IsWarn:  true,
 				IsFail:  false,
@@ -249,21 +253,30 @@ func Test_cephStatus(t *testing.T) {
 				}
 			}`,
 		},
+		{
+			name:         "pass case when get file returns not found error",
+			analyzer:     troubleshootv1beta2.CephStatusAnalyze{},
+			expectResult: nil,
+			getFile: func(n string) ([]byte, error) {
+				return nil, &types.NotFoundError{}
+			},
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			req := require.New(t)
-
-			getFile := func(n string) ([]byte, error) {
-				assert.Equal(t, n, test.filePath)
-				return []byte(test.file), nil
+			if test.getFile == nil {
+				test.getFile = func(n string) ([]byte, error) {
+					assert.Equal(t, n, test.filePath)
+					return []byte(test.file), nil
+				}
 			}
 
-			actual, err := cephStatus(&test.analyzer, getFile)
+			actual, err := cephStatus(&test.analyzer, test.getFile)
 			req.NoError(err)
 
-			assert.Equal(t, test.expectResult, *actual)
+			assert.Equal(t, test.expectResult, actual)
 		})
 	}
 }