canonical-ubuntu-qa team mailing list archive

[Paride Legovini <mp+442546@xxxxxxxxxxxxxxxxxx>]

Review: Needs Information



Diff comments:

> diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances
> old mode 100755
> new mode 100644
> 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..13a6eef
> --- /dev/null
> +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/update_ubuntu_csv
> @@ -0,0 +1,112 @@
> +#!/bin/bash
> +
> +set -e
> +
> +increment_date(){
> +        DAY=$(printf "%s" "${1}" | cut -d'-' -f3)
> +        MONTH=$(printf "%s" "${1}" | cut -d'-' -f2)
> +        YEAR=$(printf "%s" "${1}" | cut -d'-' -f1)
> +        NEW_DATE=$(date --date="${MONTH}/${DAY}/${YEAR}"''"${2}"'' +'%Y-%m-%d')
> +        printf "%s" "${NEW_DATE}"
> +}
> +
> +autocomplete_dates(){
> +        THIS_RELEASE=$1
> +        CSV_PATH=$2
> +        RELEASE_ID=$(printf "%s" "${THIS_RELEASE}" | cut -d',' -f1)
> +        LATEST_RELEASE=$(tail -1 "${CSV_PATH}")
> +        LATEST_RELEASE_DATE=$(printf "%s" "${LATEST_RELEASE}" | cut -d',' -f5)
> +        THIS_RELEASE="${THIS_RELEASE},${LATEST_RELEASE_DATE}"
> +        NEW_RELEASE_DATE=$(increment_date "${LATEST_RELEASE_DATE}" "+6 months")
> +        THIS_RELEASE="${THIS_RELEASE},${NEW_RELEASE_DATE}"
> +        if [[ "${RELEASE_ID}" =~ .*"LTS".* ]]; then
> +                NEW_EOL_DATE=$(increment_date "${NEW_RELEASE_DATE}" "+5 years +5days")
> +                THIS_RELEASE="${THIS_RELEASE},${NEW_EOL_DATE},${NEW_EOL_DATE}"
> +                NEW_ESM_DATE=$(increment_date "${NEW_RELEASE_DATE}" "+10 years")
> +                THIS_RELEASE="${THIS_RELEASE},${NEW_ESM_DATE}"
> +        else
> +                NEW_EOL_DATE=$(increment_date "${NEW_RELEASE_DATE}" "+9 months")
> +                THIS_RELEASE="${THIS_RELEASE},${NEW_EOL_DATE}"
> +        fi 
> +        printf "%s" "${THIS_RELEASE}"
> +}
> +
> +CSV_PATH="/usr/share/distro-info/ubuntu.csv"
> +
> +HELP_STRING="#==========================================================================================================#
> +This script is used to add a new release to ${CSV_PATH} on all of the machines in
> +the autopkgtest-cloud environment. The dates autocomplete so there is no need to input them.
> +The user HAS to input the release following the most recent one in ${CSV_PATH},
> +as the dates are autocompleted based on the latest release.
> +
> +Usage (LTS):
> +version,codename,series
> +Example (LTS):
> +./update_ubuntu_csv \"24.04 LTS,Noble Newt,noble\"
> +Usage (non-LTS):
> +version,codename,series
> +Example (non-LTS):
> +./update_ubuntu_csv \"24.10,Ostracized Octopus,ostracized\"
> +
> +In case the user of this script messes up and adds an incorrect line to ${CSV_PATH},
> +please run the following:
> +./update_ubuntu_csv RESTORE
> +which will restore the ${CSV_PATH} file on all the machines in the environment to
> +the file on the bastion.
> +
> +The file created on the remote machine is the file on the bastion, appended with the user input, not the
> +file from the remote machine
> +#==========================================================================================================#
> +"
> +
> +
> +if [[ $# -ne 1 ]]; then
> +        printf "Number of input arguments wrong, please check them.\n"
> +        exit 1
> +fi
> +
> +if [[ "${1}" == "-h" || "${1}" == "--help" ]]; then
> +        printf "%s" "${HELP_STRING}"
> +        exit 0
> +fi
> +
> +RELEASE_TO_ADD=$(autocomplete_dates "${1}" "${CSV_PATH}")
> +
> +RELEASE_ID=$(echo "${RELEASE_TO_ADD}" | cut -d',' -f1)
> +
> +if [[ $(cat "${CSV_PATH}") =~ .*"${RELEASE_ID}".* ]]; then
> +        printf "This release already exists. Check state of %s on the bastion, and try again." "${CSV_PATH}"
> +        exit 1
> +fi
> +
> +TMP_CSV=$(mktemp)
> +cp "${CSV_PATH}" "${TMP_CSV}"
> +
> +if [[ "${RELEASE_TO_ADD}" != "RESTORE" ]]; then
> +        printf "%s" "${RELEASE_TO_ADD}" >> "${TMP_CSV}"
> +fi
> +
> +juju machines --format=json | jq '.machines' > /tmp/machines.json

Brian and me suggested replacing all this dumping to file and parsing with something like:

machines=$(juju machines --format=json | jq -r '.machines | keys[]')
for machine in $machines; do
   ...
done

but you are avoiding that. I'm fine if the suggestion is bad, but please reply to the review comments if you have reasons to disagree with them! Once you push to the branch they're lost...

> +jq 'keys' /tmp/machines.json > /tmp/machine_keys.json
> +
> +jq -c '.[]' /tmp/machine_keys.json | while read -r 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}"
> +        printf "Copying file to machine: %s\n" "${MACHINE}"
> +        juju scp "${TMP_CSV}" "${MACHINE}":"${TMP_CSV}"
> +        juju ssh "${MACHINE}" "sudo bash -c 'mv ${TMP_CSV} ${CSV_PATH}'"
> +done
> +
> +printf "========================================================================\n"
> +printf "All done!\nFYI, this is the last few lines of the file you've copied across: \n\n"
> +tail -3 "${TMP_CSV}"
> +printf "\n\n"
> +rm "${TMP_CSV}"


-- 
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.