Add script to create/delete multiple GCE instances #573

pgarg66 · 2018-07-10T19:15:13Z

This script outputs the IP address array that can be used
with start_nodes script to launch multinode demo

- This script outputs the IP address array that can be used with start_nodes script to launch multinode demo - Changes to start_nodes to compress files for rsync

garious · 2018-07-10T19:24:53Z

multinode-demo/gce_multinode.sh

+
+Creates a GCE multinode network
+
+ -c create/delete - Create or delete the network


Can you make the command an argument instead of an option? So you'd write gce_multinode.sh create instead of gce_multinode.sh -c create?

mvines

It looks like on create, there won't be much stdout output? It could be useful to report the number of VMs created or something, to let the user know machines are now running and 💸 is happening :)

mvines · 2018-07-10T22:19:48Z

multinode-demo/gce_multinode.sh

+  nodes+=("$prefix$i")
+done
+
+if [ "$command" = "create" ]; then


[[ instead of [ please, same for elif

mvines · 2018-07-10T22:22:42Z

multinode-demo/gce_multinode.sh

@@ -0,0 +1,89 @@
+#!/bin/bash
+
+command=$1


I think you'll want a shift after command=$1 so that the getopts below won't see $1 (or is that what OPTIND=$OPTIND+1 is doing?)

Yes, OPTIND+1 is doing the same. If shift is the better form, I can use that.

I prefer shift

mvines · 2018-07-10T22:24:20Z

multinode-demo/gce_multinode.sh

+
+set -e
+
+if [[ -z "$command" ]]; then


I'm partial to this one-liner: [[ -n $command ]] || usage (same for the rest below)
(also note the lack of double quotes, which are unnecessary for [[)

mvines · 2018-07-10T22:25:03Z

multinode-demo/gce_multinode.sh

+elif [ "$command" = "delete" ]; then
+  gcloud beta compute instances delete "${nodes[@]}"
+else
+  usage


nit: usage "Unknown command: $command"

mvines · 2018-07-10T22:27:17Z

multinode-demo/gce_multinode.sh

+  ip_addr_list=$(gcloud beta compute instances create "${nodes[@]}" --zone=us-west1-b --tags=testnet \
+    --image="$image_name" | grep "RUNNING" | tr -s ' ' | cut -f 5 -d ' ')
+
+  echo "ip_addr_array=($ip_addr_list)" >"$out_file"


nit: add one space between >"

My shell format plugin (for VSCode) removes the space if I add it.

vim :)
oh well, I guess we have no canonical bash formatter right now.

mvines · 2018-07-10T22:28:24Z

multinode-demo/gce_multinode.sh

+    echo "Error: $*"
+  fi
+  cat <<EOF
+usage: $0 <create/delete> <-p prefix> <-n num_nodes> <-o file> [-i image-name]


nit: create/delete -> create|delete?

rob-solana · 2018-07-10T22:41:57Z

multinode-demo/gce_multinode.sh

+    --image="$image_name" | grep "RUNNING" | tr -s ' ' | cut -f 5 -d ' ')
+
+  echo "ip_addr_array=($ip_addr_list)" >"$out_file"
+elif [ "$command" = "delete" ]; then


prefer [[ to [

if you use [[, you can also use ==, and skip quotes

rob-solana · 2018-07-10T22:42:25Z

multinode-demo/gce_multinode.sh

+    exit 0
+    ;;
+  p)
+    prefix="$OPTARG"


quotes unnecessary when command is only assignment

rob-solana · 2018-07-10T22:42:37Z

multinode-demo/gce_multinode.sh

+  case $opt in
+  h | \?)
+    usage
+    exit 0


doesn't usage always exit?

rob-solana · 2018-07-10T22:43:10Z

multinode-demo/gce_multinode.sh

+fi
+
+if [[ -z "$prefix" ]]; then
+  usage


have usage throw an error for these cases, i.e. supply an error string

rob-solana · 2018-07-10T22:46:17Z

multinode-demo/gce_multinode.sh

+  fi
+
+  ip_addr_list=$(gcloud beta compute instances create "${nodes[@]}" --zone=us-west1-b --tags=testnet \
+    --image="$image_name" | grep "RUNNING" | tr -s ' ' | cut -f 5 -d ' ')


grep "RUNNING" | tr -s ' ' | cut -f 5 -d ' '

is the same as

awk '/RUNNING/ {print $5}'

rob-solana · 2018-07-10T22:51:38Z

multinode-demo/gce_multinode.sh

+    usage
+  fi
+
+  ip_addr_list=$(gcloud beta compute instances create "${nodes[@]}" --zone=us-west1-b --tags=testnet \


you could instead do:

ip_addr_array=( $(gcloud beta compute instances create "${nodes[@]}" --zone=us-west1 ... ) declare -p ip_addr_array > "$out_file"

This change is giving me shellcheck errors.

directive it away, shellcheck is sometimes dumb

pgarg66 · 2018-07-10T23:29:29Z

The script outputs the list of GCE instances that gets created. For delete, it prompts the user to confirm the delete operation.

Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.20 to 1.0.21. - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@1.0.20...1.0.21) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump thiserror from 1.0.57 to 1.0.58 Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.57 to 1.0.58. - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@1.0.57...1.0.58) --- updated-dependencies: - dependency-name: thiserror dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> * [auto-commit] Update all Cargo lock files --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite <[email protected]>

Add script to create/delete multiple GCE instances

e096f16

- This script outputs the IP address array that can be used with start_nodes script to launch multinode demo - Changes to start_nodes to compress files for rsync

pgarg66 requested review from garious, aeyakovenko, mvines and rob-solana July 10, 2018 19:15

pgarg66 force-pushed the pankaj-remote-multinode-env branch from 6559818 to b93d0ee Compare July 10, 2018 19:16

garious reviewed Jul 10, 2018

View reviewed changes

pgarg66 force-pushed the pankaj-remote-multinode-env branch 2 times, most recently from 98b9f11 to a17ac0f Compare July 10, 2018 20:23

garious approved these changes Jul 10, 2018

View reviewed changes

mvines reviewed Jul 10, 2018

View reviewed changes

rob-solana reviewed Jul 10, 2018

View reviewed changes

multinode-demo/gce_multinode.sh Outdated

exit 0

;;

p)

prefix="$OPTARG"

Copy link

Contributor

rob-solana Jul 10, 2018

Choose a reason for hiding this comment

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

quotes unnecessary when command is only assignment

rob-solana reviewed Jul 10, 2018

View reviewed changes

multinode-demo/gce_multinode.sh Outdated

case $opt in

h | \?)

usage

exit 0

Copy link

Contributor

rob-solana Jul 10, 2018

Choose a reason for hiding this comment

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

doesn't usage always exit?

rob-solana reviewed Jul 10, 2018

View reviewed changes

pgarg66 force-pushed the pankaj-remote-multinode-env branch from a17ac0f to cf1019b Compare July 10, 2018 23:09

garious changed the title ~~Script to create/delete multiple GCE instances~~ Add script to create/delete multiple GCE instances Jul 10, 2018

pgarg66 force-pushed the pankaj-remote-multinode-env branch from cf1019b to e096f16 Compare July 10, 2018 23:28

rob-solana approved these changes Jul 11, 2018

View reviewed changes

garious merged commit d7d9853 into master Jul 11, 2018

pgarg66 deleted the pankaj-remote-multinode-env branch July 11, 2018 00:17

snyk-bot mentioned this pull request Jun 7, 2023

[Snyk] Upgrade @crowdin/cli from 3.9.0 to 3.11.1 masakazu12/solana#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add script to create/delete multiple GCE instances #573

Add script to create/delete multiple GCE instances #573

pgarg66 commented Jul 10, 2018

garious Jul 10, 2018

pgarg66 Jul 10, 2018

mvines left a comment

mvines Jul 10, 2018

mvines Jul 10, 2018

pgarg66 Jul 10, 2018

mvines Jul 10, 2018

pgarg66 Jul 10, 2018

mvines Jul 10, 2018

mvines Jul 10, 2018

mvines Jul 10, 2018

pgarg66 Jul 10, 2018

mvines Jul 10, 2018

mvines Jul 10, 2018

pgarg66 Jul 10, 2018

rob-solana Jul 10, 2018

rob-solana Jul 10, 2018

rob-solana Jul 10, 2018

rob-solana Jul 10, 2018

rob-solana Jul 10, 2018

rob-solana Jul 10, 2018

pgarg66 Jul 10, 2018

rob-solana Jul 11, 2018

pgarg66 commented Jul 10, 2018


		Creates a GCE multinode network

		-c create/delete - Create or delete the network

		@@ -0,0 +1,89 @@
		#!/bin/bash

		command=$1

Add script to create/delete multiple GCE instances #573

Add script to create/delete multiple GCE instances #573

Conversation

pgarg66 commented Jul 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgarg66 commented Jul 10, 2018