-
Notifications
You must be signed in to change notification settings - Fork 254
preflight: Don't put error message when check fails #4803
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
base: main
Are you sure you want to change the base?
Conversation
preflight checks have auto fix functionality (most of them) otherwise Nofix tag make sure it is not fixable automatic so error out. we shouldn't put logging.error for these checks because it shows as part of setup.
Reviewer's GuideThis PR adjusts the logging behavior in the preflight check implementation by downgrading failure log messages from error to debug level to prevent unnecessary error output during setup. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe logging level for errors encountered during the execution of the Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (17)
🔇 Additional comments (1)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @praveenkumar - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfergeau The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -62,7 +62,7 @@ func (check *Check) doCheck(config crcConfig.Storage) error { | |||
|
|||
err := check.check() | |||
if err != nil { | |||
logging.Error(err.Error()) | |||
logging.Debug(err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log level was changed to enhance clarity because of this issue: #4662. The user was complaining that the error message was not clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes all the preflight check, I think port check is non fix
so it should automatic go to error
side? can you check it once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crc setup --bundle ~/.crc/cache/crc_vfkit_4.18.2_arm64.crcbundle
WARN Using crc_vfkit_4.18.2_arm64.crcbundle bundle, but crc_vfkit_4.19.0_arm64.crcbundle is expected for this release
INFO Using bundle path /Users/gvyas/.crc/cache/crc_vfkit_4.18.2_arm64.crcbundle
INFO Checking if running macOS version >= 13.x
INFO Checking if running as non-root
INFO Checking if crc-admin-helper executable is cached
INFO Checking if running on a supported CPU architecture
INFO Checking if crc executable symlink exists
INFO Checking minimum RAM requirements
INFO Check if Podman binary exists in: /Users/gvyas/.crc/bin/oc
INFO Checking if running emulated on Apple silicon
INFO Checking if vfkit is installed
INFO Checking if CRC bundle is extracted in '$HOME/.crc'
INFO Checking if /Users/gvyas/.crc/cache/crc_vfkit_4.18.2_arm64.crcbundle exists
INFO Checking if old launchd config for tray and/or daemon exists
INFO Checking if crc daemon plist file is present and loaded
INFO Checking SSH port availability
ERRO port 2222 already in use: listen tcp 127.0.0.1:2222: bind: address already in use
ERRO crc requires port 2222 to run SSH
After the change ERRO port 2222 already in use: listen tcp 127.0.0.1:2222: bind: address already in use
gets hidden unless log-level is debug
gvyas@Gunjans-MacBook-Pro crc % crc setup --bundle ~/.crc/cache/crc_vfkit_4.18.2_arm64.crcbundle
WARN Using crc_vfkit_4.18.2_arm64.crcbundle bundle, but crc_vfkit_4.19.0_arm64.crcbundle is expected for this release
INFO Using bundle path /Users/gvyas/.crc/cache/crc_vfkit_4.18.2_arm64.crcbundle
INFO Checking if running macOS version >= 13.x
INFO Checking if running as non-root
INFO Checking if crc-admin-helper executable is cached
INFO Checking if running on a supported CPU architecture
INFO Checking if crc executable symlink exists
INFO Checking minimum RAM requirements
INFO Check if Podman binary exists in: /Users/gvyas/.crc/bin/oc
INFO Checking if running emulated on Apple silicon
INFO Checking if vfkit is installed
INFO Checking if CRC bundle is extracted in '$HOME/.crc'
INFO Checking if /Users/gvyas/.crc/cache/crc_vfkit_4.18.2_arm64.crcbundle exists
INFO Checking if old launchd config for tray and/or daemon exists
INFO Checking if crc daemon plist file is present and loaded
INFO Checking SSH port availability
ERRO crc requires port 2222 to run SSH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vyasgun in this case the error message for port is Port 2222 already in use which crc requires to run SSH
. I will update that also. For end user listen tcp 127.0.0.1:2222: bind: address already in use
doesn't make more sense than just letting them know port which we want to use.
@praveenkumar: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
preflight checks have auto fix functionality (most of them) otherwise Nofix tag make sure it is not fixable automatic so error out. we shouldn't put logging.error for these checks because it shows as part of setup.
Summary by Sourcery
Enhancements:
Summary by CodeRabbit