Skip to content

Commit

Permalink
Merge pull request juju#18481 from CodingCookieRookie/remove-bash-ssh…
Browse files Browse the repository at this point in the history 
…-key-generation

juju#18481

In the Bash tests for Juju 4.0 we made a change to generate a new ssh key for every new model created. We want to stop this and just use the ssh key that is generated by the Juju client under /home/jenkins/.local/share/juju/ssh. Furthermore, we do not need to specifiy the private SSH key to use for authentication as we have already added the ssh key to the model through `juju add-ssh-key`

## Checklist

~~- [ ] Code style: imports ordered, good names, simple structure, etc~~
~~- [ ] Comments saying why design decisions were made~~
~~- [ ] Go unit tests, with comments saying what you're testing~~
~~- [ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~~
~~- [ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~~

## QA steps

 1. Run all bash tests in tests folder under project root directory.
```sh
$ cd tests
$ ./main.sh -A
```

## Links

**Jira card:** https://warthogs.atlassian.net/browse/JUJU-7258
  • Loading branch information
@jujubot
jujubot authored Dec 9, 2024
2 parents 0588170 + 360bf0d commit 65a4a2d
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 37 deletions.
47 changes: 37 additions & 10 deletions tests/includes/juju.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,19 +408,46 @@ post_add_model() {
fi

if [[ ${BOOTSTRAP_PROVIDER:-} != "k8s" ]]; then
# As of Juju 4.0, the user's SSH keys are no longer added automatically to
# newly created models. So that `juju ssh` works in tests, we should generate
# a new SSH key pair here, and add it to the model.
mkdir -p "${TEST_DIR}/.ssh"
ssh-keygen -t ed25519 -f "$(ssh_key_file "${model}")" -P ""
juju add-ssh-key -m "${model}" "$(cat "$(ssh_key_file "${model}").pub")"
# As of Juju 4.0, the user's SSH keys are no longer added automatically to newly
# created models. To ensure that `juju ssh`` works in tests for newly created models,
# we add the SSH key generated by the Juju client to the model.
add_client_ssh_key_to_juju_model "${model}"
fi
}

# ssh_key_file returns the path to the SSH key for the given model.
ssh_key_file() {
local model=${1}
echo "${TEST_DIR}/.ssh/${model}"
# add_client_ssh_key_to_juju_model adds juju client public SSH key to the specified Juju model
# usage: add_client_ssh_key_to_juju_model <model_name>
add_client_ssh_key_to_juju_model() {
local model_name="$1"
local ssh_key_file="${JUJU_DATA:-${HOME}/.local/share/juju}/ssh/juju_id_ed25519.pub"

# Check if model name is provided
if [[ -z "$model_name" ]]; then
echo "Error: Invalid usage. The function signature should be: add_client_ssh_key_to_juju_model <model_name>"
return
fi

# Check if the SSH key file exists
if [[ ! -f "$ssh_key_file" ]]; then
echo "Error: SSH key file '$ssh_key_file' not found."
return
fi

# Validate SSH key
if ssh-keygen -l -f "${ssh_key_file}" > /dev/null 2>&1; then
output=$(juju add-ssh-key -m "${model_name}" "$(cat "${ssh_key_file}")" 2>&1)
exit_code=$?
if [[ $exit_code -ne 0 ]]; then
echo "Error adding SSH key to model '${model_name}': $output"
return
fi
else
echo "Error: The file '${ssh_key_file}' does not contain a valid SSH public key."
return
fi

echo "SSH key added successfully to model '${model_name}'."
return
}

# destroy_model takes a model name and destroys a model. It first checks if the
Expand Down
8 changes: 4 additions & 4 deletions tests/suites/appdata/appdata.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ EOF

# Check that the token is in /var/run/appdata-sink/token on each
# one.
output=$(juju ssh appdata-sink/0 -i "$(ssh_key_file "appdata-basic")" cat /var/run/appdata-sink/token)
output=$(juju ssh appdata-sink/0 cat /var/run/appdata-sink/token)
check_contains "$output" "appdata-source/0 test-value"

output=$(juju ssh appdata-sink/1 -i "$(ssh_key_file "appdata-basic")" cat /var/run/appdata-sink/token)
output=$(juju ssh appdata-sink/1 cat /var/run/appdata-sink/token)
check_contains "$output" "appdata-source/0 test-value"

juju add-unit appdata-source
Expand All @@ -59,10 +59,10 @@ EOF
wait_for "value2" "$(workload_status appdata-sink 0).message"
wait_for "value2" "$(workload_status appdata-sink 1).message"

output=$(juju ssh appdata-sink/0 -i "$(ssh_key_file "appdata-basic")" cat /var/run/appdata-sink/token)
output=$(juju ssh appdata-sink/0 cat /var/run/appdata-sink/token)
check_contains "$output" "appdata-source/1 value2"

output=$(juju ssh appdata-sink/1 -i "$(ssh_key_file "appdata-basic")" cat /var/run/appdata-sink/token)
output=$(juju ssh appdata-sink/1 cat /var/run/appdata-sink/token)
check_contains "$output" "appdata-source/1 value2"

juju config appdata-source --reset token
Expand Down
8 changes: 4 additions & 4 deletions tests/suites/hooks/reboot.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ run_start_hook_fires_after_reboot() {
# does not fire. In juju 2.9+, we use a unified agent so we need to restart
# the machine agent.
echo "[+] ensuring that implicit start hook does not fire after restarting the (unified) unit agent"
juju ssh juju-qa-test/0 -i "$(ssh_key_file "${model_name}")" 'sudo service jujud-machine-0 restart'
juju ssh juju-qa-test/0 'sudo service jujud-machine-0 restart'
echo
wait_for "$charm" "$(charm_rev "$charm" 22)"
logs=$(juju debug-log --include-module juju.worker.uniter --replay --no-tail | grep -n "reboot detected" || true)
Expand Down Expand Up @@ -60,7 +60,7 @@ run_start_hook_fires_after_reboot() {

# Trigger a reboot and verify that the implicit start hook fires
echo "[+] ensuring that implicit start hook fires after a machine reboot"
juju ssh juju-qa-test/0 -i "$(ssh_key_file "${model_name}")" 'sudo reboot now' || true
juju ssh juju-qa-test/0 'sudo reboot now' || true
sleep 1
wait_for "$charm" "$(idle_condition "$charm")"
echo
Expand Down Expand Up @@ -93,7 +93,7 @@ run_reboot_monitor_state_cleanup() {
# the subordinate. Note: juju ssh adds whitespace which we need to trim
# with a bit of awk magic to ensure that our comparisons work correctly
echo "[+] Verifying that reboot monitor state files are in place"
num_files=$(juju ssh juju-qa-test/0 -i "$(ssh_key_file "${model_name}")" 'ls -1 /var/run/juju/reboot-monitor/ | wc -l' 2>/dev/null | tr -d "[:space:]")
num_files=$(juju ssh juju-qa-test/0 'ls -1 /var/run/juju/reboot-monitor/ | wc -l' 2>/dev/null | tr -d "[:space:]")
echo " | number of monitor state files: ${num_files}"
if [ "$num_files" != "2" ]; then
# shellcheck disable=SC2046
Expand All @@ -107,7 +107,7 @@ run_reboot_monitor_state_cleanup() {
wait_for "juju-qa-test" "$(idle_condition "juju-qa-test" 1)"

wait_for_subordinate_count "juju-qa-test"
num_files=$(juju ssh juju-qa-test/0 -i "$(ssh_key_file "${model_name}")" 'ls -1 /var/run/juju/reboot-monitor/ | wc -l' 2>/dev/null | tr -d "[:space:]")
num_files=$(juju ssh juju-qa-test/0 'ls -1 /var/run/juju/reboot-monitor/ | wc -l' 2>/dev/null | tr -d "[:space:]")
echo " | number of monitor state files: ${num_files}"
if [ "$num_files" != "1" ]; then
# shellcheck disable=SC2046
Expand Down
8 changes: 4 additions & 4 deletions tests/suites/machine/machine.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ test_log_permissions() {

wait_for "started" '.machines."0"."juju-status".current'

check_contains "$(juju ssh 0 -i "$(ssh_key_file "correct-log")" -- stat -c '%G' /var/log/juju/unit-source-0.log)" adm
check_contains "$(juju ssh 0 -i "$(ssh_key_file "correct-log")" -- stat -c '%a' /var/log/juju/unit-source-0.log)" 640
check_contains "$(juju ssh 0 -- stat -c '%G' /var/log/juju/unit-source-0.log)" adm
check_contains "$(juju ssh 0 -- stat -c '%a' /var/log/juju/unit-source-0.log)" 640

check_contains "$(juju ssh 0 -i "$(ssh_key_file "correct-log")" -- stat -c '%a' /var/log/juju/machine-0.log)" 640
check_contains "$(juju ssh 0 -i "$(ssh_key_file "correct-log")" -- stat -c '%G' /var/log/juju/machine-0.log)" adm
check_contains "$(juju ssh 0 -- stat -c '%a' /var/log/juju/machine-0.log)" 640
check_contains "$(juju ssh 0 -- stat -c '%G' /var/log/juju/machine-0.log)" adm

# Clean up!
destroy_model "correct-log"
Expand Down
2 changes: 1 addition & 1 deletion tests/suites/spaces_ec2/juju_bind.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ run_juju_bind() {
add_multi_nic_machine "$hotplug_nic_id" "spaces-juju-bind"

juju_machine_id=$(juju show-machine --format json | jq -r '.["machines"] | keys[0]')
ifaces=$(juju ssh ${juju_machine_id} -i "$(ssh_key_file "spaces-juju-bind")" 'ip -j link' | jq -r '.[].ifname | select(. | startswith("en") or startswith("eth"))')
ifaces=$(juju ssh ${juju_machine_id} 'ip -j link' | jq -r '.[].ifname | select(. | startswith("en") or startswith("eth"))')
primary_iface=$(echo $ifaces | cut -d " " -f1)
hotplug_iface=$(echo $ifaces | cut -d " " -f2)
configure_multi_nic_netplan "$juju_machine_id" "$hotplug_iface" "spaces-juju-bind"
Expand Down
4 changes: 2 additions & 2 deletions tests/suites/spaces_ec2/machines_in_spaces.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ run_machines_in_spaces() {
machine_2_space_ip=$(assert_machine_ip_is_in_cidrs "2" "172.31.254.0/24")

echo "Verify machines can ping each other within and across spaces"
juju ssh 0 -i "$(ssh_key_file "machines-in-spaces")" "ping -c4 ${machine_1_space_ip}"
juju ssh 0 -i "$(ssh_key_file "machines-in-spaces")" "ping -c4 ${machine_2_space_ip}"
juju ssh 0 "ping -c4 ${machine_1_space_ip}"
juju ssh 0 "ping -c4 ${machine_2_space_ip}"

echo "Attempt assigning a container to a different space to it's host machine and assert this fails"
juju add-machine lxd:0 --constraints spaces=isolated
Expand Down
2 changes: 1 addition & 1 deletion tests/suites/spaces_ec2/upgrade_charm_with_bind.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ run_upgrade_charm_with_bind() {
add_multi_nic_machine "$hotplug_nic_id" "spaces-upgrade-charm-with-bind-ec2"

juju_machine_id=$(juju show-machine --format json | jq -r '.["machines"] | keys[0]')
ifaces=$(juju ssh ${juju_machine_id} -i "$(ssh_key_file "spaces-upgrade-charm-with-bind-ec2")" 'ip -j link' | jq -r '.[].ifname | select(. | startswith("en") or startswith("eth"))')
ifaces=$(juju ssh ${juju_machine_id} 'ip -j link' | jq -r '.[].ifname | select(. | startswith("en") or startswith("eth"))')
primary_iface=$(echo $ifaces | cut -d " " -f1)
hotplug_iface=$(echo $ifaces | cut -d " " -f2)
configure_multi_nic_netplan "$juju_machine_id" "$hotplug_iface" "spaces-upgrade-charm-with-bind-ec2"
Expand Down
20 changes: 9 additions & 11 deletions tests/suites/spaces_ec2/util.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
# Create a new machine, wait for it to boot and hotplug a pre-allocated
# network interface which has been tagged: "nic-type: hotpluggable".
add_multi_nic_machine() {
local hotplug_nic_id model_name
local hotplug_nic_id
hotplug_nic_id=$1
model_name=$2

# Ensure machine is deployed to the same az as our nic
az=$(aws ec2 describe-network-interfaces --filters Name=network-interface-id,Values="$hotplug_nic_id" | jq -r ".NetworkInterfaces[0].AvailabilityZone")
Expand All @@ -26,7 +25,7 @@ add_multi_nic_machine() {
timeout=${3:-600} # default timeout: 600s = 10m
start_time="$(date -u +%s)"
while true; do
if juju ssh ${juju_machine_id} -i "$(ssh_key_file "$model_name")" 'test $(ls /sys/class/net | grep "ens\|enp\|eth" | wc -l) -eq 2 && echo done' | grep "done"; then
if juju ssh ${juju_machine_id} 'test $(ls /sys/class/net | grep "ens\|enp\|eth" | wc -l) -eq 2 && echo done' | grep "done"; then
echo "[+] second NIC attached."
break
fi
Expand All @@ -47,28 +46,27 @@ add_multi_nic_machine() {
# restart the machine agent and wait for juju to detect the new interface
# before returning.
configure_multi_nic_netplan() {
local juju_machine_id hotplug_iface model_name
local juju_machine_id hotplug_iface
juju_machine_id=$1
hotplug_iface=$2
model_name=$3

# Add an entry to netplan and apply it so the second interface comes online
echo "[+] updating netplan and restarting machine agent"
# shellcheck disable=SC2086,SC2016
juju ssh ${juju_machine_id} -i "$(ssh_key_file "$model_name")" 'sudo sh -c "sed -i \"/version:/d\" /etc/netplan/50-cloud-init.yaml"'
juju ssh ${juju_machine_id} 'sudo sh -c "sed -i \"/version:/d\" /etc/netplan/50-cloud-init.yaml"'
# shellcheck disable=SC2086,SC2016
default_route=$(juju ssh ${juju_machine_id} -i "$(ssh_key_file "$model_name")" 'ip route | grep default | cut -d " " -f3')
default_route=$(juju ssh ${juju_machine_id} 'ip route | grep default | cut -d " " -f3')
# shellcheck disable=SC2086,SC2016
juju ssh ${juju_machine_id} -i "$(ssh_key_file "$model_name")" "sudo sh -c 'echo \" routes:\n - to: default\n via: ${default_route}\n ${hotplug_iface}:\n dhcp4: true\n version: 2\n\" >> /etc/netplan/50-cloud-init.yaml'"
juju ssh ${juju_machine_id} "sudo sh -c 'echo \" routes:\n - to: default\n via: ${default_route}\n ${hotplug_iface}:\n dhcp4: true\n version: 2\n\" >> /etc/netplan/50-cloud-init.yaml'"

# shellcheck disable=SC2086,SC2016
echo "[+] Reconfiguring netplan:"
juju ssh ${juju_machine_id} -i "$(ssh_key_file "$model_name")" 'sudo cat /etc/netplan/50-cloud-init.yaml'
juju ssh ${juju_machine_id} 'sudo cat /etc/netplan/50-cloud-init.yaml'
# shellcheck disable=SC2086,SC2016
juju ssh ${juju_machine_id} -i "$(ssh_key_file "$model_name")" 'sudo netplan apply' || true
juju ssh ${juju_machine_id} 'sudo netplan apply' || true
echo "[+] Applied"
# shellcheck disable=SC2086,SC2016
juju ssh ${juju_machine_id} -i "$(ssh_key_file "$model_name")" 'sudo systemctl restart jujud-machine-*'
juju ssh ${juju_machine_id} 'sudo systemctl restart jujud-machine-*'

# Wait for the interface to be detected by juju
echo "[+] waiting for juju to detect added NIC"
Expand Down

0 comments on commit 65a4a2d

Please sign in to comment.