Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add cli flag for --static-nodes-file #1414

Closed
mFarghaly opened this issue Oct 3, 2020 · 9 comments
Closed

add cli flag for --static-nodes-file #1414

mFarghaly opened this issue Oct 3, 2020 · 9 comments
Assignees
@terrencecooke
Labels
good first issue Good for newcomers

Comments

@mFarghaly
Copy link

mFarghaly commented Oct 3, 2020 •
edited by timbeiko
Loading

Description

The only way to inject static nodes is to add static-nodes.json inside the data directory which makes it very hard to automate in environments like Kubernetes

required feature

besu --static-nodes-file /some/dir/static-nodes.json
@timbeiko timbeiko added the good first issue Good for newcomers label Oct 5, 2020
@timbeiko timbeiko changed the title add cli flag for --static-nodes add cli flag for --static-nodes-file Oct 5, 2020
@terrencecooke
Copy link
Contributor

Hi there, I would like to have this issue assigned to me.

@timbeiko
Copy link
Contributor

@terrencecooke it's assigned to you now 😄 !

@mFarghaly
Copy link
Author

mFarghaly commented Nov 30, 2020 •
edited
Loading

This will simplify things in http://kotal.co
thanks @terrencecooke

@terrencecooke
Copy link
Contributor

Thanks!

@terrencecooke
Copy link
Contributor

Would like to get a second (3rd, 4th, ... ) opinion.
After the changes I've made, this is how it currently works:

Situation 1:
When --static-nodes-file option is specified, besu chooses the SPECIFIED file, even if there is a static-nodes.json file in the data / default directory.

Situation 2:
When this is option is specified and the JSON file is ill-formed, besu will NOT continue the launch and will return an error.

Situation 3:
When this option is specified and the file isn't found (wrong path specified, a typo, just not there, etc), besu will simply display INFO message stating the file does not exist, and it will continue loading without connecting to static nodes. It will NOT consequently look for the default static-nodes.json file.

What do you think?
Does this operation make sense to the user?
Do you think most users would expect this to work in a different way?
I'm open to opinions.

@terrencecooke
Copy link
Contributor

On second thought, the aforementioned situations I mentioned, collectively seem pretty consistent with the way it already works with static-nodes.json file. It fails only when the file chosen is ill-formed, and continues loading without connecting to static nodes, if the file is not found.

I do think situation 3 is clear as is. At first I was thinking that if the file specified on the option line wasn't found, that it should then search for the default static-nodes.json in the default / data directory. However, it's probably better to NOT connect to static nodes at all, rather than to connect to a list of static nodes inadvertently.

@mFarghaly
Copy link
Author

currently static nodes are read from a single location data-dir/static-nodes
instead of reading from this single/fixed location, accept it as cli flag and/or env var
that's it, I think the rest of the scenarios are already taken care of.

@terrencecooke
Copy link
Contributor

Great! Thanks!

terrencecooke added a commit to terrencecooke/besu that referenced this issue Dec 1, 2020
@terrencecooke
Added command line option --static-nodes-file (hyperledger#1414)
9db8b9b 
Now able to inject static nodes by explicitly specifying
a static nodes JSON file (.json) on the command line

Signed-off-by: Terrence Cooke <terrence.s.cooke@gmail.com>
@terrencecooke terrencecooke mentioned this issue Dec 4, 2020
Merged
1 task
terrencecooke added a commit to terrencecooke/besu that referenced this issue Dec 7, 2020
@terrencecooke
Added command line option --static-nodes-file (hyperledger#1414)
c3333b9 
Now able to inject static nodes by explicitly specifying
a static nodes JSON file (.json) on the command line

Signed-off-by: Terrence Cooke <terrence.s.cooke@gmail.com>
terrencecooke added a commit to terrencecooke/besu that referenced this issue Dec 7, 2020
@terrencecooke
Added tests to BesuCommandTest (hyperledger#1414)
34630d8 
Three methods added to BesuCommandTest to test newly added
--static-nodes-file cli option

Signed-off-by: Terrence Cooke <terrence.s.cooke@gmail.com>
@terrencecooke
Copy link
Contributor

PR 1644 is available

RatanRSur added a commit that referenced this issue Jan 4, 2021
@terrencecooke @RatanRSur
Added command line option --static-nodes-file (#1414) (#1644)
9d22d03 
Now able to inject static nodes by explicitly specifying
a static nodes JSON file (.json) on the command line

Co-authored-by: Ratan (Rai) Sur <ratan.r.sur@gmail.com>
Signed-off-by: Terrence Cooke <terrence.s.cooke@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this issue Nov 3, 2023
@terrencecooke @RatanRSur @eum602
Added command line option --static-nodes-file (hyperledger#1414) (hyp…
b7e0897 
…erledger#1644)

Now able to inject static nodes by explicitly specifying
a static nodes JSON file (.json) on the command line

Co-authored-by: Ratan (Rai) Sur <ratan.r.sur@gmail.com>
Signed-off-by: Terrence Cooke <terrence.s.cooke@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@terrencecooke terrencecooke

Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mFarghaly @RatanRSur @timbeiko @terrencecooke