Merge ~paride/autopkgtest-cloud:lint-shellscripts into autopkgtest-cloud:master

Proposed by Paride Legovini
Status: Merged
Approved by: Tim Andersson
Approved revision: ef73341a1cf9f455d77cc93d9b58c45530253045
Merged at revision: ef73341a1cf9f455d77cc93d9b58c45530253045
Proposed branch: ~paride/autopkgtest-cloud:lint-shellscripts
Merge into: autopkgtest-cloud:master
Diff against target: 341 lines (+61/-52)
12 files modified
.launchpad.yaml (+8/-0)
.pre-commit-config.yaml (+5/-0)
charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image (+11/-10)
charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release (+19/-20)
charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances (+1/-1)
charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair (+5/-4)
charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region (+2/-1)
charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh (+2/-0)
ci/lint_test (+0/-8)
lxc-slave-admin/cmd (+5/-5)
mojo/make-lxd-secgroup (+1/-1)
mojo/postdeploy (+2/-2)
Reviewer Review Type Date Requested Status
Tim Andersson Approve
Review via email: mp+445422@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paride Legovini (paride) wrote :

WIP branch to work at linting with Tim.

Revision history for this message
Tim Andersson (andersson123) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/.launchpad.yaml b/.launchpad.yaml
2index b08e2c3..a744b71 100755
3--- a/.launchpad.yaml
4+++ b/.launchpad.yaml
5@@ -1,8 +1,16 @@
6 pipeline:
7+ - pre_commit
8 - build_charms
9 - lint_test
10
11 jobs:
12+ pre_commit:
13+ series: jammy
14+ architectures: amd64
15+ packages:
16+ - git
17+ - pre-commit
18+ run: pre-commit run --all-files --show-diff-on-failure
19 build_charms:
20 series: focal
21 architectures: amd64
22diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
23new file mode 100644
24index 0000000..eb05b79
25--- /dev/null
26+++ b/.pre-commit-config.yaml
27@@ -0,0 +1,5 @@
28+repos:
29+ - repo: https://github.com/shellcheck-py/shellcheck-py
30+ rev: v0.9.0.5
31+ hooks:
32+ - id: shellcheck
33diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image
34index be6b4d3..a0ad87a 100755
35--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image
36+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/build-adt-image
37@@ -1,10 +1,11 @@
38 #!/bin/bash
39 # Build adt cloud images with create-nova-image-new-release for the given
40 # cloud, release and arch
41+# shellcheck disable=SC1090
42
43 set -eu
44
45-IFS="[- ]" read -r RELEASE REGION ARCH bootstrap <<< "$@"
46+IFS="[- ]" read -r RELEASE REGION ARCH _bootstrap <<< "$@"
47
48 if [ -z "${RELEASE}" ] || [ -z "${REGION}" ] || [ -z "${ARCH}" ]; then
49 echo "Usage: $0 RELEASE REGION ARCH" >&2
50@@ -12,8 +13,8 @@ if [ -z "${RELEASE}" ] || [ -z "${REGION}" ] || [ -z "${ARCH}" ]; then
51 fi
52
53 if [ -z "${MIRROR:-}" ]; then
54- if [ -e ~/mirror-${REGION}.rc ]; then
55- . ~/mirror-${REGION}.rc
56+ if [ -e ~/mirror-"${REGION}".rc ]; then
57+ . ~/mirror-"${REGION}".rc
58 else
59 . ~/mirror.rc
60 fi
61@@ -24,10 +25,10 @@ export MIRROR
62 export NET_NAME
63
64 if [ -z "${USE_CLOUD_CONFIG_FROM_ENV:-}" ]; then
65- if [ -e ~/cloudrcs/${REGION}-${ARCH}.rc ]; then
66- . ~/cloudrcs/${REGION}-${ARCH}.rc
67+ if [ -e ~/cloudrcs/"${REGION}"-"${ARCH}".rc ]; then
68+ . ~/cloudrcs/"${REGION}"-"${ARCH}".rc
69 else
70- . ~/cloudrcs/${REGION}.rc
71+ . ~/cloudrcs/"${REGION}".rc
72 fi
73 fi
74
75@@ -73,11 +74,11 @@ fi
76
77 echo "$REGION-$ARCH: using image $IMG"
78 KEYNAME=${KEYNAME:-testbed-$(hostname)}
79-$(dirname $0)/create-nova-image-new-release $RELEASE $ARCH $IMG "${KEYNAME}" "$IMAGE_NAME"
80+"$(dirname "${0}")/create-nova-image-new-release" "${RELEASE}" "${ARCH}" "${IMG}" "${KEYNAME}" "${IMAGE_NAME}"
81 # clean old images
82-openstack image list --private -f value | grep --color=none -v "$IMAGE_NAME" | while read id img state; do
83- if $(echo ${img} | grep -qs "adt/ubuntu-${RELEASE}-${ARCH}") && [ ${state} = active ]; then
84+openstack image list --private -f value | grep --color=none -v "$IMAGE_NAME" | while read -r id img state; do
85+ if echo "${img}" | grep -qs "adt/ubuntu-${RELEASE}-${ARCH}" && [ "${state}" = active ]; then
86 echo "Cleaning up old image $img ($id)"
87- openstack image delete $id
88+ openstack image delete "${id}"
89 fi
90 done
91diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release
92index bc51ff1..349c217 100755
93--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release
94+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-nova-image-new-release
95@@ -1,6 +1,7 @@
96 #!/bin/bash
97 # create an autopkgtest nova image for a new release, based on a generic image
98 # Author: Martin Pitt <martin.pitt@ubuntu.com>
99+# shellcheck disable=SC2154
100 set -eu
101 RELEASE="${1:-}"
102 ARCH="${2:-}"
103@@ -48,9 +49,9 @@ else
104 fi
105
106 # unbreak my server option :-(
107-userdata=`mktemp`
108-trap "rm $userdata" EXIT TERM INT QUIT PIPE
109-cat <<EOF > $userdata
110+userdata=$(mktemp)
111+trap 'rm ${userdata}' EXIT TERM INT QUIT PIPE
112+cat <<EOF > "${userdata}"
113 #cloud-config
114
115 manage_etc_hosts: true
116@@ -67,13 +68,13 @@ EOF
117
118 # create new instance
119 INSTNAME="${BASEIMG}-adt-prepare"
120-eval "$(openstack network show -f shell ${NET_NAME})"
121+eval "$(openstack network show -f shell "${NET_NAME}")"
122
123-NET_ID=${id}
124+NET_ID="${id}"
125
126 retries=20
127 while true; do
128- eval "$(openstack server create -f shell --flavor autopkgtest --image $BASEIMG --user-data $userdata --key-name $KEYNAME --wait $INSTNAME --nic net-id=${NET_ID})"
129+ eval "$(openstack server create -f shell --flavor autopkgtest --image "${BASEIMG}" --user-data "${userdata}" --key-name "${KEYNAME}" --wait "${INSTNAME}" --nic net-id="${NET_ID}")"
130 if openstack server show "${id}" >/dev/null 2>/dev/null; then
131 break
132 fi
133@@ -90,27 +91,25 @@ done
134
135 SRVID="${id}"
136
137-trap "openstack server delete ${SRVID}" EXIT TERM INT QUIT PIPE
138+trap 'openstack server delete ${SRVID}' EXIT TERM INT QUIT PIPE
139
140 # determine IP address
141-eval "$(openstack server show -f shell ${SRVID})"
142-ipaddr=$(echo ${addresses} | awk 'BEGIN { FS="=" } { print $2 }')
143+eval "$(openstack server show -f shell "${SRVID}")"
144+ipaddr=$(echo "${addresses}" | awk 'BEGIN { FS="=" } { print $2 }')
145
146 SSH_CMD="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no ubuntu@$ipaddr"
147 echo "Waiting for ssh (may cause some error messages)..."
148 timeout 300 sh -c "while ! $SSH_CMD true; do sleep 5; done"
149
150 echo "Waiting until cloud-init is done..."
151-timeout 25m $SSH_CMD 'while [ ! -e /var/lib/cloud/instance/boot-finished ]; do sleep 1; done'
152+timeout 25m "${SSH_CMD}" 'while [ ! -e /var/lib/cloud/instance/boot-finished ]; do sleep 1; done'
153
154 echo "Running setup script..."
155-cat "${SETUP_TESTBED}" | $SSH_CMD "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -"
156+"${SSH_CMD}" "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -" < "${SETUP_TESTBED}"
157
158 echo "Running Canonical setup script..."
159-CANONICAL_SCRIPT=$(dirname $(dirname $(readlink -f $0)))/worker-config-production/setup-canonical.sh
160-cat "$CANONICAL_SCRIPT" | $SSH_CMD "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -"
161-
162-arch=$($SSH_CMD dpkg --print-architecture)
163+CANONICAL_SCRIPT="$(dirname "$(dirname "$(readlink -f "${0}")")")"/worker-config-production/setup-canonical.sh
164+"${SSH_CMD}" "sudo env MIRROR='${MIRROR:-}' RELEASE='$RELEASE' sh -" < "${CANONICAL_SCRIPT}"
165
166 echo "Check that the upgraded image boots..."
167 while true; do
168@@ -138,10 +137,10 @@ $SSH_CMD sudo journalctl --rotate --vacuum-time=12h || true
169
170 echo "Powering off to get a clean file system..."
171 $SSH_CMD sudo poweroff || true
172-eval "$(openstack server show -f shell ${SRVID})"
173-while [ ${os_ext_sts_vm_state} != "stopped" ]; do
174+eval "$(openstack server show -f shell "${SRVID}")"
175+while [ "${os_ext_sts_vm_state}" != "stopped" ]; do
176 sleep 1
177- eval "$(openstack server show -f shell ${SRVID})"
178+ eval "$(openstack server show -f shell "${SRVID}")"
179 done
180
181 echo "Creating image $IMAGE_NAME ..."
182@@ -155,8 +154,8 @@ while true; do
183 while [ $inner_retries -gt 0 ]; do
184 # server image create often loses its connection but it's actually
185 # working - if the image is uploading, wait a bit for it to finish
186- eval $(openstack image show -f shell --prefix=image_ "${IMAGE_NAME}")
187- eval $(openstack server show -f shell --prefix=server_ "${SRVID}")
188+ eval "$(openstack image show -f shell --prefix=image_ "${IMAGE_NAME}")"
189+ eval "$(openstack server show -f shell --prefix=server_ "${SRVID}")"
190 if [ "${server_os_ext_sts_task_state}" = "image_uploading" ] ||
191 [ "${image_status}" = "saving" ]; then
192 echo "image ${IMAGE_NAME} is uploading, waiting..." >&2
193diff --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
194index 6730c03..450ea83 100755
195--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances
196+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/create-test-instances
197@@ -13,5 +13,5 @@ IMAGE=$(openstack image list | grep "adt/ubuntu-$DEVEL-$ARCH" | cut -d' ' -f2)
198 NET_ID=$(openstack network list | grep 'net_prod-proposed-migration' | cut -d' ' -f2)
199
200 for i in $(seq 1 10); do
201- openstack server create --image $IMAGE --flavor cpu4-ram8-disk50 --nic net-id=$NET_ID -- "creation-test-$ARCH-$i"
202+ openstack server create --image "${IMAGE}" --flavor cpu4-ram8-disk50 --nic net-id="${NET_ID}" -- "creation-test-$ARCH-$i"
203 done
204diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair
205index be664d6..4ef1daa 100755
206--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair
207+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/ensure-keypair
208@@ -2,12 +2,13 @@
209
210 set -eu
211
212-IFS="[- ]" read -r RELEASE REGION ARCH bootstrap <<< "$@"
213+IFS="[- ]" read -r _RELEASE REGION ARCH _bootstrap <<< "$@"
214
215-if [ -e ~/cloudrcs/${REGION}-${ARCH}.rc ]; then
216- . ~/cloudrcs/${REGION}-${ARCH}.rc
217+# shellcheck disable=SC1090
218+if [ -e ~/cloudrcs/"${REGION}"-"${ARCH}".rc ]; then
219+ . ~/cloudrcs/"${REGION}"-"${ARCH}".rc
220 else
221- . ~/cloudrcs/${REGION}.rc
222+ . ~/cloudrcs/"${REGION}".rc
223 fi
224
225 if ! [ -e "${HOME}/.ssh/id_rsa" ]; then
226diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region
227index 0261108..2e78e83 100755
228--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region
229+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/exec-in-region
230@@ -1,5 +1,6 @@
231 #!/bin/sh
232 # usage: exec-in-region <region name> <command> <argument>...
233+# shellcheck disable=SC1090
234
235 set -e
236
237@@ -25,7 +26,7 @@ export REGION
238 if [ "${REGION#lxd-}" != "$REGION" ]; then
239 LXD_ARCH=${REGION#*-}; LXD_ARCH=${LXD_ARCH%%-*}
240 else
241- . ${HOME}/cloudrcs/${REGION}.rc
242+ . "${HOME}"/cloudrcs/"${REGION}".rc
243 fi
244
245 exec "$@"
246diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh
247index 93d48d8..7cd94c8 100644
248--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh
249+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker-config-production/setup-canonical.sh
250@@ -1,3 +1,4 @@
251+#!/bin/sh
252 # Canonical/Ubuntu specific testbed setup
253
254 # Remove trailing dot from the machine fqdn.
255@@ -70,6 +71,7 @@ if type iptables >/dev/null 2>&1; then
256 iptables -w -t mangle -A FORWARD -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --clamp-mss-to-pmtu || true
257 EOF
258 chmod 755 /etc/rc.local
259+ # shellcheck disable=SC1091
260 . /etc/rc.local
261 fi
262
263diff --git a/ci/lint_test b/ci/lint_test
264index e52edc4..15fc01d 100755
265--- a/ci/lint_test
266+++ b/ci/lint_test
267@@ -94,14 +94,6 @@ if __name__=="__main__":
268 "output": "",
269 "code": 0
270 },
271- "shellcheck": {
272- "files": [],
273- "extensions": [".sh", ".bash"],
274- "shebangs": ["#!/bin/bash", "#!/bin/sh"],
275- "args": None,
276- "output": "",
277- "code": 0
278- },
279 'yamllint': {
280 "files": ["../"],
281 "extensions": None,
282diff --git a/lxc-slave-admin/cmd b/lxc-slave-admin/cmd
283index ff991ff..0700964 100755
284--- a/lxc-slave-admin/cmd
285+++ b/lxc-slave-admin/cmd
286@@ -1,6 +1,6 @@
287 #!/bin/sh
288 set -e
289-MYDIR=`dirname $0`
290+MYDIR=$(dirname "${0}")
291
292 if [ -z "$1" ]; then
293 echo "Usage: $0 <hosts> <commands or .commands file>" >&2
294@@ -8,11 +8,11 @@ if [ -z "$1" ]; then
295 fi
296
297 if [ "$1" = "all" ]; then
298- for f in $MYDIR/*.hosts; do
299+ for f in "${MYDIR}"/*.hosts; do
300 hosts="$hosts -h $f";
301 done
302 else
303- if [ -e ${1} ]; then
304+ if [ -e "${1}" ]; then
305 hosts="-h ${1}"
306 elif [ -e "${1}.hosts" ]; then
307 hosts="-h ${1}.hosts"
308@@ -29,8 +29,8 @@ if [ "${1%.commands}" != "$1" ]; then
309 exit 1
310 fi
311 # command file
312- cat "$1" | parallel-ssh -x "-F $MYDIR/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes $hosts -p8 -t 0 -i -I
313+ parallel-ssh -x "-F ${MYDIR}/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes "${hosts}" -p8 -t 0 -i -I < "${1}"
314 else
315 # command
316- parallel-ssh -x "-F $MYDIR/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes $hosts -p8 -t 0 -i -- "$@"
317+ parallel-ssh -x "-F ${MYDIR}/ssh_config" -OUserKnownHostsFile=/dev/null -OStrictHostKeyChecking=no -OIdentitiesOnly=yes "${hosts}" -p8 -t 0 -i -- "$@"
318 fi
319diff --git a/mojo/make-lxd-secgroup b/mojo/make-lxd-secgroup
320index d41274c..3199669 100755
321--- a/mojo/make-lxd-secgroup
322+++ b/mojo/make-lxd-secgroup
323@@ -1,5 +1,5 @@
324 #!/bin/sh
325-
326+# shellcheck disable=SC1090
327 set -eu
328
329 # there's apparently no way to get this dynamically
330diff --git a/mojo/postdeploy b/mojo/postdeploy
331index 0f857ae..79fe50a 100755
332--- a/mojo/postdeploy
333+++ b/mojo/postdeploy
334@@ -11,5 +11,5 @@ if [ "${MOJO_STAGE_NAME}" == "staging" ]; then
335 fi
336
337 echo "Setting up the floating IP address of the front end..."
338-$(dirname $0)/add-floating-ip haproxy
339-$(dirname $0)/add-floating-ip rabbitmq-server
340+"$(dirname "$0")/add-floating-ip" haproxy
341+"$(dirname "$0")/add-floating-ip" rabbitmq-server

Subscribers

People subscribed via source and target branches