xunliu commented on code in PR #103:
URL:
https://github.com/apache/gravitino-playground/pull/103#discussion_r1834013187
##########
playground.sh:
##########
@@ -152,16 +233,16 @@ status() {
}
stop() {
- echo "INFO: Stopping the playground..."
+ echo "[INFO] Stopping the playground..."
case "$runtime" in
k8s)
- helm uninstall --namespace gravitino-playground gravitino-playground
+ helm uninstall --namespace gravitino-playground gravitino-playground
Review Comment:
I think need to add spaces in the line head, keep code sytle.
##########
playground.sh:
##########
@@ -77,50 +137,71 @@ checkHelm() {
major_version="${BASH_REMATCH[1]}"
echo "$major_version"
if [[ $major_version =~ "v3" ]]; then
- echo "INFO: helm check PASS."
+ echo "[INFO] helm check PASS."
return
else
- echo "ERROR: Please install helm v3"
+ echo "[ERROR] Please install helm v3"
exit
fi
fi
}
-checkPortInUse() {
- local port=$1
- if [[ "$(uname)" == "Darwin" ]]; then
- openPort=$(lsof -i :$port -sTCP:LISTEN)
- elif [[ "$(uname)" == "Linux" ]]; then
- openPort=$(sudo lsof -i :$port -sTCP:LISTEN)
+checkPortsInUse() {
+ local usedPorts=()
+ local availablePorts=()
+
+ for port in "${requiredPorts[@]}"; do
+ if [[ "$(uname)" == "Darwin" ]]; then
+ openPort=$(lsof -i :${port} -sTCP:LISTEN)
+ # Use sudo only when necessary
+ elif [[ "$(uname)" == "Linux" ]]; then
+ openPort=$(sudo lsof -i :${port} -sTCP:LISTEN)
+ fi
+
+ if [ -z "${openPort}" ]; then
+ availablePorts+=("${port}")
+ else
+ usedPorts+=("${port}")
+ fi
+ done
+
+ echo "[INFO] Port status check results:"
+
+ if [ ${#availablePorts[@]} -gt 0 ]; then
+ echo "[INFO] Available ports: ${availablePorts[*]}"
fi
- if [ -z "${openPort}" ]; then
- echo "INFO: Port $port is ok."
- else
- echo "ERROR: Port $port is in use. Please check it."
+
+ if [ ${#usedPorts[@]} -gt 0 ]; then
+ echo "[ERROR] Ports in use: ${usedPorts[*]}"
+ echo "[ERROR] Please check the ports."
exit 1
fi
}
start() {
- echo "INFO: Starting the playground..."
+ echo "[INFO] Starting the playground..."
+ echo "[INFO] The playground requires ${requiredCpuCores} CPU cores,
${requiredRamGB} GB of RAM, and ${requiredDiskSpaceGB} GB of disk storage to
operate efficiently."
- case "$runtime" in
- k8s)
- testK8s
- checkHelm
- ;;
- docker)
- testDocker
- checkCompose
- ports=(8090 9001 3307 19000 19083 60070 13306 15342 18080 18888 19090
13000)
- for port in "${ports[@]}"; do
- checkPortInUse ${port}
- done
- ;;
- esac
+ if [ "${skipChecks}" == false ]; then
+ case "$runtime" in
+ k8s)
+ testK8s
+ checkHelm
+ ;;
+ docker)
+ testDocker
+ checkDockerCompose
+ checkDockerDisk
+ checkDockerRam
+ checkDockerCpu
+
Review Comment:
Please remove this blank line.
##########
playground.sh:
##########
@@ -18,44 +18,104 @@
# under the License.
#
-set -x
-
playground_dir="$(dirname "${BASH_SOURCE-$0}")"
playground_dir="$(
- cd "${playground_dir}" >/dev/null
+ cd "${playground_dir}" >/dev/null || exit 1
pwd
)"
+requiredDiskSpaceGB=25
+requiredRamGB=8
+requiredCpuCores=2
+requiredPorts=(8090 9001 3307 19000 19083 60070 13306 15342 18080 18888 19090
13000)
+
testDocker() {
- echo "INFO: Testing Docker environment by running hello-world..."
- docker run --pull always hello-world >/dev/null 2>&1
+ echo "[INFO] Testing Docker environment by running hello-world..."
+ # Use always to test network connection
+ docker run --rm --pull always hello-world:latest >/dev/null 2>&1
Review Comment:
Please specify `hello-world` fixed tag, do not use `latest`
##########
playground.sh:
##########
@@ -177,27 +258,22 @@ docker)
runtime="docker";
;;
*)
- echo "ERROR: please specify which runtime you want to use, available
runtime: [docker|k8s]"
+ echo "[ERROR] please specify which runtime you want to use, available
runtime: [docker|k8s]"
+esac
+
+skipChecks=false
+
+case "$3" in
+--skip-checks)
+ skipChecks=true;
+ ;;
+-s)
+ skipChecks=true;
+ ;;
Review Comment:
Do we need `skipChecks` startup param? It feels too complicated.
I think we must check docker or k8s envorinments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
