-
Notifications
You must be signed in to change notification settings - Fork 10
feat(qa-script): adds qa scripts for mac (arm) developers #1602
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
base: v3
Are you sure you want to change the base?
Conversation
bc7a385 to
fbe7308
Compare
There are two scripts. qa-lxd-mac and qa-lxd-mac-forward.sh. The first spins up a multipass VM, runs jimm within compose inside and adds a controller. The second port-forwards JIMM and keycloak, and is expected to be run AFTER qa-lxd-mac, such that you can connect and QA. The VM has a volume mount and changes will be reflected and updated from the hosts jimm repo.
fbe7308 to
e425cf2
Compare
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.
Pull Request Overview
This pull request adds two QA scripts for Mac (and Linux) developers, designed to easily set up a multipass VM running JIMM with port-forwarding for keycloak.
- qa-lxd-mac.sh spins up a multipass VM, configures mounts, installs dependencies, and sets up the JIMM environment.
- qa-lxd-mac-forward.sh sets up SSH tunnels for JIMM and keycloak thereby enabling local connections.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| local/jimm/qa-lxd-mac.sh | Creates a multipass VM and configures the JIMM environment. |
| local/jimm/qa-lxd-mac-forward.sh | Forwards SSH tunnels for keycloak and JIMM based on specified OS flags. |
Comments suppressed due to low confidence (2)
local/jimm/qa-lxd-mac-forward.sh:51
- The variable '$KEY' is undefined; consider replacing it with '$KEY_PATH' to ensure the correct SSH key path is used.
if [ ! -f "$KEY" ]; then
local/jimm/qa-lxd-mac-forward.sh:61
- Replace '$KEY' with '$KEY_PATH' for consistency when echoing the key path being used.
echo "Using SSH key: $KEY"
|
|
||
| echo "Installing & setting up dependencies" | ||
| multipass exec $VM_NAME -- sudo snap install juju | ||
| multipass exec $VM_NAME -- sudo sudo apt-get -y install make |
Copilot
AI
May 19, 2025
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.
Remove the duplicate 'sudo' to improve command clarity.
| multipass exec $VM_NAME -- sudo sudo apt-get -y install make | |
| multipass exec $VM_NAME -- sudo apt-get -y install make |
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.
Looks good overall, just some minor comments
|
|
||
| echo "Copying ssh key to local" | ||
| KEY_COPIED_PATH="../../local/vm/id_rsa" | ||
| if [ ! -f "$KEY" ]; 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.
Should this be
| if [ ! -f "$KEY" ]; then | |
| if [ ! -f "$KEY_COPIED_PATH" ]; then |
? Or are you trying to allow the user to provide a key via an env var?
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.
Yes! Good spot
| # We want a classic mount to reflect changes on the host to the VM. | ||
| echo "Setting up classic mount" | ||
| multipass umount jimm jimm || true | ||
| multipass mount --type=classic ../../ jimm:jimm || true |
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.
Why the || true? If it fails wouldn't the rest of the script fail to work?
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.
I want the script to continue even if this fails
| multipass exec --working-directory /home/ubuntu/jimm/local/traefik/certs $VM_NAME -- sudo update-ca-certificates | ||
| multipass exec --working-directory /home/ubuntu/jimm $VM_NAME -- make version/commit.txt | ||
| multipass exec --working-directory /home/ubuntu/jimm $VM_NAME -- make version/version.txt | ||
| # TODO(ale8k): Have docker cache images somewhere that can be shared, the compose takes forever otherwise. |
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.
Usually what takes longest is pulling all JIMM dependencies (Juju and others) and then building.
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.
Honestly the pulling of images was slowest for me.
| multipass exec --working-directory /home/ubuntu/jimm $VM_NAME -- docker compose --profile dev up --wait -d | ||
|
|
||
| echo "Building JAAS CLI" | ||
| $(cd ../../ && go build ./cmd/jaas) |
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.
Because of the path assumptions, we should document where the script needs to be run from i.e. it cannot be run from the root-directory of jimm.
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.
I'm gonna switch it into a make target afterwards, just wanted it qa'd a little first.
|
|
||
| echo "To continue the QA environment setup, please login to keycloak." | ||
| echo "The username is: \"jimm-test\" and the password is: \"password\"" | ||
| multipass exec --working-directory /home/ubuntu/jimm $VM_NAME -- juju login jimm.localhost -c jimm-dev |
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.
I wouldn't include this step since the web team don't usually use the Juju CLI afaik
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.
This is to add the controller?
| # ./qa-lxd-mac-forward.sh jimm2 --linux - Run for a VM named jimm2, on linux, and exit immediately | ||
| # ./qa-lxd-mac-forward.sh --linux - Run for the default VM (jimm), on linux, and exit immediately | ||
| # ./qa-lxd-mac-forward.sh jimm2 --mac --wait - Run for a VM named jimm2, on mac, and wait after tunnels are established | ||
|
|
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.
Always a good idea to add set -euo pipefail at the top to ensure errors stop the 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.
True, I'll add Wednesday (this applies to all comments I'll do them Wednesday)
| echo "VM Address: $VM_ADDR" | ||
|
|
||
| # Test SSH connection first (non-blocking, just exits if failure) | ||
| ssh -i "$KEY_COPIED_PATH" -o BatchMode=yes -o ConnectTimeout=5 ubuntu@$VM_ADDR "exit" |
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.
There's a connectTimeout on this one but not on the next.
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.
Rightly or wrongly I just presumed if it's reachable now, it will be immediately afterwards. Can add the timeouts to the others if you'd like?
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 seems very complicate to do all of this. Can you explain why the docker is not working on mac, and maybe we can try to work out that before using multipass, etc, etc.
- should we add a arm64 smoke test to make sure this script keeps working?
| # Set the key path based on OS | ||
| if [[ "$OS" == "mac" ]]; then | ||
| KEY_PATH="$KEY_PATH_MAC" | ||
| elif [[ "$OS" == "linux" ]]; then | ||
| KEY_PATH="$KEY_PATH_LINUX" | ||
| else | ||
| echo "Error: You must specify --linux or --mac" | ||
| exit 1 | ||
| fi |
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.
do you need all of this? For example you could use multipass exec, multipass shell, multipass transfer.
It would read much better adn you don't need the custom rsa key
There are two scripts. qa-lxd-mac and qa-lxd-mac-forward.sh. The first spins up a multipass VM, runs jimm within compose inside and adds a controller. The second port-forwards JIMM and keycloak, and is expected to be run AFTER qa-lxd-mac, such that you can connect and QA. The VM has a volume mount and changes will be reflected and updated from the hosts jimm repo.
This needs QA'ing on a mac before merging. I will update the PR once this has been done.
The exact steps for running this are simple.
And that should be enough to get anybody up and running. I've also added a linux option, for anybody who wishes to run the QA of JIMM within a VM.