feat: [sc-113128] Create node list file before running remote host collector (#1632)

* create node list

Author: nvanthao

pkg/analyze/host_os_info.go

@@ -10,6 +10,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/constants"
 )
 
 type AnalyzeHostOS struct {
@@ -39,8 +40,8 @@ func (a *AnalyzeHostOS) Analyze(
 	// check if the host os info file exists (local mode)
 	contents, err := getCollectedFileContents(collect.HostOSInfoPath)
 	if err != nil {
-		//check if the node list file exists (remote mode)
-		contents, err := getCollectedFileContents(collect.NODE_LIST_FILE)
+		// 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")
 		}

pkg/collect/collect.go

@@ -12,8 +12,6 @@ import (
 	"k8s.io/client-go/rest"
 )
 
-const NODE_LIST_FILE = "host-collectors/system/node_list.json"
-
 var (
 	// ErrCollectorNotFound is returned when an undefined host collector is
 	// specified by the user.

pkg/collect/host_os_info.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"os"
 
 	"github.com/pkg/errors"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
@@ -95,7 +94,6 @@ func (c *CollectHostOS) RemoteCollect(progressChan chan<- interface{}) (map[stri
 	}
 
 	output := NewResult()
-	nodes := []string{}
 
 	// save the first result we find in the node and save it
 	for node, result := range results.AllCollectedData {
@@ -114,20 +112,9 @@ func (c *CollectHostOS) RemoteCollect(progressChan chan<- interface{}) (map[stri
 			if err != nil {
 				return nil, errors.Wrap(err, "failed to marshal host os info")
 			}
-			nodes = append(nodes, node)
 			output.SaveResult(c.BundlePath, fmt.Sprintf("host-collectors/system/%s/%s", node, HostInfoFileName), bytes.NewBuffer(b))
 		}
 	}
 
-	// check if NODE_LIST_FILE exists
-	_, err = os.Stat(NODE_LIST_FILE)
-	// if it not exists, save the nodes list
-	if err != nil {
-		nodesBytes, err := json.MarshalIndent(HostOSInfoNodes{Nodes: nodes}, "", " ")
-		if err != nil {
-			return nil, errors.Wrap(err, "failed to marshal host os info nodes")
-		}
-		output.SaveResult(c.BundlePath, NODE_LIST_FILE, bytes.NewBuffer(nodesBytes))
-	}
 	return output, nil
 }

pkg/constants/constants.go

@@ -109,4 +109,7 @@ const (
 	OUTCOME_PASS = "pass"
 	OUTCOME_WARN = "warn"
 	OUTCOME_FAIL = "fail"
+
+	// List of remote nodes to collect data from in a support bundle
+	NODE_LIST_FILE = "host-collectors/system/node_list.json"
 )

pkg/supportbundle/supportbundle.go

@@ -3,6 +3,7 @@ package supportbundle
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"os"
@@ -22,6 +23,8 @@ import (
 	"github.com/replicatedhq/troubleshoot/pkg/convert"
 	"github.com/replicatedhq/troubleshoot/pkg/version"
 	"go.opentelemetry.io/otel"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 )
@@ -46,6 +49,11 @@ type SupportBundleResponse struct {
 	FileUploaded    bool
 }
 
+// NodeList is a list of remote nodes to collect data from in a support bundle
+type NodeList struct {
+	Nodes []string `json:"nodes"`
+}
+
 // CollectSupportBundleFromSpec collects support bundle from start to finish, including running
 // collectors, analyzers and after collection steps. Input arguments are specifications.
 // if FromCLI option is set to true, the output is the name of the archive on disk in the cwd.
@@ -98,7 +106,7 @@ func CollectSupportBundleFromSpec(
 		return nil, errors.Wrap(err, "create bundle dir")
 	}
 
-	var result, files, hostFiles collect.CollectorResult
+	result := make(collect.CollectorResult)
 
 	ctx, root := otel.Tracer(constants.LIB_TRACER_NAME).Start(
 		context.Background(), constants.TROUBLESHOOT_ROOT_SPAN_NAME,
@@ -110,10 +118,32 @@ func CollectSupportBundleFromSpec(
 		root.End()
 	}()
 
+	// only create a node list if we are running host collectors in a pod
+	if opts.RunHostCollectorsInPod {
+		clientset, err := kubernetes.NewForConfig(opts.KubernetesRestConfig)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to create kubernetes clientset to run host collectors in pod")
+		}
+		nodeList, err := getNodeList(clientset, opts)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to get remote node list")
+		}
+		nodeListBytes, err := json.MarshalIndent(nodeList, "", "  ")
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to marshal remote node list")
+		}
+		err = result.SaveResult(bundlePath, constants.NODE_LIST_FILE, bytes.NewBuffer(nodeListBytes))
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to write remote node list")
+		}
+	}
+
 	// Cache error returned by collectors and return it at the end of the function
 	// so as to have a chance to run analyzers and archive the support bundle after.
 	// If both host and in cluster collectors fail, the errors will be wrapped
 	collectorsErrs := []string{}
+	var files, hostFiles collect.CollectorResult
+
 	if spec.HostCollectors != nil {
 		// Run host collectors
 		hostFiles, err = runHostCollectors(ctx, spec.HostCollectors, additionalRedactors, bundlePath, opts)
@@ -130,16 +160,16 @@ func CollectSupportBundleFromSpec(
 		}
 	}
 
-	if files != nil && hostFiles != nil {
-		result = files
-		for k, v := range hostFiles {
-			result[k] = v
-		}
-	} else if files != nil {
-		result = files
-	} else if hostFiles != nil {
-		result = hostFiles
-	} else {
+	// merge in-cluster and host collectors results
+	for k, v := range files {
+		result[k] = v
+	}
+
+	for k, v := range hostFiles {
+		result[k] = v
+	}
+
+	if len(result) == 0 {
 		if len(collectorsErrs) > 0 {
 			return nil, fmt.Errorf("failed to generate support bundle: %s", strings.Join(collectorsErrs, "\n"))
 		}
@@ -288,3 +318,18 @@ func ConcatSpec(target *troubleshootv1beta2.SupportBundle, source *troubleshootv
 	}
 	return newBundle
 }
+
+func getNodeList(clientset kubernetes.Interface, opts SupportBundleCreateOpts) (*NodeList, error) {
+	// todo: any node filtering on opts?
+	nodes, err := clientset.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{})
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list nodes")
+	}
+
+	nodeList := NodeList{}
+	for _, node := range nodes.Items {
+		nodeList.Nodes = append(nodeList.Nodes, node.Name)
+	}
+
+	return &nodeList, nil
+}

pkg/supportbundle/supportbundle_test.go

@@ -5,6 +5,10 @@ import (
 	"testing"
 
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	testclient "k8s.io/client-go/kubernetes/fake"
 )
 
 func Test_LoadAndConcatSpec(t *testing.T) {
@@ -70,3 +74,43 @@ func Test_LoadAndConcatSpec_WithNil(t *testing.T) {
 		t.Error("concatenating sourceBundle of nil pointer has error.")
 	}
 }
+
+func Test_getNodeList(t *testing.T) {
+	tests := []struct {
+		name        string
+		clientset   kubernetes.Interface
+		opts        SupportBundleCreateOpts
+		expected    *NodeList
+		expectError bool
+	}{
+		{
+			name: "successful node list",
+			clientset: testclient.NewSimpleClientset(
+				&corev1.NodeList{
+					Items: []corev1.Node{
+						{ObjectMeta: metav1.ObjectMeta{Name: "node1"}},
+						{ObjectMeta: metav1.ObjectMeta{Name: "node2"}},
+					},
+				},
+			),
+			opts: SupportBundleCreateOpts{},
+			expected: &NodeList{
+				Nodes: []string{"node1", "node2"},
+			},
+			expectError: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			nodeList, err := getNodeList(tt.clientset, tt.opts)
+			if (err != nil) != tt.expectError {
+				t.Errorf("getNodeList() error = %v, expectError %v", err, tt.expectError)
+				return
+			}
+			if !reflect.DeepEqual(nodeList, tt.expected) {
+				t.Errorf("getNodeList() = %v, expected %v", nodeList, tt.expected)
+			}
+		})
+	}
+}