Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log validateConfig, PrepareWebooks and doInstall errors (backport #775) #777

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jul 24, 2024

Problem:
Prior to this commit, errors returned from validateConfig(), PrepareWebooks() and doInstall() would be printed to the console, but not logged. This meant that if you weren't able to access the console for whatever reason, but could access the log file, you would have no idea what went wrong if one of those function calls failed.

Solution:
Call logrus.Error() in addition to printToPanel().

Bonus: call logrus.Info("Installation will proceed (harvester.install.skipchecks = true)") to make it explicit when preflight checks fail but are intentionally skipped.

Related Issue:
harvester/harvester#6214

Test plan:

For example, with this configuration:

install:
  mode: This ain't right

...we should see something like the following on the console:

image

...and something like this in the log:

image


This is an automatic backport of pull request #775 done by [Mergify](https://mergify.com).

Prior to this commit, errors returned from the above functions
would be printed to the console, but not logged.  This meant
that if you weren't able to access the console for whatever
reason, but _could_ access the log file, you would have no idea
what went wrong if one of those function calls failed.

Related issue: harvester/harvester#6214

Signed-off-by: Tim Serong <[email protected]>
(cherry picked from commit 5ffebed)
@bk201 bk201 merged commit ff64884 into v1.3 Jul 24, 2024
6 checks passed
@mergify mergify bot deleted the mergify/bp/v1.3/pr-775 branch July 24, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants