From 4450d4a85391f390c6bc099667bb3a57573e31d4 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Tue, 19 Sep 2023 17:10:12 +0100 Subject: [PATCH] Improved logging for both the cli and kotsadm install logs --- cmd/kots/cli/install.go | 54 +++++++++++++++++++---------------- pkg/handlers/preflight.go | 2 +- pkg/preflight/execute.go | 13 ++++++--- pkg/preflight/preflight.go | 35 +++++++++++++++++++++-- pkg/reporting/distribution.go | 7 ++++- 5 files changed, 77 insertions(+), 34 deletions(-) diff --git a/cmd/kots/cli/install.go b/cmd/kots/cli/install.go index 302099b4d0..b1a855d61f 100644 --- a/cmd/kots/cli/install.go +++ b/cmd/kots/cli/install.go @@ -429,6 +429,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: @@ -447,17 +448,22 @@ func InstallCmd() *cobra.Command { switch status { case storetypes.VersionPendingPreflight: log.ActionWithSpinner("Waiting for preflight checks to complete") + log.FinishSpinner() if err := ValidatePreflightStatus(deployOptions, authSlug, apiEndpoint); err != nil { - log.FinishSpinnerWithError() + log.Errorf("Preflight checks contain warnings or errors. The application was not deployed") perr := preflightError{} if errors.As(err, &perr) { cmd.SilenceErrors = true - log.Info(fmt.Sprintf("\n%s", err.Error())) + var s strings.Builder + s.WriteString("\nPreflight check results (state - title - message)") + for _, result := range perr.Results { + s.WriteString(fmt.Sprintf("\n%s - %q - %q", preflightState(result), result.Title, result.Message)) + } + log.Info(s.String()) os.Exit(1) } return err } - log.FinishSpinner() case storetypes.VersionPendingConfig: log.ActionWithoutSpinnerWarning("Additional app configuration is required. Please login to the Admin Console to continue", nil) } @@ -1015,14 +1021,14 @@ func checkPreflightsComplete(response *handlers.GetPreflightResultResponse) (boo } type preflightError struct { - Msg string `json:"msg"` + Results []*preflight.UploadPreflightResult } func (e preflightError) Error() string { - return e.Msg + return "preflight checks have warnings or errors" } -func (e preflightError) Unwrap() error { return fmt.Errorf(e.Msg) } +func (e preflightError) Unwrap() error { return fmt.Errorf("preflight checks have warnings or errors") } func checkPreflightResults(response *handlers.GetPreflightResultResponse) (bool, error) { if response.PreflightResult.Result == "" { @@ -1035,24 +1041,8 @@ func checkPreflightResults(response *handlers.GetPreflightResultResponse) (bool, return false, errors.Wrap(err, fmt.Sprintf("failed to unmarshal upload preflight results from response: %v", response.PreflightResult.Result)) } - status := func(r *preflight.UploadPreflightResult) string { - if r.IsFail { - return "FAIL" - } - if r.IsWarn { - return "WARN" - } - if r.IsPass { - return "PASS" - } - // We should never get here - return "UNKNOWN" - } - - var s strings.Builder var isWarn, isFail bool for _, result := range results.Results { - s.WriteString(fmt.Sprintf("%s - %q\n", status(result), result.Title)) if result.IsWarn { isWarn = true } @@ -1063,20 +1053,34 @@ func checkPreflightResults(response *handlers.GetPreflightResultResponse) (bool, if isWarn && isFail { return false, preflightError{ - Msg: fmt.Sprintf("There are preflight check failures and warnings for the application. The app was not deployed. %s", s.String()), + Results: results.Results, } } if isWarn { return false, preflightError{ - Msg: fmt.Sprintf("There are preflight check warnings for the application. The app was not deployed. %s", s.String()), + Results: results.Results, } } if isFail { return false, preflightError{ - Msg: fmt.Sprintf("There are preflight check failures for the application. The app was not deployed. %s", s.String()), + Results: results.Results, } } return true, nil } + +func preflightState(r *preflight.UploadPreflightResult) string { + if r.IsFail { + return "FAIL" + } + if r.IsWarn { + return "WARN" + } + if r.IsPass { + return "PASS" + } + // We should never get here + return "UNKNOWN" +} diff --git a/pkg/handlers/preflight.go b/pkg/handlers/preflight.go index fc9cd17c98..5138d6b8af 100644 --- a/pkg/handlers/preflight.go +++ b/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) diff --git a/pkg/preflight/execute.go b/pkg/preflight/execute.go index 0d1a9b9472..697a068475 100644 --- a/pkg/preflight/execute.go +++ b/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 diff --git a/pkg/preflight/preflight.go b/pkg/preflight/preflight.go index fddf9ea7df..ef0b8a83e5 100644 --- a/pkg/preflight/preflight.go +++ b/pkg/preflight/preflight.go @@ -160,13 +160,27 @@ 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" { + // TODO: Are there conditions when the application gets installed? + logger.Warnf("Preflight checks completed with warnings or errors. The application may not get installed") + for _, result := range uploadPreflightResults.Results { + if result == nil { + continue + } + logger.Infof("preflight state=%s title=%q message=%q", preflightState(*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 @@ -194,7 +208,7 @@ func Run(appID string, appSlug string, sequence int64, isAirgap bool, archiveDir // preflight reporting if isDeployed { if err := reporting.WaitAndReportPreflightChecks(appID, sequence, false, false); err != nil { - logger.Debugf("failed to send preflights data to replicated app: %v", err) + logger.Errorf("failed to send preflights data to replicated app: %v", err) return } } @@ -216,6 +230,21 @@ func Run(appID string, appSlug string, sequence int64, isAirgap bool, archiveDir return nil } +func preflightState(p troubleshootpreflight.UploadPreflightResult) string { + 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 { diff --git a/pkg/reporting/distribution.go b/pkg/reporting/distribution.go index eb91f41c25..8d2d8c6124 100644 --- a/pkg/reporting/distribution.go +++ b/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()) }