Skip to content

Conversation

@mplsgrant
Copy link
Collaborator

@mplsgrant mplsgrant commented Aug 1, 2024

Issue

#417

Solution

The commits detail each solution, but broadly speaking they check for and run Docker Desktop if minikube is unavailable or if minikube has a known broken version on Mac. Otherwise, the script still treats minikube as the happy path. Looking for feedback on this approach.

  • Check if docker is running
  • When users follow install minikube instructions, they end up doinging minikube start twice because the minikube installation instructions on the web tell the user to do minikube start

@willcl-ark
Copy link
Contributor

Approach seems good to me! Thanks

@mplsgrant mplsgrant force-pushed the 2024-07-quickstart-macos branch from dc4c986 to cae92a9 Compare August 1, 2024 16:01
@willcl-ark
Copy link
Contributor

willcl-ark commented Aug 1, 2024

I wonder, might it be more useful to do something like:

Diff
diff --git a/resources/scripts/quick_start.sh b/resources/scripts/quick_start.sh
index 5105686..a604258 100755
--- a/resources/scripts/quick_start.sh
+++ b/resources/scripts/quick_start.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 set -euo pipefail
 
+ERROR_CODE=0
 
 is_cygwin_etal() {
     uname -s | grep -qE "CYGWIN|MINGW|MSYS"
@@ -10,7 +11,7 @@ is_wsl() {
 }
 if is_cygwin_etal || is_wsl; then
     echo "Quick start does not support Windows"
-    exit 1
+    ERROR_CODE=1
 fi
 
 
@@ -72,7 +73,7 @@ elif [[ "$(uname)" == "Darwin" ]]; then
 else
     print_partial_message " 💥 Could not find " "$current_user" " in the docker group. Please add it like this..." "$BOLD"
     print_message "" "   sudo usermod -aG docker $current_user && newgrp docker" "$BOLD"
-    exit 1
+    ERROR_CODE=1
 fi
 
 minikube_path=$(command -v minikube || true)
@@ -82,14 +83,14 @@ if [[ "$(uname)" == "Darwin" ]] && command -v minikube &> /dev/null && [[ "$(min
     else
         print_partial_message " 💥 Could not find " "minikube version > 1.33.1" ". Please upgrade..." "$BOLD"
         print_message "" "   https://minikube.sigs.k8s.io/docs/start/" "$BOLD"
-        exit 127
+        ERROR_CODE=127
     fi
 elif [ -n "$minikube_path" ]; then
     print_partial_message " ⭐️ Found " "minikube" ": $minikube_path " "$BOLD"
 else
     print_partial_message " 💥 Could not find " "minikube" ". Please follow this link to install it..." "$BOLD"
     print_message "" "   https://minikube.sigs.k8s.io/docs/start/" "$BOLD"
-    exit 127
+    ERROR_CODE=127
 fi
 
 kubectl_path=$(command -v kubectl || true)
@@ -98,7 +99,7 @@ if [ -n "$kubectl_path" ]; then
 else
     print_partial_message " 💥 Could not find " "kubectl" ". Please follow this link to install it..." "$BOLD"
     print_message "" "   https://kubernetes.io/docs/tasks/tools/" "$BOLD"
-    exit 127
+    ERROR_CODE=127
 fi
 
 helm_path=$(command -v helm || true)
@@ -107,7 +108,7 @@ if [ -n "$helm_path" ]; then
 else
     print_partial_message " 💥 Could not find " "helm" ". Please follow this link to install it..." "$BOLD"
     print_message "" "   https://helm.sh/docs/intro/install/" "$BOLD"
-    exit 127
+    ERROR_CODE=127
 fi
 
 just_path=$(command -v just || true)
@@ -116,7 +117,7 @@ if [ -n "$just_path" ]; then
 else
     print_partial_message " 💥 Could not find " "just" ". Please follow this link to install it..." "$BOLD"
     print_message "" "   https://github.com/casey/just?tab=readme-ov-file#pre-built-binaries" "$BOLD"
-    exit 127
+    ERROR_CODE=127
 fi
 
 python_path=$(command -v python3 || true)
@@ -125,7 +126,7 @@ if [ -n "$python_path" ]; then
 else
     print_partial_message " 💥 Could not find " "python3" ". Please follow this link to install it (or use your package manager)..." "$BOLD"
     print_message "" "   https://www.python.org/downloads/" "$BOLD"
-    exit 127
+    ERROR_CODE=127
 fi
 
 venv_status=$(python3 -m venv --help || true)
@@ -133,9 +134,13 @@ if [ -n "$venv_status" ]; then
     print_partial_message " ⭐️ Found " "venv" ": a python3 module" "$BOLD"
 else
     print_partial_message " 💥 Could not find " "venv" ". Please install it using your package manager." "$BOLD"
-    exit 127
+    ERROR_CODE=127
 fi
 
+if [ $ERROR_CODE -ne 0 ]; then
+    print_message "" "There were errors in the setup process. Please fix them and try again." "$BOLD"
+    exit $ERROR_CODE
+fi
 
 print_message "" "" ""
 print_message "" "    Let's try to spin up a python virtual environment..." ""

This gives us an output like this (which still appears to have a minikube issue... edit: I see, current version is the bad version!)

~/src/warnet on  2024-07-quickstart-macos [$!?] is 📦 v0.9.11 : 🐍 (warnet)
$ resources/scripts/quick_start.sh

   ╭───────────────────────────────────╮
   │  Welcome to the Warnet Quickstart │
   ╰───────────────────────────────────╯

    Let's find out if your system has what it takes to run Warnet...

 ⭐️ Found docker: /usr/local/bin/docker
 ⭐️ Running Docker on Darwin
 💥 Could not find minikube version > 1.33.1. Please upgrade...
   https://minikube.sigs.k8s.io/docs/start/
 ⭐️ Found kubectl: /usr/local/bin/kubectl
 💥 Could not find helm. Please follow this link to install it...
   https://helm.sh/docs/intro/install/
 ⭐️ Found just: /usr/local/bin/just
 ⭐️ Found python3: /Users/will/src/warnet/.venv/bin/python3
 ⭐️ Found venv: a python3 module
There were errors in the setup process. Please fix them and try again.

~/src/warnet on  2024-07-quickstart-macos [$!?] is 📦 v0.9.11 : 🐍 (warnet)
✗ minikube version
minikube version: v1.33.1

fi

current_user=$(whoami)
current_context=$(docker context show)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm getting an error here

--> docker context show
default

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What happens when you run uname?

Is the error that it "could not find in the docker group"?

if docker context ls | grep -q "docker-desktop"; then
print_message " ⭐️ Found " "Docker Desktop" "$BOLD"
else
print_partial_message " 💥 Could not find " "minikube version > 1.33.1" ". Please upgrade..." "$BOLD"
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw this error too, I thought it was going to be OK to use docker desktop instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What does docker context ls return?

Also, I have now added a check for Docker Desktop.

@bdp-DrahtBot
Copy link
Collaborator

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #419 (warcli cluster refactors by willcl-ark)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@mplsgrant mplsgrant force-pushed the 2024-07-quickstart-macos branch from c31318d to 990473f Compare August 9, 2024 21:49
@willcl-ark
Copy link
Contributor

Let's perhaps add these checks to a top-level warcli start command. This would make the workflow for a user:

warcli start
warcli cluster deploy
warcli network start

(with optionally a warcli cluster setup-minikube command if using minikube)?

@mplsgrant mplsgrant self-assigned this Aug 19, 2024
@mplsgrant mplsgrant force-pushed the 2024-07-quickstart-macos branch from 9f18036 to 29e95e1 Compare August 19, 2024 09:40
@mplsgrant mplsgrant marked this pull request as draft August 19, 2024 10:17
@mplsgrant mplsgrant force-pushed the 2024-07-quickstart-macos branch from 1be0e8e to b82fa13 Compare August 19, 2024 13:28
@mplsgrant mplsgrant marked this pull request as ready for review August 19, 2024 13:32
@mplsgrant mplsgrant requested a review from willcl-ark August 19, 2024 13:33
@mplsgrant mplsgrant marked this pull request as draft August 19, 2024 13:43
@mplsgrant mplsgrant marked this pull request as ready for review August 19, 2024 13:53
@mplsgrant mplsgrant force-pushed the 2024-07-quickstart-macos branch from b82fa13 to 2896a65 Compare August 19, 2024 14:31
@adamjonas adamjonas added this to the Tabconf milestone Aug 20, 2024
@mplsgrant
Copy link
Collaborator Author

YAML fixes this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants