Skip to content

feat(block-node): implement 'solo block node add' command and modify remote config to support multiple relays, block nodes, mirror nodes and explorers #1821

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

Open
wants to merge 75 commits into
base: main
Choose a base branch
from

Conversation

instamenta
Copy link
Contributor

@instamenta instamenta commented Apr 8, 2025

Description

  • adds new command and command group block node with the class BlockNodeCommand

    • adds new sub-command for add used to install the block node official helm chart inside the specified cluster
  • Adds wrapper classes and logic for creating commands with 3 levels of depth

    • Example: solo block node add

Related Issues

Pull request (PR) checklist

  • This PR added tests (unit, integration, and/or end-to-end)
  • This PR updated documentation
  • This PR added no TODOs or commented out code
  • This PR has no breaking changes
  • Any technical debt has been documented as a separate issue and linked to this PR
  • Any package.json changes have been explained to and approved by a repository manager
  • All related issues have been linked to this PR
  • All changes in this PR are included in the description
  • When this PR merges the commits will be squashed and the title will be used as the commit message, the 'commit message guidelines' below have been followed

Testing

  • This PR added unit tests
  • This PR added integration/end-to-end tests
  • These changes required manual testing that is documented below
  • Anything not tested is documented

…ition, added the new flag about the block nodes chart version together with the initial constants

Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
… and initial setup for m4 apple chip compatibility

Signed-off-by: Zhan Milenkov <[email protected]>
…block-nodes-deploy

# Conflicts:
#	src/commands/base.ts
#	src/commands/flags.ts
#	src/core/constants.ts
…ngs in the remote config related classes and worked on adding support for the new component type 'BlockNodeComponents'

Signed-off-by: Zhan Milenkov <[email protected]>
…cy and make it less error prone

Signed-off-by: Zhan Milenkov <[email protected]>
…and renamed ComponentType to ComponentTypes to follow convention

Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
…codebase to use them, fixed most unit tests, todo: fix all tests, add new logic for editing remote components which should only work for consensus node's node state field

Signed-off-by: Zhan Milenkov <[email protected]>
…onExpressions to avoid having to have dublicated function return value difinitions

Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
@instamenta instamenta added the PR: Needs Manager Approval A pull request that needs review from a manager. label Apr 10, 2025
@jeromy-cannon jeromy-cannon added PR: Checks Failed A pull request where the checks have failed. PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. and removed PR: Checks Failed A pull request where the checks have failed. labels Apr 14, 2025
rbarker-dev
rbarker-dev previously approved these changes Apr 15, 2025
…block-nodes-deploy

# Conflicts:
#	src/core/config/remote/components/consensus-node-component.ts
#	src/core/config/remote/components/relay-component.ts
#	src/core/config/remote/remote-config-manager.ts
@instamenta instamenta dismissed stale reviews from rbarker-dev and JeffreyDallas via 52f085d April 15, 2025 07:36
@instamenta instamenta removed PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. PR: Needs Team Approval A pull request that needs review from a team member. labels Apr 15, 2025
Signed-off-by: Zhan Milenkov <[email protected]>
@instamenta instamenta added Blocked Further development work is blocked by other item and removed PR: Needs Manager Approval A pull request that needs review from a manager. labels Apr 16, 2025
@instamenta instamenta marked this pull request as draft April 16, 2025 12:25
@jeromy-cannon jeromy-cannon added the P0 An issue impacting production environments or impacting multiple releases or multiple individuals. label Apr 22, 2025
…block-nodes-deploy

# Conflicts:
#	src/commands/explorer.ts
#	src/commands/mirror-node.ts
#	version.ts
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
@instamenta instamenta marked this pull request as ready for review April 24, 2025 12:51
@instamenta instamenta requested review from a team as code owners April 24, 2025 12:51
@instamenta instamenta removed the Blocked Further development work is blocked by other item label Apr 24, 2025
@@ -137,7 +137,7 @@ export default [
'warn',
{
allowExpressions: false,
allowTypedFunctionExpressions: false,
allowTypedFunctionExpressions: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

} catch (error) {
logger.showUserError(error);
expect.fail();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good verification to add:
image

get-block.sh.txt
block_service.proto.txt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you apply this to the block node destroy PR where the integration with consensus happens? I'm not sure what happens with block node for this test without consensus node actually integrated.

This is my original question that the image above was a response to:

What is the best way to see if Block Nodes is functional after Consensus Nodes are running?

name: 'block-node-version',
definition: {
describe: 'Block nodes chart version',
defaultValue: version.MIRROR_NODE_VERSION,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy/paste error?

defaultValue: version.MIRROR_NODE_VERSION,
type: 'string',
},
prompt: async function promptMirrorNodeVersion(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy/paste error?

Comment on lines +1638 to +1641
Flags.mirrorNodeVersion.definition.defaultValue as boolean,
'Would you like to choose mirror node version? ',
null,
Flags.mirrorNodeVersion.name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy/paste error?


if (isAppleM4SeriesChip) {
valuesArgument += helpers.populateHelmArguments({
'blockNode.config.JAVA_OPTS': '"-Xms8G -Xmx8G -XX:UseSVE=0"',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... this won't work if we deploy to GKE, AWS, or Latitude from our MacOS laptops that have a M4 chip and they have overrode the min/max JVM size in the values file.

task: async (context_): Promise<void> => {
const config: BlockNodeDeployConfigClass = context_.config;

config.isChartInstalled = await this.chartManager.isChartInstalled(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this being used?

};
}

private displayHealthcheckData(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private displayHealthcheckData(
private displayHealthCheckData(

task: async (context_, task): Promise<void> => {
const config: BlockNodeDeployConfigClass = context_.config;

const displayHealthcheckCallback: (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const displayHealthcheckCallback: (
const displayHealthCheckCallback: (

@jeromy-cannon jeromy-cannon changed the title feat(block-node): implement 'solo block node add' command and modify remote config to support multiple relays, blokck nodes, mirror nodes and explorers feat(block-node): implement 'solo block node add' command and modify remote config to support multiple relays, block nodes, mirror nodes and explorers Apr 24, 2025
@jeromy-cannon jeromy-cannon added the PR: Unresolved Comments A pull request where there are comments and they need to be resolved. label Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 An issue impacting production environments or impacting multiple releases or multiple individuals. PR: Unresolved Comments A pull request where there are comments and they need to be resolved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement solo block-nodes deploy
4 participants