feat(cosmovisor): Add prepare-upgrade cmd #21972
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce new commands and enhancements to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
looks good, sorry misread the pr. didnt mean to approve |
lucaslopezf
requested review from
julienrbrt,
facundomedica and
a team
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
tools/cosmovisor/cmd/cosmovisor/root.go (1)
23-23: LGTM! Consider grouping related commands.The addition of
NewPrepareUpgradeCmd()aligns well with the PR objectives and follows the existing naming convention. Good job!For improved readability and maintenance, consider grouping related commands together. For instance, you could place
NewPrepareUpgradeCmd()next to other upgrade-related commands likeNewAddUpgradeCmd()andNewShowUpgradeInfoCmd().NewVersionCmd(), NewAddUpgradeCmd(), NewShowUpgradeInfoCmd(), - NewPrepareUpgradeCmd(), + NewPrepareUpgradeCmd(),tools/cosmovisor/CHANGELOG.md (1)
39-42: LGTM! Consider grouping related features.The new entries in the "Unreleased" section are well-formatted and provide clear information about the changes. Great job!
Consider grouping the two "Features" sections together for better readability. You can move the "Improvements" section between the two "Features" entries up, so all features are listed consecutively.
tools/cosmovisor/README.md (1)
21-21: Adjust indentation in table of contentsThe new entry "Preparing for an Upgrade" should be indented to match the level of other entries in this section.
Please update the indentation as follows:
- * [Preparing for an Upgrade](#preparing-for-an-upgrade) + * [Preparing for an Upgrade](#preparing-for-an-upgrade)🧰 Tools
🪛 Markdownlint
21-21: Expected: 8; Actual: 4
Unordered list indentation(MD007, ul-indent)
tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go (1)
17-19: Consider improving the formatting of the long descriptionThe
Longdescription contains a multiline string. For better readability and to adhere to Go formatting conventions, consider adjusting the string formatting to maintain consistent indentation and line breaks.Apply this diff to adjust the formatting:
Long: `Prepare for the next upgrade by downloading and verifying the upgrade binary. This command will download the binary specified in the upgrade-info.json file, verify its checksum, and place it in the appropriate directory for Cosmovisor to use.`, +Long: `Prepare for the next upgrade by downloading and verifying the upgrade binary. +This command will download the binary specified in the upgrade-info.json file, +verify its checksum, and place it in the appropriate directory for Cosmovisor to use.`,
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (4)
- tools/cosmovisor/CHANGELOG.md (2 hunks)
- tools/cosmovisor/README.md (2 hunks)
- tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go (1 hunks)
- tools/cosmovisor/cmd/cosmovisor/root.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
tools/cosmovisor/CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"tools/cosmovisor/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tools/cosmovisor/cmd/cosmovisor/root.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 Markdownlint
tools/cosmovisor/README.md
21-21: Expected: 8; Actual: 4
Unordered list indentation(MD007, ul-indent)
🔇 Additional comments (3)
tools/cosmovisor/README.md (1)
267-295: LGTM: New section on preparing for an upgradeThe new section "Preparing for an Upgrade" is well-written and provides clear instructions on using the
prepare-upgradecommand. The example output is helpful for users to understand what to expect when running the command.tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go (2)
13-25: Command initialization follows best practicesThe
NewPrepareUpgradeCmdfunction correctly sets up the newprepare-upgradecommand using the Cobra library. It includes appropriate usage (Use), short and long descriptions (Short,Long), assigns the correct handler (RunE), and specifies that no arguments are expected (Args).
27-66: FunctionprepareUpgradeHandleris well-structured and handles errors appropriatelyThe
prepareUpgradeHandlerfunction effectively manages the preparation steps for the upgrade, including configuration loading, upgrade information retrieval, parsing, and downloading the upgrade binary. Error handling is consistent and follows idiomatic Go practices, with errors wrapped using%wfor better context.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
tools/cosmovisor/args.go (1)
Line range hint
1-590: Update DetailString method to include GRPCAddressThe
DetailStringmethod should be updated to include the newGRPCAddressfield in its output. This will ensure that all configuration details are properly displayed when requested.Consider adding the following line to the
configEntriesslice in theDetailStringmethod:{EnvGRPCAddress, cfg.GRPCAddress},tools/cosmovisor/README.md (1)
267-298: Enhance the "Preparing for an Upgrade" section.The new section provides valuable information about the
prepare-upgradecommand. Consider the following improvements:
- Adjust the indentation of list items to be consistent with the rest of the document (8 spaces instead of 4).
- Add syntax highlighting to the example output for better readability.
- Make the note about manual downloads more prominent, perhaps by using a blockquote or admonition syntax.
Here's an example of how you could apply these changes:
### Preparing for an Upgrade To prepare for an upgrade, use the `prepare-upgrade` command: ```shell cosmovisor prepare-upgradeThis command performs the following actions:
-1. Retrieves upgrade information directly from the blockchain about the next scheduled upgrade.
-2. Downloads the new binary specified in the upgrade plan.
-3. Verifies the binary's checksum (if required by configuration).
-4. Places the new binary in the appropriate directory for Cosmovisor to use during the upgrade.
1. Retrieves upgrade information directly from the blockchain about the next scheduled upgrade.2. Downloads the new binary specified in the upgrade plan.3. Verifies the binary's checksum (if required by configuration).4. Places the new binary in the appropriate directory for Cosmovisor to use during the upgrade.The
prepare-upgradecommand provides detailed logging throughout the process, including:-* The name and height of the upcoming upgrade
-* The URL from which the new binary is being downloaded
-* Confirmation of successful download and verification
-* The path where the new binary has been placed
* The name and height of the upcoming upgrade* The URL from which the new binary is being downloaded* Confirmation of successful download and verification* The path where the new binary has been placedExample output:
-
bash +log
INFO Preparing for upgrade name=v1.0.0 height=1000000
INFO Downloading upgrade binary url=https://example.com/binary/v1.0.0?checksum=sha256:339911508de5e20b573ce902c500ee670589073485216bee8b045e853f24bce8
INFO Upgrade preparation complete name=v1.0.0 height=1000000-*Note: The current way of downloading manually and placing the binary at the right place would still work.* +> **Note:** The current way of downloading manually and placing the binary at the right place would still work.🧰 Tools
🪛 LanguageTool
[uncategorized] ~287-~287: Possible missing preposition found.
Context: ... successful download and verification * The path where the new binary has been plac...(AI_HYDRA_LEO_MISSING_OF)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (4)
- tools/cosmovisor/README.md (2 hunks)
- tools/cosmovisor/args.go (3 hunks)
- tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go (1 hunks)
- tools/cosmovisor/go.mod (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/cosmovisor/cmd/cosmovisor/prepare_upgrade.go
🧰 Additional context used
📓 Path-based instructions (2)
tools/cosmovisor/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"tools/cosmovisor/args.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 Markdownlint
tools/cosmovisor/README.md
21-21: Expected: 8; Actual: 4
Unordered list indentation(MD007, ul-indent)
🪛 LanguageTool
tools/cosmovisor/README.md
[uncategorized] ~287-~287: Possible missing preposition found.
Context: ... successful download and verification * The path where the new binary has been plac...(AI_HYDRA_LEO_MISSING_OF)
🔇 Additional comments (5)
tools/cosmovisor/go.mod (1)
13-13: Verify the necessity and implications of adding gRPC as a direct dependency.The addition of
google.golang.org/grpc v1.67.0as a direct dependency suggests that the Cosmovisor tool now explicitly requires gRPC functionality. This change aligns with the PR objective of adding a newprepare-upgradecommand, which may involve network communication.However, please consider the following points:
Ensure that this direct dependency is necessary for the new
prepare-upgradecommand. If it's only used in a small part of the new functionality, consider if it can remain an indirect dependency.Verify that version 1.67.0 is the most appropriate version for your needs. It's generally good practice to use the latest stable version unless there are specific reasons not to.
Check for any potential conflicts or incompatibilities with other dependencies that might be using gRPC indirectly.
Consider the impact on the module's size and compilation time. Adding gRPC as a direct dependency might increase both.
To ensure this change doesn't introduce any unforeseen issues, please run the following commands:
If these checks pass without issues, the change can be considered safe.
tools/cosmovisor/args.go (3)
36-36: LGTM: New environment variable constant addedThe addition of
EnvGRPCAddressconstant follows the existing pattern and naming convention for environment variables in this file.
67-67: LGTM: New GRPCAddress field added to Config structThe
GRPCAddressfield is correctly added to theConfigstruct with appropriatetomlandmapstructuretags, consistent with other fields in the struct.
287-290: LGTM: GRPCAddress configuration addedThe
GetConfigFromEnvfunction has been correctly updated to handle the newGRPCAddressconfiguration. It reads from the environment variable and provides a sensible default value of "localhost:9090" if not set.tools/cosmovisor/README.md (1)
21-21: LGTM: Table of contents updated correctly.The new section "Preparing for an Upgrade" has been properly added to the table of contents, maintaining consistency with the document structure.
🧰 Tools
🪛 Markdownlint
21-21: Expected: 8; Actual: 4
Unordered list indentation(MD007, ul-indent)
Description
Closes: #10910
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
prepare-upgradecommand to automate upgrade preparation, including downloading and verifying upgrade binaries.cosmovisor show-upgrade-infocommand to display relevant upgrade information.Improvements
initandconfigcommands to support custom configuration file paths.Bug Fixes
add-upgradecommand and improved output parsing.