-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Fixed issues with configuring new GCE instances #566
Conversation
pgarg66
commented
Jul 10, 2018
- New nodes cloned from a working node can be used with the script
- Script takes care of installing SSH keys, and package dependencies correctly
- New nodes cloned from a working node can be used with the script - Script takes care of installing SSH keys, and package dependencies correctly
multinode-demo/start_nodes.sh
Outdated
|
||
ssh "$remote_user"@"$ip_addr" 'mkdir ~/.ssh' | ||
|
||
ssh -n -f "$remote_user"@"$ip_addr" "sudo service sshguard stop" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a TODO comment here to avoid killing sshguard (filing an issue works too), as well as documenting why in a comment. For now this is ok, but in general we don't want to do be doing this IMO
multinode-demo/start_nodes.sh
Outdated
ssh -n -f "$remote_user"@"$ip_addr" "sudo service sshguard stop" | ||
ssh -n -f "$remote_user"@"$ip_addr" 'sudo apt-get --assume-yes install rsync' | ||
|
||
if [[ -n "$leader" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rob-solana is gonna tell you to remove those double quotes, [[
doesn't need them :)
multinode-demo/start_nodes.sh
Outdated
|
||
if [[ -n "$leader" ]]; then | ||
echo "Adding known hosts for $ip_addr" | ||
ssh -n -f "$remote_user"@"$ip_addr" "ssh-keygen -R $leader" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "$remote_user@$ip_addr"
seems nicer than "$remote_user"@"$ip_addr"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just nits, r+
Addressing comments. Will push another patch after testing it. |
33a16fa
to
abe075a
Compare
@garious , I believe, this change can be merged. |
* Add Ristretto version of THEMIS BN BPF instruction counts: CalculateAggregate: 83,511 SubmitProofDecryption: 33,755,027 Ristretto BPF instruction counts: CalculateAggregate: 13,049,558 SubmitProofDecryption: 13,149,232 * Fix CI script
…ee (solana-labs#566) * refactor shareble code into its own function; update and add tests * add function to reward with full prio fee and burnt transaction fee