Bug #20934
closedtest-provision-centos7 is failing
Description
The last time it passed was August 2. Since then it has consistently failed with this error:
++ for node in '"${!NODES[@]}"' ++ '[' -z '' ']' ++ NODELIST=localhost ++ for node in '"${!NODES[@]}"' ++ roles= ++ IFS=, ++ read -ra roles_array common.sh: line 35: roles_array[@]: unbound variable Build step 'Execute shell' marked build as failure
Looking at the Git logs around then I'm guessing this is a consequence of 39f741f8e4ca8eb8aa2538df0bedc6ae143a038a, common.sh
may be relying on bash features than are more modern than centos7 includes.
Updated by Brett Smith over 1 year ago
I think common.sh
needs to say declare -a roles_array
at some point before the read
. installer.sh
does this for other arrays but not this one.
Updated by Brett Smith over 1 year ago
- Story points set to 0.5
- Target version changed from Future to Development 2023-09-13 sprint
- Assigned To set to Brett Smith
- Status changed from New to In Progress
Updated by Brett Smith over 1 year ago
It's not quite that simple. You get the error when the array is empty, whether or not the array was declared.
% cat declare.sh #!/bin/bash set -eu set -o pipefail declare -a roles_array IFS=',' read -ra roles_array <<<"" if [[ "${#roles_array[@]}" -eq 0 ]]; then echo "array is empty" fi for role in "${roles_array[@]}"; do echo "role is $role" done echo done % bash declare.sh array is empty done % podman run --rm -i centos:7 bash - <declare.sh array is empty bash: line 11: roles_array[@]: unbound variable
20934-centos7-bash-fix @ afe2449b11c0953b5f6006daf9b43a4659c49b68 - test-provision-centos7: #707
Updated by Brett Smith over 1 year ago
Now the build is failing with:
+ '[' self-signed = self-signed ']' + echo 'Copying the Arvados CA certificate '\''local-arvados-snakeoil-ca.crt'\'' to the installer dir, so you can import it' Copying the Arvados CA certificate 'local-arvados-snakeoil-ca.crt' to the installer dir, so you can import it + '[' x = xyes ']' + cp /etc/ssl/certs/arvados-snakeoil-ca.pem /tmp/workspace/test-provision-centos7/tools/salt-install/local-arvados-snakeoil-ca.crt cp: cannot stat ‘/etc/ssl/certs/arvados-snakeoil-ca.pem’: No such file or directory Build step 'Execute shell' marked build as failure
This part of provision.sh
assumes SSL certs use Debian-style paths:
# Leave a copy of the Arvados CA so the user can copy it where it's required if [ "${SSL_MODE}" = "self-signed" ]; then echo "Copying the Arvados CA certificate '${DOMAIN}-arvados-snakeoil-ca.crt' to the installer dir, so you can import it" if [ "x${VAGRANT:-}" = "xyes" ]; then cp /etc/ssl/certs/arvados-snakeoil-ca.pem /vagrant/${DOMAIN}-arvados-snakeoil-ca.pem else cp /etc/ssl/certs/arvados-snakeoil-ca.pem ${SCRIPT_DIR}/${DOMAIN}-arvados-snakeoil-ca.crt fi fi
This logic needs to be updated to accommodate Red Hat-based distros.
I'm setting this aside for a moment, I'm not sure this is what I should be doing. Skimming provision.sh
real quick, there are some CentOS-specific branches, and now I'm wondering, do those need to be extended for Rocky? RHEL? Others? We should talk about the scope of this ticket and how it relates to the 2.7 release schedule.
Updated by Brett Smith over 1 year ago
- Related to Idea #20610: Installer supports deploying a load-balancer to horizontally scale the controller node added
Updated by Brett Smith over 1 year ago
- Related to Feature #20797: Build Rocky Linux / RHEL compatible packages added
Updated by Brett Smith over 1 year ago
This code has been in here for a while, so it seems like centos7 used to avoid this whole cert-copying branch… somehow.
Updated by Lucas Di Pentima over 1 year ago
I've just forked your branch to add a new change, to avoid this last error at aa3e26a75 (branch 20934-centos7-bash-fix-lucas
). As this seems to be a long standing bug, this change aims to unblock the release process until we properly fix it. WDYT?
Updated by Brett Smith over 1 year ago
Per discussion at standup, we don't really want this test job: it's not clear anyone is using the installer on a Red Hat distro, and we definitely don't want any new installs on CentOS 7 specifically this way. I have removed test-provision-centos7 from the test-provision multijob. Given this, I don't see any reason to merge the branch; at this point, it would just be historical cruft.
Updated by Brett Smith over 1 year ago
- Status changed from In Progress to Closed