canonical-ubuntu-qa team mailing list archive

[Brian Murray <mp+442546@xxxxxxxxxxxxxxxxxx>]

This looks great. I have just a couple of small in-line comments that you can feel free to address or ignore.

Diff comments:

> diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/update_ubuntu_csv b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/update_ubuntu_csv
> new file mode 100755
> index 0000000..59ec51a
> --- /dev/null
> +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/update_ubuntu_csv
> @@ -0,0 +1,86 @@
> +#!/bin/bash
> +
> +HELP_STRING='''#==========================================================================================================#
> +This script is used to add a new release to /usr/share/distro-info/ubuntu.csv on all of the machines in
> +the autopkgtest-cloud environment.
> +
> +Usage (LTS):
> +version,codename,series,created,release,eol,eol-server,eol-esm
> +Example (LTS):
> +./update_ubuntu_csv "24.04 LTS,Noble Narwhal,noble,2023-10-19,2024-04-20,2029-04-20,2029-04-20,2034-04-20"

It's not important but we already had a Narwhal release sometime ago. (I'm afraid to look how long ago.)

> +Usage (non-LTS):
> +version,codename,series,created,release,eol
> +Example (non-LTS):
> +./update_ubuntu_csv "24.10,Ostracized Octopus,ostracized,2024-04-20,2024-10-19,2025-12-10"
> +
> +Incase the user of this script messes up and adds an incorrect line to /usr/share/disto-info/ubuntu.csv,

"In case"

> +please run the following:
> +./update_ubuntu_csv RESTORE
> +which will restore the /usr/share/distro-info/ubuntu.csv file on all the machines in the environment to
> +the file on the bastion.
> +
> +The file created on the cloud machine is the file on the bastion, appended with the user input, not the
> +file from the remote machine

"remote machine."

> +#==========================================================================================================#
> +'''
> +
> +set -e
> +
> +ORIG_CSV=$(cat /usr/share/distro-info/ubuntu.csv)

Its a small optimization but you might move this down past the early exits.

> +
> +if [[ $# -ne 1 ]]; then
> +        printf "Number of input arguments wrong, please check them.\n"
> +        exit 1
> +fi
> +
> +if [[ "${1}" == "-h" ]]; then
> +        printf "%s" "${HELP_STRING}"
> +        exit 0
> +elif [[ "${1}" == "--help" ]]; then
> +        printf "%s" "${HELP_STRING}"
> +        exit 0
> +fi
> +
> +RELEASE_TO_ADD="${1}"
> +RELEASE_ID=$(echo "${RELEASE_TO_ADD}" | cut -d',' -f1)
> +
> +if [[ "${ORIG_CSV}" =~ .*"${RELEASE_ID}".* ]]; then
> +        printf "This release already exists. Check state of /usr/share/distro-info/ubuntu.csv on the bastion, and try again."
> +        exit 1
> +fi
> +
> +if [[ "${RELEASE_TO_ADD}" == "RESTORE" ]]; then
> +        printf "%s" "${ORIG_CSV}" > "${HOME}"/ubuntu.csv
> +else
> +        printf "%s\n%s" "${ORIG_CSV}" "${RELEASE_TO_ADD}" > "${HOME}"/ubuntu.csv
> +fi
> +
> +juju machines --format=json | jq '.machines' > /tmp/machines.json
> +jq 'keys' /tmp/machines.json > /tmp/machine_keys.json
> +
> +jq -c '.[]' /tmp/machine_keys.json | while read i; do
> +        MACHINE_ID=$(printf "%s" "${i}" | sed 's/"//g')
> +        printf "%s " "${MACHINE_ID}" >> /tmp/machine_nums
> +done
> +
> +ITERATE_ME=$(cat /tmp/machine_nums)
> +rm /tmp/machine_nums /tmp/machines.json /tmp/machine_keys.json
> +
> +for MACHINE in $ITERATE_ME; do
> +        printf "========================================================================\n"
> +        printf "Checking validity of machine: %s\n" "${MACHINE}"
> +        CHECK_MACHINES=$(juju show-machine "${MACHINE}")
> +        if [[ "${CHECK_MACHINES}" =~ .*"machines: {}".* ]]; then
> +                printf "Invalid machine %s\n" "${MACHINE}"
> +        else
> +                printf "Copying file to machine: %s\n" "${MACHINE}"
> +                juju scp "${HOME}"/ubuntu.csv "${MACHINE}":/home/ubuntu
> +                juju ssh "${MACHINE}" "sudo bash -c 'mv /home/ubuntu/ubuntu.csv /usr/share/distro-info/ubuntu.csv'"
> +        fi
> +done
> +
> +printf "========================================================================\n"
> +printf "All done!\nFYI, this is the contents of the file you've copied across: \n\n"

I appreciate the FYI addition but I wonder if we really want to see the whole file. Would it make more sense to only display the last couple of lines?

> +cat "${HOME}"/ubuntu.csv
> +printf "\n\n"
> +rm "${HOME}"/ubuntu.csv
> \ No newline at end of file


-- 
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/442546
Your team Canonical's Ubuntu QA is subscribed to branch autopkgtest-cloud:master.