-
Notifications
You must be signed in to change notification settings - Fork 2k
ci-operator/step-registry/upi: Fix ShellCheck quoting, fgrep, etc. #10113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci-operator/step-registry/upi: Fix ShellCheck quoting, fgrep, etc. #10113
Conversation
Addressing:
$ sudo podman run --rm --volume "${PWD}:/workdir:ro,z" --workdir /workdir registry.svc.ci.openshift.org/ci/shellcheck sh -c 'find ci-operator/step-registry/upi -name "*.sh" -print0 | xargs -0 -n1 shellcheck'
In ci-operator/step-registry/upi/conf/gcp/upi-conf-gcp-commands.sh line 32:
HOST_PROJECT=$(jq -r '.hostProject' ${SHARED_DIR}/xpn.json)
^-----------^ SC2086: Double quote to prevent globbing and word splitting.
Did you mean:
HOST_PROJECT=$(jq -r '.hostProject' "${SHARED_DIR}"/xpn.json)
...
In ci-operator/step-registry/upi/install/gcp/upi-install-gcp-commands.sh line 46:
echo "command failed, retrying in $(( 2 ** $attempt )) seconds"
^------^ SC2004: $/${} is unnecessary on arithmetic variables.
...
In ci-operator/step-registry/upi/install/gcp/upi-install-gcp-commands.sh line 290:
IMAGE_SOURCE=$(cat /var/lib/openshift-install/rhcos.json | jq -r .gcp.url)
^-- SC2002: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.
...
In ci-operator/step-registry/upi/install/gcp/upi-install-gcp-commands.sh line 365:
fgrep -e '-master-0' 05_control_plane.py
^---^ SC2197: fgrep is non-standard and deprecated. Use grep -F instead.
...
For more information:
https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
https://www.shellcheck.net/wiki/SC2197 -- fgrep is non-standard and depreca...
https://www.shellcheck.net/wiki/SC2002 -- Useless cat. Consider 'cmd < file...
|
/assign @jstuever Because most touches are to the GCP steps, and that's your space :). |
|
e2e-gcp-upi-xpn died on e2e test-case failures, so not a result of step script changes (or at least, very unlikely ;). e2e-snc died on install: Also seems unlikely to be a result of my changes. e2e-gcp-upi died in install: followed by a number of later steps failing because the cluster wasn't up. Not related to my changes either. So I think we're good to go for this PR. |
|
/hold |
|
/lgtm |
|
/test pj-rehearse |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jstuever, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@wking: Updated the following 2 configmaps:
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Addressing:
More groundwork for #9772.