-
Notifications
You must be signed in to change notification settings - Fork 33
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
[P2P] Peer discovery debug CLI sub-commands #730
Open
10 tasks
bryanchriswhite opened this issue
May 4, 2023
· 2 comments
· Fixed by #891 or #895 · May be fixed by #801 or #892
Open
10 tasks
[P2P] Peer discovery debug CLI sub-commands #730
bryanchriswhite opened this issue
May 4, 2023
· 2 comments
· Fixed by #891 or #895 · May be fixed by #801 or #892
Milestone
Comments
bryanchriswhite
changed the title
[P2P] Peer discovery debug CLI sub-commands
[WIP] [P2P] Peer discovery debug CLI sub-commands
May 4, 2023
bryanchriswhite
changed the title
[WIP] [P2P] Peer discovery debug CLI sub-commands
[P2P] Peer discovery debug CLI sub-commands
May 4, 2023
20 tasks
20 tasks
20 tasks
bryanchriswhite
added a commit
that referenced
this issue
Jun 13, 2023
## Description <!-- reviewpad:summarize:start --> ### Summary generated by Reviewpad on 13 Jun 23 12:53 UTC This pull request includes various updates across the codebase, including changes to CLI flags, import statements, and default values. It also involves refactoring code to use shared helper functions, removing unused code, and improving error handling. Some variables have been renamed to be more descriptive. The specific changes include modifications to files such as `validator.go`, `cluster-manager.yaml`, `cli-client.yaml`, `utils.go`, and `runtime_defaults.go`. For example, the `RPC_HOST` variable has been replaced with `POCKET_REMOTE_CLI_URL`. A new `helper` package has been added, and a new `flags` package contains several CLI flags. Code has also been updated to use `flags.RemoteCLIURL` instead of `remoteCLIURL`. Some constants have been renamed to include `Hostname`, and there are updates to endpoint hostnames for some validators. <!-- reviewpad:summarize:end --> ## Issue Dependants: - #730 ## Type of change Please mark the relevant option(s): - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Major breaking change - [ ] Documentation - [ ] Other <!-- add details here if it a different type of change --> ## List of changes - Exported `rootCmd` to support cross-package usage (i.e. subcommands w/ own pkgs) - Refactored common CLI code - Moved & refactored `PersistentPreRunE()` helper - Moved & exported `busCLICtxKey` - Exported `GetValueFromCLIContext()` and `SetValueInCLIContext()` - Moved `setupAndStartP2PModule` - Moved `setupCurrentHeightProvider` - Moved `setupPeerstoreProvider` - Refactored CLI flags to own package for cross-package use - Replaced RPC_HOST with POCKET_REMOTE_CLI_URL where appropriate - Added `remote-cli-url` flag to cluster manager & refactor - Added support for overriding of `remote-cli-url` flag with env var - Append "Hostname" to validator endpoint hostname constants - Promoted string literal to `RandomValidatorEndpointK8SHostname` constant - Improved e2e tests error messaging - Simplify e2e debug command calls - Improve error handling in client CLI ## Testing - [ ] `make develop_test`; if any code changes were made - [ ] `make test_e2e` on [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any code changes were made - [ ] `e2e-devnet-test` passes tests on [DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd); if any code was changed - [x] [Docker Compose LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md); if any major functionality was changed or introduced - [x] [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any infrastructure or configuration changes were made ## Required Checklist - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment)) - [x] I have tested my changes using the available tooling - [x] I have updated the corresponding CHANGELOG ### If Applicable Checklist - [ ] I have updated the corresponding README(s); local and/or global - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s) - [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s)
20 tasks
20 tasks
20 tasks
20 tasks
20 tasks
20 tasks
bryanchriswhite
added a commit
that referenced
this issue
Jul 13, 2023
## @Reviewer This PR may be more digestible / reviewable on a commit-by-commit basis. Commits are organized logically and any given line is only modified in a single commit, with few exceptions*. *(In the interest of preserving the git-time-continuum :police_officer::rotating_light:, this applies in batches of commits between comments or reviews *by humans*, only once "in review") --- ## Description Refactor P2P module dependencies as submodules, ultimately to support usage P2P module usage in the CLI. ## Issue Related: - #730 Dependant(s): - #891 - #801 - #892 ## Type of change Please mark the relevant option(s): - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Major breaking change - [ ] Documentation - [ ] Other <!-- add details here if it a different type of change --> ## List of changes - decoupled P2P module dependencies from Consensus module (disambiguates modules registry names) - added "current_height_provider" module registry slot - promoted `CurrentHeightProvider` to a submodule interface type - added `consensusCurrentHeightProvider` implementation - promoted `Router` interface to a submodule interface type - added "staked_actor_router" modules registry slot - added "unstaked_actor_router" modules registry slot - converted `backgroundRouter` implementation to submodule - converted `rainTreeRouter` implementation to submodule - simplified router base config ## Testing - [x] `make develop_test`; if any code changes were made - [x] `make test_e2e` on [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any code changes were made - [x] `e2e-devnet-test` passes tests on [DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd); if any code was changed - [ ] [Docker Compose LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md); if any major functionality was changed or introduced - [x] [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any infrastructure or configuration changes were made <!-- REMOVE this comment block after following the instructions If you added additional tests or infrastructure, describe it here. Bonus points for images and videos or gifs. --> ## Required Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment)) - [x] I have tested my changes using the available tooling - [ ] I have updated the corresponding CHANGELOG ### If Applicable Checklist - [ ] I have updated the corresponding README(s); local and/or global - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s) - [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s) --------- Co-authored-by: Daniel Olshansky <olshansky@pokt.network>
@bryanchriswhite We haven't merged in the P2P subcommand in yet, so do you think this was closed prematurely? |
Per my comment above, reopening until it's merged or told otherwise. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Objective
Extend the debug CLI to expose sub-commands for driving peer discovery in the P2P module (analogous to how it currently supports driving consensus). This will facilitate both manual and automated testing efforts.
Origin Document
Based on a discussion between @dylanlott and myself about end-to-end test planning for peer discovery. It has been made clear in discussions past (probably in documentation somewhere as well) that we want to prioritize supporting robust manual testing/investigation of these dynamic systems (before they get a chance to run away from us).
Goals
Deliverable
CLI UX -
peer
&p2p
subcommandsThe following commands should send/broadcast debug messages into the network, resulting in nodes to print to their respective logs, similar to the debug subcommand's behavior.
p2pCmd
persisten flags:--trace
NOTE:
p2p
commands MAY interact with a remote/standalone node rather than utilize the CLI's own P2P moduleNon-goals / Non-deliverables
Router
interfaceGeneral issue deliverables
Testing Methodology
make test_all
LocalNet
is still functioning correctly by following the instructions at docs/development/README.mdk8s LocalNet
is still functioning correctly by following the instructions hereCreator: @bryanchriswhite
The text was updated successfully, but these errors were encountered: