-
Notifications
You must be signed in to change notification settings - Fork 8
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
Instacollector Kafka 1.0.0 #9
base: main
Are you sure you want to change the base?
Conversation
kafka/README.md
Outdated
* The term "VM" in environment of script `cluster_collector.sh` means if running in kernel. | ||
|
||
# Design info: | ||
There are two scripts used in instacollector tool for kafka. |
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.
Could you go through and capitalise the product names in here and the scripts? Kafka, Docker, etc.
kafka/README.md
Outdated
# Execution settings: | ||
The `cluster_collector.sh` has setting of connecting to cluster nodes using the provided ssh key file or an id file. | ||
|
||
If the ssh key has passphrase enabled then please use `ssh-agent` & `ssh-add` commands to add the passphrase before running `cluster_collector.sh` script. |
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.
Maybe add an example of how to do that?
kafka/README.md
Outdated
Note: | ||
* This won't work on versions before kafka 2 | ||
* User is requested to change the path values in `node_collector.sh` before running any of the scripts, as by default the script uses `Kafka configuration file locations`, `data directory location`, and other setting locations as per **Apache Kafka** default setup. | ||
* The term "VM" in environment of script `cluster_collector.sh` means if running in kernel. |
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.
What do you mean by this?
kafka/README.md
Outdated
``` | ||
2. `cluster_collector.sh`: to be executed on a machine connected to Kafka cluster e.g. user laptop with a running docker or a running VM. It executes node_collector.sh on each Kafka node using ssh. The cluster_collector.sh requires 4 user inputs : | ||
``` | ||
* Enter your kafka environment (vm/docker) : |
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.
Seems to make more sense asking how we want to connect - SSH/Docker.
kafka/cluster_collector.sh
Outdated
|
||
#id_file='/Users/rahulchakrabarty-instaclustr/.ssh/rahulchakrabarty-instaclustr' | ||
elif [[ "${kenv}" == "docker" ]]; then | ||
read -p "Enter docker home directory :" docker_home |
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.
It's not clear to me as a user what is this directory supposed to be? It looks like it's used further down as a temporary location to copy the node_collector.sh script to and run from. You should specify that here - otherwise people are going to assume you are asking for a directory on the host machine. Also, do we actually need the user to specify it? Can we just use /tmp, ~, something else?
kafka/cluster_collector.sh
Outdated
#Collect environment info (VM/docker) | ||
read -p "Enter your kafka environment (vm/docker) :" kenv | ||
|
||
if [[ "${kenv}" == "vm" ]]; 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.
Seems to make more sense asking how we want to connect - SSH/Docker.
kafka/cluster_collector.sh
Outdated
|
||
if [[ "${kenv}" == "vm" ]]; then | ||
#Collect user info. | ||
read -p "Enter username for login on Kafka cluster nodes (Press Enter for default admin) :" user |
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.
Specify SSH username
kafka/cluster_collector.sh
Outdated
|
||
#user='rahulchakrabarty' | ||
|
||
read -p "Enter Identity file path:" id_file |
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.
Specify SSH identity file
kafka/cluster_collector.sh
Outdated
fi | ||
echo "Copying file node_collector.sh to container" | ||
docker cp ./node_collector.sh $peer:$docker_home/ | ||
docker exec $peer /bin/bash -c "sh $docker_home/node_collector.sh $peer $config_file" & |
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.
Should this be docker exec $peer sh "$docker_home/node_collector.sh $peer $config_file" &
? What happens if the container doesn't have bash?
You're also not providing the -ip / --ip
or -c / --command-config
flags. Config file will never be used, and IP will be set to the default hostname output in the node_collector script.
kafka/cluster_collector.sh
Outdated
if [[ -z "$peer" ]]; then | ||
break | ||
fi | ||
ssh -i $id_file $user@$peer "bash -s" < node_collector.sh $peer $config_file & |
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.
You're not providing the -ip / --ip
or -c / --command-config
flags. Config file will never be used, and IP will be set to the default hostname output in the node_collector script.
version 1.0.0