Revert "Use px/agent_status_diagnostics script within px cli to detect missing kernel headers (#2065)" (#2081)

Summary: Revert "Use `px/agent_status_diagnostics` script within px cli
to detect missing kernel headers (#2065)"

This reverts commit 3c9c4bd. While
testing the latest cloud release, I noticed that the functionality added
in #2065 can cause `px deploy` to hang indefinitely at the "Wait for
healthcheck" step. The deploy will finish successfully, but it's a poor
user experience.

There seems to be a goroutine blocking issue that is dependent on
cluster size. On 1 and 2 node clusters that I tested #2065 on, the issue
doesn't surface. However, the issue reproduces reliably on the larger
clusters that have pixie deployed to. Let's revert and release a new cli
version once this is tracked down.

Relevant Issues: #2051

Type of change: /kind bugfix

Test Plan: N/A

Changelog Message: Reverted the recent advanced diagnostics added during
`px deploy` as in some cases it can cause that caused `px deploy` to
hang

Signed-off-by: Dom Del Nano <[email protected]>

Author: ddelnano

src/pixie_cli/pkg/cmd/collect_logs.go

@@ -27,7 +27,7 @@ import (
 	"github.com/spf13/viper"
 
 	"px.dev/pixie/src/pixie_cli/pkg/utils"
-	"px.dev/pixie/src/pixie_cli/pkg/vizier"
+	"px.dev/pixie/src/utils/shared/k8s"
 )
 
 func init() {
@@ -42,7 +42,7 @@ var CollectLogsCmd = &cobra.Command{
 		viper.BindPFlag("namespace", cmd.Flags().Lookup("namespace"))
 	},
 	Run: func(cmd *cobra.Command, args []string) {
-		c := vizier.NewLogCollector(mustCreateBundleReader(), viper.GetString("cloud_addr"))
+		c := k8s.NewLogCollector()
 		fName := fmt.Sprintf("pixie_logs_%s.zip", time.Now().Format("20060102150405"))
 		err := c.CollectPixieLogs(fName)
 		if err != nil {

src/pixie_cli/pkg/cmd/deploy.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"strings"
 	"time"
@@ -71,7 +72,6 @@ var BlockListedLabels = []string{
 }
 
 func init() {
-	DeployCmd.Flags().StringP("bundle", "b", "", "Path/URL to bundle file")
 	DeployCmd.Flags().StringP("extract_yaml", "e", "", "Directory to extract the Pixie yamls to")
 	DeployCmd.Flags().StringP("vizier_version", "v", "", "Pixie version to deploy")
 	DeployCmd.Flags().BoolP("check", "c", true, "Check whether the cluster can run Pixie")
@@ -106,7 +106,6 @@ var DeployCmd = &cobra.Command{
 	Use:   "deploy",
 	Short: "Deploys Pixie on the current K8s cluster",
 	PreRun: func(cmd *cobra.Command, args []string) {
-		viper.BindPFlag("bundle", cmd.Flags().Lookup("bundle"))
 		viper.BindPFlag("extract_yaml", cmd.Flags().Lookup("extract_yaml"))
 		viper.BindPFlag("vizier_version", cmd.Flags().Lookup("vizier_version"))
 		viper.BindPFlag("check", cmd.Flags().Lookup("check"))
@@ -605,6 +604,61 @@ func deploy(cloudConn *grpc.ClientConn, clientset *kubernetes.Clientset, vzClien
 	return clusterID
 }
 
+func runSimpleHealthCheckScript(cloudAddr string, clusterID uuid.UUID) error {
+	v, err := vizier.ConnectionToVizierByID(cloudAddr, clusterID)
+	br := mustCreateBundleReader()
+	if err != nil {
+		return err
+	}
+	execScript := br.MustGetScript(script.AgentStatusScript)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	resp, err := v.ExecuteScriptStream(ctx, execScript, nil)
+	if err != nil {
+		return err
+	}
+
+	// TODO(zasgar): Make this use the Null output. We can't right now
+	// because of fatal message on vizier failure.
+	errCh := make(chan error)
+	// Eat all responses.
+	go func() {
+		for {
+			select {
+			case <-ctx.Done():
+				if ctx.Err() != nil {
+					errCh <- ctx.Err()
+					return
+				}
+				errCh <- nil
+				return
+			case msg := <-resp:
+				if msg == nil {
+					errCh <- nil
+					return
+				}
+				if msg.Err != nil {
+					if msg.Err == io.EOF {
+						errCh <- nil
+						return
+					}
+					errCh <- msg.Err
+					return
+				}
+				if msg.Resp.Status != nil && msg.Resp.Status.Code != 0 {
+					errCh <- errors.New(msg.Resp.Status.Message)
+				}
+				// Eat messages.
+			}
+		}
+	}()
+
+	err = <-errCh
+	return err
+}
+
 func waitForHealthCheckTaskGenerator(cloudAddr string, clusterID uuid.UUID) func() error {
 	return func() error {
 		timeout := time.NewTimer(5 * time.Minute)
@@ -614,15 +668,10 @@ func waitForHealthCheckTaskGenerator(cloudAddr string, clusterID uuid.UUID) func
 			case <-timeout.C:
 				return errors.New("timeout waiting for healthcheck  (it is possible that Pixie stabilized after the healthcheck timeout. To check if Pixie successfully deployed, run `px debug pods`)")
 			default:
-				_, err := vizier.RunSimpleHealthCheckScript(mustCreateBundleReader(), cloudAddr, clusterID)
+				err := runSimpleHealthCheckScript(cloudAddr, clusterID)
 				if err == nil {
 					return nil
 				}
-				// The health check warning error indicates the cluster successfully deployed, but there are some warnings.
-				// Return the error to end the polling and show the warnings.
-				if _, ok := err.(*vizier.HealthCheckWarning); ok {
-					return err
-				}
 				time.Sleep(5 * time.Second)
 			}
 		}
@@ -642,17 +691,13 @@ func waitForHealthCheck(cloudAddr string, clusterID uuid.UUID, clientset *kubern
 	hc := utils.NewSerialTaskRunner(healthCheckJobs)
 	err := hc.RunAndMonitor()
 	if err != nil {
-		if _, ok := err.(*vizier.HealthCheckWarning); ok {
-			utils.WithError(err).Error("Pixie healthcheck detected the following warnings:")
-		} else {
-			_ = pxanalytics.Client().Enqueue(&analytics.Track{
-				UserId: pxconfig.Cfg().UniqueClientID,
-				Event:  "Deploy Healthcheck Failed",
-				Properties: analytics.NewProperties().
-					Set("err", err.Error()),
-			})
-			utils.WithError(err).Fatal("Failed Pixie healthcheck")
-		}
+		_ = pxanalytics.Client().Enqueue(&analytics.Track{
+			UserId: pxconfig.Cfg().UniqueClientID,
+			Event:  "Deploy Healthcheck Failed",
+			Properties: analytics.NewProperties().
+				Set("err", err.Error()),
+		})
+		utils.WithError(err).Fatal("Failed Pixie healthcheck")
 	}
 	_ = pxanalytics.Client().Enqueue(&analytics.Track{
 		UserId: pxconfig.Cfg().UniqueClientID,

src/pixie_cli/pkg/cmd/root.go

@@ -203,6 +203,7 @@ var RootCmd = &cobra.Command{
 
 // Name a variable to store a slice of commands that don't require cloudAddr
 var cmdsCloudAddrNotReqd = []*cobra.Command{
+	CollectLogsCmd,
 	VersionCmd,
 }
 
@@ -244,7 +245,7 @@ func checkAuthForCmd(c *cobra.Command) {
 			os.Exit(1)
 		}
 		switch c {
-		case CollectLogsCmd, DeployCmd, UpdateCmd, GetCmd, DeployKeyCmd, APIKeyCmd:
+		case DeployCmd, UpdateCmd, GetCmd, DeployKeyCmd, APIKeyCmd:
 			utils.Errorf("These commands are unsupported in Direct Vizier mode.")
 			os.Exit(1)
 		default:
@@ -253,7 +254,7 @@ func checkAuthForCmd(c *cobra.Command) {
 	}
 
 	switch c {
-	case CollectLogsCmd, DeployCmd, UpdateCmd, RunCmd, LiveCmd, GetCmd, ScriptCmd, DeployKeyCmd, APIKeyCmd:
+	case DeployCmd, UpdateCmd, RunCmd, LiveCmd, GetCmd, ScriptCmd, DeployKeyCmd, APIKeyCmd:
 		authenticated := auth.IsAuthenticated(viper.GetString("cloud_addr"))
 		if !authenticated {
 			utils.Errorf("Failed to authenticate. Please retry `px auth login`.")

src/pixie_cli/pkg/vizier/BUILD.bazel

@@ -25,7 +25,6 @@ go_library(
         "data_formatter.go",
         "errors.go",
         "lister.go",
-        "logs.go",
         "script.go",
         "stream_adapter.go",
         "utils.go",