Skip to content

Commit b8b149b

Browse files
dhensel-rhclaude
andcommitted
Fix shellcheck SC2086 and auto-discover hypervisor from inventory
- Convert SSH_OPTS from string to bash array, expand with "${SSH_OPTS[@]}" - Auto-discover hypervisor from TNT inventory (deploy/openshift-clusters/inventory.ini) with explicit argument as override - Add pre-flight check for dev-scripts on the hypervisor - Make missing config file non-fatal (Patch 0 skips gracefully) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7af8e43 commit b8b149b

2 files changed

Lines changed: 90 additions & 20 deletions

File tree

helpers/README.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,11 @@ ansible all -i deploy/openshift-clusters/inventory.ini -m shell -a "./fencing_va
132132

133133
Patches dev-scripts on a hypervisor to generate `macaddress:`-based fencing credentials instead of `hostname:`-based ones in the ABI install-config. Used for verifying [OCPEDGE-2692](https://issues.redhat.com/browse/OCPEDGE-2692) (MAC address fencing credential support in assisted-service).
134134

135-
**What it patches** (3 files under `dev-scripts/agent/`):
136-
1. `05_agent_configure.sh` — collects master MAC addresses into `AGENT_MASTER_MACS_STR`
137-
2. `roles/manifests/vars/main.yml` — adds `agent_master_macs` Ansible variable
138-
3. `roles/manifests/templates/install-config_baremetal_yaml.j2` — replaces `hostname:` with `macaddress:` in the fencing credentials block
139-
140-
Also fixes `OPENSHIFT_CI` case sensitivity (`"TRUE"``"true"`) in the dev-scripts config file.
135+
**What it patches** (4 files under `dev-scripts/`):
136+
1. `config_*.sh` — ensures `AGENT_E2E_TEST_SCENARIO="TNF_IPV4"` is set; fixes `OPENSHIFT_CI` case sensitivity (`"TRUE"``"true"`)
137+
2. `agent/05_agent_configure.sh` — collects master MAC addresses into `AGENT_MASTER_MACS_STR`
138+
3. `agent/roles/manifests/vars/main.yml` — adds `agent_master_macs` Ansible variable
139+
4. `agent/roles/manifests/templates/install-config_baremetal_yaml.j2` — replaces `hostname:` with `macaddress:` in the fencing credentials block
141140

142141
**Prerequisites:**
143142
- Hypervisor must be initialized (TNT `make init` or `make deploy`) so that dev-scripts is already cloned
@@ -146,14 +145,19 @@ Also fixes `OPENSHIFT_CI` case sensitivity (`"TRUE"` → `"true"`) in the dev-sc
146145
**Usage:**
147146

148147
```bash
149-
# From helpers/ directory
148+
# From helpers/ directory — auto-discovers hypervisor from TNT inventory
149+
./patch-devscripts-mac-fencing.sh
150+
151+
# Or specify an explicit hypervisor
150152
./patch-devscripts-mac-fencing.sh ec2-user@<hypervisor-ip>
151153

152154
# Then on the hypervisor:
153155
cd /home/ec2-user/openshift-metal3/dev-scripts
154156
sudo make agent
155157
```
156158

159+
The hypervisor is auto-discovered from `deploy/openshift-clusters/inventory.ini` (the `[metal_machine]` group). An explicit argument overrides auto-discovery.
160+
157161
**Important:** TNT's Ansible role runs `git checkout --force` on dev-scripts before every deployment, wiping these patches. Either run this script after TNT's git checkout (before `make agent`), or deploy directly on the hypervisor.
158162

159163
### Resource Agents Patching

helpers/patch-devscripts-mac-fencing.sh

Lines changed: 79 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ set -euo pipefail
1010
# - A running cluster is NOT required — this patches source files
1111
# before the cluster is deployed.
1212
#
13-
# Usage: ./patch-devscripts-mac-fencing.sh <hypervisor_host>
14-
# Example: ./patch-devscripts-mac-fencing.sh ec2-user@3.21.82.70
13+
# Usage: ./patch-devscripts-mac-fencing.sh [hypervisor_host]
14+
#
15+
# The hypervisor is auto-discovered from the TNT inventory file
16+
# (deploy/openshift-clusters/inventory.ini). An explicit argument
17+
# overrides auto-discovery.
1518
#
1619
# IMPORTANT: TNT's Ansible role runs `git checkout --force` on dev-scripts
1720
# before every deployment, wiping these patches. Either:
@@ -25,21 +28,45 @@ set -euo pipefail
2528
# rm -rf ocp/ostest
2629
# sudo make agent
2730

28-
HYPERVISOR="${1:?Usage: $0 <hypervisor_host> (e.g. ec2-user@3.21.82.70)}"
31+
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
32+
INVENTORY="${SCRIPT_DIR}/../deploy/openshift-clusters/inventory.ini"
33+
34+
if [[ -n "${1:-}" ]]; then
35+
HYPERVISOR="$1"
36+
elif [[ -f "${INVENTORY}" ]]; then
37+
HYPERVISOR=$(awk '/^\[metal_machine\]/{found=1; next} found && /^[[:space:]]*$/{next} found && /^#/{next} found && /^\[/{exit} found{print $1; exit}' "${INVENTORY}")
38+
if [[ -z "${HYPERVISOR}" ]]; then
39+
echo "ERROR: Could not parse hypervisor from ${INVENTORY}"
40+
exit 1
41+
fi
42+
echo "Auto-discovered hypervisor from inventory: ${HYPERVISOR}"
43+
else
44+
echo "ERROR: No hypervisor specified and inventory not found at ${INVENTORY}"
45+
echo "Usage: $0 [hypervisor_host]"
46+
exit 1
47+
fi
48+
2949
DEV_SCRIPTS="/home/ec2-user/openshift-metal3/dev-scripts"
30-
SSH_OPTS="-o StrictHostKeyChecking=no -o ServerAliveInterval=30 -o ServerAliveCountMax=120"
50+
SSH_OPTS=(-o StrictHostKeyChecking=accept-new -o ServerAliveInterval=30 -o ServerAliveCountMax=120)
3151

3252
echo "==> Patching dev-scripts on ${HYPERVISOR} for MAC-only fencing credentials"
3353

54+
# Pre-flight: verify dev-scripts is cloned on the hypervisor
55+
if ! ssh "${SSH_OPTS[@]}" "${HYPERVISOR}" test -d "${DEV_SCRIPTS}/agent"; then
56+
echo "ERROR: dev-scripts not found at ${DEV_SCRIPTS} on ${HYPERVISOR}"
57+
echo "Run 'make init' or 'make deploy' from deploy/ first."
58+
exit 1
59+
fi
60+
3461
# Patch 0: Ensure AGENT_E2E_TEST_SCENARIO is set in the config file
3562
echo "--- Patch 0/3: config file (AGENT_E2E_TEST_SCENARIO)"
3663
# shellcheck disable=SC2087
37-
ssh ${SSH_OPTS} "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH0_EOF'
64+
ssh "${SSH_OPTS[@]}" "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH0_EOF'
3865
DEV_SCRIPTS="$1"
3966
CONFIG=$(ls "${DEV_SCRIPTS}"/config_*.sh 2>/dev/null | head -1)
4067
if [[ -z "$CONFIG" ]]; then
41-
echo " ERROR: No config_*.sh found in ${DEV_SCRIPTS}"
42-
exit 1
68+
echo " SKIP: No config_*.sh found yet — will be created by 'make deploy'"
69+
exit 0
4370
fi
4471
echo " Config file: $(basename "$CONFIG")"
4572
@@ -67,23 +94,41 @@ PATCH0_EOF
6794
# - Export AGENT_MASTER_MACS_STR (after AGENT_MASTER_HOSTNAMES_STR)
6895
echo "--- Patch 1/3: agent/05_agent_configure.sh"
6996
# shellcheck disable=SC2087
70-
ssh ${SSH_OPTS} "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH1_EOF'
97+
ssh "${SSH_OPTS[@]}" "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH1_EOF'
7198
DEV_SCRIPTS="$1"
7299
FILE="${DEV_SCRIPTS}/agent/05_agent_configure.sh"
73100
74101
# 1a: Add AGENT_MASTER_MACS collection after AGENT_NODES_MACS line
75102
if grep -q 'AGENT_MASTER_MACS' "$FILE"; then
76103
echo " Already patched (AGENT_MASTER_MACS found), skipping 1a"
77104
else
105+
if ! grep -q 'AGENT_NODES_MACS+=("$node_mac")$' "$FILE"; then
106+
echo " ERROR: Anchor pattern 'AGENT_NODES_MACS+=(\"\\$node_mac\")' not found in $FILE"
107+
echo " Upstream dev-scripts may have changed. Patch 1a cannot be applied."
108+
exit 1
109+
fi
78110
sed -i '/AGENT_NODES_MACS+=("$node_mac")$/a\ if [[ "$node_type" == "master" ]]; then\n AGENT_MASTER_MACS+=("$node_mac")\n fi' "$FILE"
111+
if ! grep -q 'AGENT_MASTER_MACS' "$FILE"; then
112+
echo " ERROR: Patch 1a failed — AGENT_MASTER_MACS not found after sed"
113+
exit 1
114+
fi
79115
echo " Applied: AGENT_MASTER_MACS collection"
80116
fi
81117
82118
# 1b: Export AGENT_MASTER_MACS_STR after AGENT_MASTER_HOSTNAMES_STR
83119
if grep -q 'AGENT_MASTER_MACS_STR' "$FILE"; then
84120
echo " Already patched (AGENT_MASTER_MACS_STR found), skipping 1b"
85121
else
122+
if ! grep -q 'export AGENT_MASTER_HOSTNAMES_STR=' "$FILE"; then
123+
echo " ERROR: Anchor pattern 'export AGENT_MASTER_HOSTNAMES_STR=' not found in $FILE"
124+
echo " Upstream dev-scripts may have changed. Patch 1b cannot be applied."
125+
exit 1
126+
fi
86127
sed -i '/export AGENT_MASTER_HOSTNAMES_STR=/a\ master_macs=$(printf '"'"'%s,'"'"' "${AGENT_MASTER_MACS[@]}")\n export AGENT_MASTER_MACS_STR=${master_macs::-1}' "$FILE"
128+
if ! grep -q 'AGENT_MASTER_MACS_STR' "$FILE"; then
129+
echo " ERROR: Patch 1b failed — AGENT_MASTER_MACS_STR not found after sed"
130+
exit 1
131+
fi
87132
echo " Applied: AGENT_MASTER_MACS_STR export"
88133
fi
89134
PATCH1_EOF
@@ -92,14 +137,23 @@ PATCH1_EOF
92137
# - Add agent_master_macs variable
93138
echo "--- Patch 2/3: agent/roles/manifests/vars/main.yml"
94139
# shellcheck disable=SC2087
95-
ssh ${SSH_OPTS} "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH2_EOF'
140+
ssh "${SSH_OPTS[@]}" "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH2_EOF'
96141
DEV_SCRIPTS="$1"
97142
FILE="${DEV_SCRIPTS}/agent/roles/manifests/vars/main.yml"
98143
99144
if grep -q 'agent_master_macs' "$FILE"; then
100145
echo " Already patched (agent_master_macs found), skipping"
101146
else
147+
if ! grep -q '^agent_master_hostnames:' "$FILE"; then
148+
echo " ERROR: Anchor pattern 'agent_master_hostnames:' not found in $FILE"
149+
echo " Upstream dev-scripts may have changed. Patch 2 cannot be applied."
150+
exit 1
151+
fi
102152
sed -i '/^agent_master_hostnames:/a agent_master_macs: "{{ lookup('"'"'env'"'"', '"'"'AGENT_MASTER_MACS_STR'"'"') }}"' "$FILE"
153+
if ! grep -q 'agent_master_macs' "$FILE"; then
154+
echo " ERROR: Patch 2 failed — agent_master_macs not found after sed"
155+
exit 1
156+
fi
103157
echo " Applied: agent_master_macs variable"
104158
fi
105159
PATCH2_EOF
@@ -108,17 +162,29 @@ PATCH2_EOF
108162
# - Replace hostname-based fencing with macaddress-based
109163
echo "--- Patch 3/3: install-config_baremetal_yaml.j2 (MAC-only fencing)"
110164
# shellcheck disable=SC2087
111-
ssh ${SSH_OPTS} "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH3_EOF'
165+
ssh "${SSH_OPTS[@]}" "${HYPERVISOR}" bash -s "${DEV_SCRIPTS}" <<'PATCH3_EOF'
112166
DEV_SCRIPTS="$1"
113167
FILE="${DEV_SCRIPTS}/agent/roles/manifests/templates/install-config_baremetal_yaml.j2"
114168
115169
if grep -q 'macaddress:' "$FILE"; then
116170
echo " Already patched (macaddress found), skipping"
117171
else
118-
# Replace the fencing block: hostname → macaddress
172+
MISSING_ANCHORS=()
173+
grep -q '{% set master_hostnames = agent_master_hostnames.split' "$FILE" || MISSING_ANCHORS+=("master_hostnames set")
174+
grep -q '{% for hostname in master_hostnames %}' "$FILE" || MISSING_ANCHORS+=("for hostname loop")
175+
grep -q 'hostname: {{hostname}}' "$FILE" || MISSING_ANCHORS+=("hostname field")
176+
if [[ ${#MISSING_ANCHORS[@]} -gt 0 ]]; then
177+
echo " ERROR: Anchor patterns not found in $FILE: ${MISSING_ANCHORS[*]}"
178+
echo " Upstream dev-scripts may have changed. Patch 3 cannot be applied."
179+
exit 1
180+
fi
119181
sed -i 's/{% set master_hostnames = agent_master_hostnames.split/{% set master_macs = agent_master_macs.split/' "$FILE"
120182
sed -i 's/{% for hostname in master_hostnames %}/{% for mac in master_macs %}/' "$FILE"
121183
sed -i 's/ - hostname: {{hostname}}/ - macaddress: {{ mac }}/' "$FILE"
184+
if ! grep -q 'macaddress:' "$FILE"; then
185+
echo " ERROR: Patch 3 failed — macaddress not found after sed"
186+
exit 1
187+
fi
122188
echo " Applied: macaddress-based fencing credentials"
123189
fi
124190
PATCH3_EOF
@@ -127,7 +193,7 @@ echo ""
127193
echo "==> All patches applied successfully."
128194
echo ""
129195
echo "Next steps:"
130-
echo " 1. SSH to the hypervisor: ssh ${SSH_OPTS} ${HYPERVISOR}"
196+
echo " 1. SSH to the hypervisor: ssh ${HYPERVISOR}"
131197
echo " 2. cd ${DEV_SCRIPTS}"
132198
echo " 3. make clean (if cluster exists)"
133-
echo " 4. make agent (deploy with MAC-only fencing)"
199+
echo " 4. sudo make agent (deploy with MAC-only fencing)"

0 commit comments

Comments
 (0)