Merge the `sn-node-manager` crate into the `safe_network` workspace #1218

jacderida · 2024-01-23T20:20:09Z

This retains the commit history from the node manager repository.

Description

Summary generated by Reviewpad on 23 Jan 24 20:20 UTC

This pull request includes multiple changes across different files.

The README.md file has undergone minor modifications to improve the clarity of instructions and content.
The Justfile script has been enhanced to include new targets for creating and deleting droplets, as well as modifying an existing target for error handling.
The CHANGELOG.md file documents additions, fixes, and enhancements to the project.
The Cargo.toml file in the sn-node-manager directory specifies package details, dependencies, and features.
A new script, run_local_service_network.sh, has been added to start a local service network and perform various operations.
A new file, helpers.rs, has been added containing helper functions for downloading and extracting release binaries.
The Vagrantfile has been added to configure a Vagrant virtual machine for the project.
The e2e.rs file includes a new test for installing and controlling services.
The Cargo.toml file includes a new "sn-node-manager" module in the members list.
The README.md file in the sn-node-manager directory provides information about Safenode Manager.
A new file, config.rs, has been added to handle file paths and permissions in Unix and Windows environments.
Some environment variables and CI job changes have been made in one of the files.
Some files and directories have been added to the ignored files list.

These changes span across various aspects such as documentation, scripts, configuration, tests, and code enhancements. Let me know if you need more details or assistance with any specific part.

sn-node-manager/src/service.rs

sn-node-manager/src/main.rs

sn-node-manager/src/local.rs

sn-node-manager/tests/e2e.rs

resources/run_local_service_network.sh

+
+port=$(echo "$output" | jq -r '.[0].port')
+peer_id=$(echo "$output" | jq -r '.[0].peer_id')
+genesis_multiaddr="/ip4/127.0.0.1/tcp/${port}/p2p/${peer_id}"


reviewpad · 2024-01-23T20:20:56Z

Reviewpad Report

‼️ Errors

Unconventional commit detected: 'feat: provide run command

NOTE: this commit also contains the corresponding kill command. I accidentally amended it and
couldn't untangle the changes from it without losing them.

Spin up safenode processes, along with a faucet, to run a local network. These processes are not
defined as services because they are not intended to be long running; they are for experimentation.

As in the testnet binary, the network is validated, but unlike in testnet the logs are not
inspected. We just query each RPC service to check that peers are connected.

At present you need to supply paths to both binaries, but this will be extended shortly to provide
the ability to run downloaded binaries.

The node registry was extended to keep track of the faucet process so it can easily be cleaned up in
a subsequent kill command.

The status command is also extended to accommodate the local network.' (a1482d4)

Unconventional commit detected: 'test: force skip validation

enable this once validation passes succesfully' (4cdd102)

Unconventional title detected: 'Merge the sn-node-manager crate into the safe_network workspace' illegal 'M' character in commit message type: col=00

.gitignore

sn_node_manager/src/service.rs

+            OsString::from("--port"),
+            OsString::from(config.node_port.to_string()),
+            OsString::from("--rpc"),
+            OsString::from(format!("127.0.0.1:{}", config.rpc_port)),


sn_node_manager/src/service.rs

+    }
+
+    fn get_available_port(&self) -> Result<u16> {
+        let addr: SocketAddr = "127.0.0.1:0".parse().unwrap();


sn_node_manager/src/service.rs

sn_node_manager/src/main.rs

+            } else {
+                for node in node_registry.nodes.iter_mut() {
+                    let rpc_client =
+                        RpcClient::new(&format!("https://127.0.0.1:{}", node.rpc_port));


sn_node_manager/src/main.rs

+                        ))
+                    })?;
+
+                let rpc_client = RpcClient::new(&format!("https://127.0.0.1:{}", node.rpc_port));


sn_node_manager/src/control.rs

+    // Again confirm that services which are marked running are still actually running.
+    // If they aren't we'll mark them as stopped.
+    for node in &mut node_registry.nodes {
+        let rpc_client = RpcClient::new(&format!("https://127.0.0.1:{}", node.rpc_port));


sn_node_manager/src/local.rs

+    all_peers.extend(additional_peers);
+
+    for node in node_registry.nodes.iter() {
+        let rpc_client = RpcClient::new(&format!("https://127.0.0.1:{}", node.rpc_port));


sn_node_manager/src/local.rs

+    for _ in start..=network_options.node_count {
+        let port = service_control.get_available_port()?;
+        let rpc_port = service_control.get_available_port()?;
+        let rpc_client = RpcClient::new(&format!("https://127.0.0.1:{rpc_port}"));


sn_node_manager/src/local.rs

+    } else {
+        let port = service_control.get_available_port()?;
+        let rpc_port = service_control.get_available_port()?;
+        let rpc_client = RpcClient::new(&format!("https://127.0.0.1:{rpc_port}"));


sn_node_manager/src/local.rs

+        args.push("--port".to_string());
+        args.push(port.to_string());
+        args.push("--rpc".to_string());
+        args.push(format!("127.0.0.1:{rpc_port}"));


The `peers` argument is no longer declared with an `Option` and the manual parsing of `SAFE_PEERS` is removed. If the user doesn't specify any peers using `--peer` or `SAFE_PEERS`, the `get_peers_from_args` function will return a particular type of error. In this case, we can just assume the user wants the nodes to join an existing local network.

The binary path is provided because this is useful to know, particularly for nodes installed as services. The number of connected peers are also included, because again, this can be useful to know if the network has initialised itself correctly, and how it might change over time. There was a bit of a refactor in here to have the `run_node` function return a `Node` rather than a multiaddr, because there's a function on the node now to get the multiaddr. The refactor also eliminated a test case, which was no longer necessary.

If this flag is set, the `--local` flag will be added to the arguments in the service definition. This has been added to facilitate running a local network using services, which is going to be used in the backward compatibility test in the `safe_network` repo.

Output the status report in json: this will be useful during CI and backwards compatibility testing to obtain the multiaddr for the initial service node. Fail if any nodes are not running: useful for quickly asserting that all nodes in the network must be running for the test to proceed.

The difference between this and the `run` command is that it sets up the nodes as services rather than just running them as processes. It requires root to run. The script will be used by the local testnet Github Action to start a network for the backwards compatibility test. The network will then be upgraded.

This command is useful to quickly spin up a faucet for a service-based local network, and it will be used in the backwards compatibility test.

An earlier refactor to return a node rather than a multiaddr has made it unnecessary to have the `build_multiaddr` function in the node manager.

- enable this once validation passes succesfully

Use references to crates in the workspace rather than version numbers.

The crate is renamed back to `sn-node-manager` because we've already had many versions published with that name, but the directory will still be named `sn_node_manager` to be consistent with the rest of the directories in the `safe_network` workspace.

When launching a local network, for now, we need to build the node manager rather than download the latest version. Once the node manager has been added to the `safe_network` release process, we can change this. The `sn-local-testnet-action` has been updated to assume that we're building the node manager from `safe_network`, as opposed to its own repository.

reviewpad bot requested a review from maqi January 23, 2024 20:20

github-advanced-security bot found potential problems Jan 23, 2024

View reviewed changes

reviewpad bot added Large Large sized PR waiting-for-review labels Jan 23, 2024

jacderida force-pushed the merge-node-manager branch from 088e704 to 81e4dd9 Compare January 23, 2024 21:25

RolandSherwin reviewed Jan 24, 2024

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

RolandSherwin approved these changes Jan 24, 2024

View reviewed changes

github-advanced-security bot found potential problems Jan 24, 2024

View reviewed changes

RolandSherwin force-pushed the merge-node-manager branch from 78fa02e to aae62d2 Compare January 24, 2024 08:56

joshuef enabled auto-merge January 24, 2024 09:01

joshuef added this pull request to the merge queue Jan 24, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 24, 2024

joshuef added this pull request to the merge queue Jan 24, 2024

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 24, 2024

RolandSherwin added this pull request to the merge queue Jan 24, 2024

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 24, 2024

jacderida force-pushed the merge-node-manager branch 2 times, most recently from aa80b8d to b74ef04 Compare January 24, 2024 14:59

jacderida changed the title ~~WIP -- Merge the sn-node-manager crate into the safe_network workspace~~ Merge the sn-node-manager crate into the safe_network workspace Jan 24, 2024

jacderida force-pushed the merge-node-manager branch 9 times, most recently from 4095447 to c8e187c Compare January 24, 2024 22:38

RolandSherwin force-pushed the merge-node-manager branch from 863efd8 to 45b3249 Compare January 25, 2024 07:05

jacderida and others added 24 commits January 25, 2024 12:43

chore(release): 0.1.16

9a4564f

feat: add quic/tcp features and set quic as default

a66e649

chore(release): 0.1.17

9ccf0eb

feat: provide faucet command

23962b5

This command is useful to quickly spin up a faucet for a service-based local network, and it will be used in the backwards compatibility test.

chore: fixup after rebase

38c8c8d

An earlier refactor to return a node rather than a multiaddr has made it unnecessary to have the `build_multiaddr` function in the node manager.

chore(deps): update sn_protocol to the newer version

c4bf051

chore(release): 0.1.18

9c9cb71

fix: add delay to make sure we drop the socket

d978b5e

test: force skip validation

4cdd102

- enable this once validation passes succesfully

chore(release): 0.1.19

1f1f338

chore(test): remove unused structs

6b133b4

fix(deps): use local dependency for node manager

960e9f2

refactor(manager): rename node manager crate

8fbd0e3

chore: reference crates in workspace

bae7a75

Use references to crates in the workspace rather than version numbers.

fix(ci): specify custom CARGO_TARGET_DIR for windows test bin

5cdc86c

fix(manager): increase port unbinding time

53af320

fix(ci): stop cargo test from building after the build step

37b8cb1

jacderida force-pushed the merge-node-manager branch from fe744a6 to 0bd6461 Compare January 25, 2024 12:53

ci: use main repo for local testnet action

67c9120

jacderida force-pushed the merge-node-manager branch from 0bd6461 to 67c9120 Compare January 25, 2024 12:58

jacderida added this pull request to the merge queue Jan 25, 2024

joshuef removed this pull request from the merge queue due to a manual request Jan 25, 2024

joshuef merged commit a81c4fb into maidsafe:main Jan 25, 2024
38 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge the `sn-node-manager` crate into the `safe_network` workspace #1218

Merge the `sn-node-manager` crate into the `safe_network` workspace #1218

jacderida commented Jan 23, 2024 •

edited by reviewpad bot

reviewpad bot commented Jan 23, 2024 •

edited

Merge the sn-node-manager crate into the safe_network workspace #1218

Merge the sn-node-manager crate into the safe_network workspace #1218

Conversation

jacderida commented Jan 23, 2024 • edited by reviewpad bot

Description

Summary generated by Reviewpad on 23 Jan 24 20:20 UTC

reviewpad bot commented Jan 23, 2024 • edited

Merge the `sn-node-manager` crate into the `safe_network` workspace #1218

Merge the `sn-node-manager` crate into the `safe_network` workspace #1218

jacderida commented Jan 23, 2024 •

edited by reviewpad bot

reviewpad bot commented Jan 23, 2024 •

edited