Log install preflight errors/warnings in both the CLI and kotsadm pod

cmd/kots/cli/install.go

@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"mime/multipart"
 	"net/http"
 	"net/url"
@@ -33,6 +32,7 @@ import (
 	"github.com/replicatedhq/kots/pkg/kurl"
 	"github.com/replicatedhq/kots/pkg/logger"
 	"github.com/replicatedhq/kots/pkg/metrics"
+	"github.com/replicatedhq/kots/pkg/print"
 	"github.com/replicatedhq/kots/pkg/pull"
 	"github.com/replicatedhq/kots/pkg/replicatedapp"
 	"github.com/replicatedhq/kots/pkg/store/kotsstore"
@@ -47,6 +47,10 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
+var client = &http.Client{
+	Timeout: time.Second * 30,
+}
+
 func InstallCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:           "install [upstream uri]",
@@ -321,7 +325,7 @@ func InstallCmd() *cobra.Command {
 
 				log.ActionWithoutSpinner("Extracting airgap bundle")
 
-				airgapRootDir, err := ioutil.TempDir("", "kotsadm-airgap")
+				airgapRootDir, err := os.MkdirTemp("", "kotsadm-airgap")
 				if err != nil {
 					return errors.Wrap(err, "failed to create temp dir")
 				}
@@ -426,6 +430,7 @@ func InstallCmd() *cobra.Command {
 				case err := <-errChan:
 					if err != nil {
 						log.Error(err)
+						// TODO: Why is this a negative exit code?
 						os.Exit(-1)
 					}
 				case <-stopCh:
@@ -445,8 +450,16 @@ func InstallCmd() *cobra.Command {
 				case storetypes.VersionPendingPreflight:
 					log.ActionWithSpinner("Waiting for preflight checks to complete")
 					if err := ValidatePreflightStatus(deployOptions, authSlug, apiEndpoint); err != nil {
-						log.FinishSpinnerWithError()
-						return errors.Wrap(err, "failed to validate preflight results")
+						perr := preflightError{}
+						if errors.As(err, &perr) {
+							log.FinishSpinner() // We succeeded waiting for the results. Don't finish with an error
+							log.Errorf(perr.Msg)
+							print.PreflightErrors(log, perr.Results)
+							cmd.SilenceErrors = true // Stop Cobra from printing the error, we format the message ourselves
+						} else {
+							log.FinishSpinnerWithError()
+						}
+						return err
 					}
 					log.FinishSpinner()
 				case storetypes.VersionPendingConfig:
@@ -692,7 +705,7 @@ func getIngressConfig(v *viper.Viper) (*kotsv1beta1.IngressConfig, error) {
 
 	ingressConfig := kotsv1beta1.IngressConfig{}
 	if ingressConfigPath != "" {
-		content, err := ioutil.ReadFile(ingressConfigPath)
+		content, err := os.ReadFile(ingressConfigPath)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to read ingress service config file")
 		}
@@ -719,7 +732,7 @@ func getIdentityConfig(v *viper.Viper) (*kotsv1beta1.IdentityConfig, error) {
 
 	identityConfig := kotsv1beta1.IdentityConfig{}
 	if identityConfigPath != "" {
-		content, err := ioutil.ReadFile(identityConfigPath)
+		content, err := os.ReadFile(identityConfigPath)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to read identity service config file")
 		}
@@ -910,7 +923,7 @@ func getAutomatedInstallStatus(url string, authSlug string) (*kotsstore.TaskStat
 		return nil, errors.Errorf("unexpected status code %d", resp.StatusCode)
 	}
 
-	b, err := ioutil.ReadAll(resp.Body)
+	b, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to read response body")
 	}
@@ -964,7 +977,7 @@ func getPreflightResponse(url string, authSlug string) (*handlers.GetPreflightRe
 	newReq.Header.Add("Content-Type", "application/json")
 	newReq.Header.Add("Authorization", authSlug)
 
-	resp, err := http.DefaultClient.Do(newReq)
+	resp, err := client.Do(newReq)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to execute request")
 	}
@@ -974,7 +987,7 @@ func getPreflightResponse(url string, authSlug string) (*handlers.GetPreflightRe
 		return nil, errors.Errorf("unexpected status code %d", resp.StatusCode)
 	}
 
-	b, err := ioutil.ReadAll(resp.Body)
+	b, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to read response body")
 	}
@@ -1005,6 +1018,17 @@ func checkPreflightsComplete(response *handlers.GetPreflightResultResponse) (boo
 	return true, nil
 }
 
+type preflightError struct {
+	Msg     string
+	Results []*preflight.UploadPreflightResult
+}
+
+func (e preflightError) Error() string {
+	return e.Msg
+}
+
+func (e preflightError) Unwrap() error { return fmt.Errorf(e.Msg) }
+
 func checkPreflightResults(response *handlers.GetPreflightResultResponse) (bool, error) {
 	if response.PreflightResult.Result == "" {
 		return false, nil
@@ -1027,14 +1051,23 @@ func checkPreflightResults(response *handlers.GetPreflightResultResponse) (bool,
 	}
 
 	if isWarn && isFail {
-		return false, errors.New("There are preflight check failures and warnings for the application. The app was not deployed.")
+		return false, preflightError{
+			Msg:     "There are preflight check failures and warnings for the application. The app was not deployed.",
+			Results: results.Results,
+		}
 	}
 
 	if isWarn {
-		return false, errors.New("There are preflight check warnings for the application. The app was not deployed.")
+		return false, preflightError{
+			Msg:     "There are preflight check warnings for the application. The app was not deployed.",
+			Results: results.Results,
+		}
 	}
 	if isFail {
-		return false, errors.New("There are preflight check failures for the application. The app was not deployed.")
+		return false, preflightError{
+			Msg:     "There are preflight check failures for the application. The app was not deployed.",
+			Results: results.Results,
+		}
 	}
 
 	return true, nil

cmd/kots/cli/install_test.go

@@ -223,7 +223,7 @@ var _ = Describe("Install", func() {
 		})
 
 		It("returns an error if the preflight collection results cannot be parsed", func() {
-			invalidPreflightCollection, err := json.Marshal(handlers.GetPreflightResultResponse{
+			invalidPreflightCollection, _ := json.Marshal(handlers.GetPreflightResultResponse{
 				PreflightProgress: `{invalid: json}`,
 				PreflightResult:   preflighttypes.PreflightResult{},
 			})
@@ -239,13 +239,13 @@ var _ = Describe("Install", func() {
 				),
 			)
 
-			err = ValidatePreflightStatus(validDeployOptions, authSlug, server.URL())
+			err := ValidatePreflightStatus(validDeployOptions, authSlug, server.URL())
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("failed to unmarshal collect progress for preflights"))
 		})
 
 		It("returns an error if the upload preflight results are invalid", func() {
-			invalidUploadPreflightResponse, err := json.Marshal(handlers.GetPreflightResultResponse{
+			invalidUploadPreflightResponse, _ := json.Marshal(handlers.GetPreflightResultResponse{
 				PreflightProgress: "",
 				PreflightResult: preflighttypes.PreflightResult{
 					Result: "{invalid: json}",
@@ -262,7 +262,7 @@ var _ = Describe("Install", func() {
 				),
 			)
 
-			err = ValidatePreflightStatus(validDeployOptions, authSlug, server.URL())
+			err := ValidatePreflightStatus(validDeployOptions, authSlug, server.URL())
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("failed to unmarshal upload preflight results"))
 		})

cmd/kots/cli/set-config.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -127,7 +128,7 @@ func SetConfigCmd() *cobra.Command {
 			}
 			defer resp.Body.Close()
 
-			respBody, err := ioutil.ReadAll(resp.Body)
+			respBody, err := io.ReadAll(resp.Body)
 			if err != nil {
 				log.FinishSpinnerWithError()
 				return errors.Wrap(err, "failed to read server response")

pkg/handlers/preflight.go

@@ -201,7 +201,7 @@ func (h *Handler) StartPreflightChecks(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	archiveDir, err := ioutil.TempDir("", "kotsadm")
+	archiveDir, err := os.MkdirTemp("", "kotsadm")
 	if err != nil {
 		logger.Error(errors.Wrap(err, "failed to create temp dir"))
 		w.WriteHeader(http.StatusInternalServerError)

pkg/kotsutil/troubleshoot.go

@@ -2,7 +2,6 @@ package kotsutil
 
 import (
 	"context"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 
@@ -22,7 +21,7 @@ func LoadTSKindsFromPath(dir string) (*troubleshootloader.TroubleshootKinds, err
 				return nil
 			}
 
-			contents, err := ioutil.ReadFile(path)
+			contents, err := os.ReadFile(path)
 			if err != nil {
 				return errors.Wrap(err, "failed to read file")
 			}

pkg/kurl/kurl_nodes.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"math"
 	"net/http"
+	"os"
 	"strconv"
 	"time"
 
@@ -140,7 +141,7 @@ func getNodeMetrics(nodeIP string) (*statsv1alpha1.Summary, error) {
 	port := 10255
 
 	// only use mutual TLS if client cert exists
-	_, err := ioutil.ReadFile("/etc/kubernetes/pki/kubelet/client.crt")
+	_, err := os.ReadFile("/etc/kubernetes/pki/kubelet/client.crt")
 	if err == nil {
 		cert, err := tls.LoadX509KeyPair("/etc/kubernetes/pki/kubelet/client.crt", "/etc/kubernetes/pki/kubelet/client.key")
 		if err != nil {

pkg/preflight/execute.go

@@ -45,7 +45,7 @@ func setPreflightResult(appID string, sequence int64, preflightResults *types.Pr
 // execute will execute the preflights using spec in preflightSpec.
 // This spec should be rendered, no template functions remaining
 func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta2.Preflight, ignorePermissionErrors bool) (*types.PreflightResults, error) {
-	logger.Debug("executing preflight checks",
+	logger.Info("executing preflight checks",
 		zap.String("appID", appID),
 		zap.Int64("sequence", sequence))
 
@@ -65,7 +65,12 @@ func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta2.Pr
 			if err, ok := msg.(error); ok {
 				logger.Errorf("error while running preflights: %v", err)
 			} else {
-				logger.Infof("preflight progress: %v", msg)
+				switch m := msg.(type) {
+				case preflight.CollectProgress:
+					logger.Infof("preflight progress: %s", m.String())
+				default:
+					logger.Infof("preflight progress: %+v", msg)
+				}
 			}
 
 			progress, ok := msg.(preflight.CollectProgress)
@@ -121,7 +126,7 @@ func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta2.Pr
 		KubernetesRestConfig:   restConfig,
 	}
 
-	logger.Debug("preflight collect phase")
+	logger.Info("preflight collect phase")
 	collectResults, err := troubleshootpreflight.Collect(collectOpts, preflightSpec)
 	if err != nil && !isPermissionsError(err) {
 		preflightRunError = err
@@ -147,7 +152,7 @@ func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta2.Pr
 		}
 		uploadPreflightResults.Errors = rbacErrors
 	} else {
-		logger.Debug("preflight analyze phase")
+		logger.Info("preflight analyze phase")
 		analyzeResults := collectResults.Analyze()
 
 		// the typescript api added some flair to this result

pkg/preflight/preflight.go

@@ -160,13 +160,26 @@ func Run(appID string, appSlug string, sequence int64, isAirgap bool, archiveDir
 		preflight.Spec.Collectors = collectors
 
 		go func() {
-			logger.Debug("preflight checks beginning")
+			logger.Info("preflight checks beginning")
 			uploadPreflightResults, err := execute(appID, sequence, preflight, ignoreRBAC)
 			if err != nil {
 				logger.Error(errors.Wrap(err, "failed to run preflight checks"))
 				return
 			}
-			logger.Debug("preflight checks completed")
+
+			// Log the preflight results if there are any warnings or errors
+			// The app may not get installed so we need to see this info for debugging
+			if GetPreflightState(uploadPreflightResults) != "pass" {
+				logger.Warnf("Preflight checks completed with warnings or errors. The application will not get deployed")
+				for _, result := range uploadPreflightResults.Results {
+					if result == nil {
+						continue
+					}
+					logger.Infof("preflight state=%s title=%q message=%q", GetPreflightCheckState(result), result.Title, result.Message)
+				}
+			} else {
+				logger.Info("preflight checks completed")
+			}
 
 			go func() {
 				err := reporting.GetReporter().SubmitAppInfo(appID) // send app and preflight info when preflights finish
@@ -216,6 +229,26 @@ func Run(appID string, appSlug string, sequence int64, isAirgap bool, archiveDir
 	return nil
 }
 
+// GetPreflightCheckState returns the state of a single preflight check result
+func GetPreflightCheckState(p *troubleshootpreflight.UploadPreflightResult) string {
+	if p == nil {
+		return "unknown"
+	}
+
+	if p.IsFail {
+		return "fail"
+	}
+
+	if p.IsWarn {
+		return "warn"
+	}
+
+	if p.IsPass {
+		return "pass"
+	}
+	return "unknown"
+}
+
 // maybeDeployFirstVersion will deploy the first version if preflight checks pass
 func maybeDeployFirstVersion(appID string, sequence int64, preflightResults *types.PreflightResults) (bool, error) {
 	if sequence != 0 {
@@ -247,6 +280,10 @@ func maybeDeployFirstVersion(appID string, sequence int64, preflightResults *typ
 	return true, nil
 }
 
+// GetPreflightState returns a single state based on checking all
+// preflight checks results. If there are any errors, the state is fail.
+// If there are no errors and any warnings, the state is warn.
+// Otherwise, the state is pass.
 func GetPreflightState(preflightResults *types.PreflightResults) string {
 	if len(preflightResults.Errors) > 0 {
 		return "fail"

pkg/print/config.go

@@ -6,6 +6,8 @@ import (
 
 	configtypes "github.com/replicatedhq/kots/pkg/kotsadmconfig/types"
 	"github.com/replicatedhq/kots/pkg/logger"
+	"github.com/replicatedhq/kots/pkg/preflight"
+	tsPreflight "github.com/replicatedhq/troubleshoot/pkg/preflight"
 )
 
 func ConfigValidationErrors(log *logger.CLILogger, groupValidationErrors []configtypes.ConfigGroupValidationError) {
@@ -26,3 +28,16 @@ func ConfigValidationErrors(log *logger.CLILogger, groupValidationErrors []confi
 	log.FinishSpinnerWithError()
 	log.Errorf(sb.String())
 }
+
+func PreflightErrors(log *logger.CLILogger, results []*tsPreflight.UploadPreflightResult) {
+	w := NewTabWriter()
+	defer w.Flush()
+
+	fmt.Fprintf(w, "\n")
+	fmtColumns := "%s\t%s\t%s\n"
+	fmt.Fprintf(w, fmtColumns, "STATE", "TITLE", "MESSAGE")
+	for _, result := range results {
+		fmt.Fprintf(w, fmtColumns, strings.ToUpper(preflight.GetPreflightCheckState(result)), result.Title, result.Message)
+	}
+	fmt.Fprintf(w, "\n")
+}

pkg/reporting/distribution.go

@@ -53,7 +53,12 @@ func distributionFromServerGroupAndResources(clientset kubernetes.Interface) Dis
 
 func distributionFromProviderId(clientset kubernetes.Interface) Distribution {
 	nodes, err := clientset.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{})
-	logger.Infof("Found %d nodes", len(nodes.Items))
+	nodeCount := len(nodes.Items)
+	if nodeCount > 1 {
+		logger.Infof("Found %d nodes", nodeCount)
+	} else {
+		logger.Infof("Found %d node", nodeCount)
+	}
 	if err != nil {
 		logger.Infof("got error listing node: %v", err.Error())
 	}