Skip to content

Rename ABI-specific baremetal vars to AGENT_BAREMETAL_* prefix #97

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.

Sign up for GitHub

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

Open
fonta-rh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Rename ABI-specific baremetal vars to AGENT_BAREMETAL_* prefix #97

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions deploy/openshift-clusters/scripts/baremetal-adopt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,33 +431,33 @@ generate_baremetal_config() {
echo "export MANAGE_INT_BRIDGE=n"
echo "export AGENT_E2E_TEST_SCENARIO=\"TNF_IPV4_DHCP\""

# BAREMETAL_IPS is required — dev-scripts crashes under set -u without it
# AGENT_BAREMETAL_IPS is required — dev-scripts agent pipeline needs node IPs
local ip_list=""
for ((i = 0; i < ${#NODE_IPS[@]}; i++)); do
[[ -z "${NODE_IPS[$i]}" ]] && die "Node '${NODE_NAMES[$i]}': node_ip is required for baremetal deploy"
[[ -n "${ip_list}" ]] && ip_list+=","
ip_list+="${NODE_IPS[$i]}"
Comment on lines 435 to 439

@coderabbitai coderabbitai Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use uppercase names for new shell locals.

ip_list, mac_list, and the loop index i violate the repository’s shell-script naming rule. Rename them consistently, for example, to IP_LIST, MAC_LIST, and INDEX.

As per coding guidelines, shell scripts should use meaningful uppercase variable names.

Proposed fix 
-        local ip_list=""
-        for ((i = 0; i < ${`#NODE_IPS`[@]}; i++)); do
-            [[ -z "${NODE_IPS[$i]}" ]] && die "Node '${NODE_NAMES[$i]}': node_ip is required for baremetal deploy"
-            [[ -n "${ip_list}" ]] && ip_list+=","
-            ip_list+="${NODE_IPS[$i]}"
+        local IP_LIST=""
+        local INDEX
+        for ((INDEX = 0; INDEX < ${`#NODE_IPS`[@]}; INDEX++)); do
+            [[ -z "${NODE_IPS[$INDEX]}" ]] && die "Node '${NODE_NAMES[$INDEX]}': node_ip is required for baremetal deploy"
+            [[ -n "${IP_LIST}" ]] && IP_LIST+=","
+            IP_LIST+="${NODE_IPS[$INDEX]}"
         done
-        echo "export AGENT_BAREMETAL_IPS=\"${ip_list}\""
+        echo "export AGENT_BAREMETAL_IPS=\"${IP_LIST}\""

Also applies to: 454-458

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/openshift-clusters/scripts/baremetal-adopt.sh` around lines 435 - 439,
Rename the newly introduced shell locals in the baremetal adoption logic to
uppercase names: change ip_list to IP_LIST, mac_list to MAC_LIST, and the loop
index i to INDEX. Update every reference across both affected loops, including
array indexing and accumulation, without changing the existing behavior.

Source: Coding guidelines

done
echo "export BAREMETAL_IPS=\"${ip_list}\""
echo "export AGENT_BAREMETAL_IPS=\"${ip_list}\""

# BAREMETAL_API_VIP is required — set_api_and_ingress_vip() needs it
# BAREMETAL_API_VIP is platform-level (used by both IPI and ABI in network.sh)
[[ -z "${API_VIP}" ]] && die "api_vip is required in [baremetal_network] for baremetal deploy"
[[ -z "${ISO_URL}" ]] && die "iso_url is required in [baremetal_network] for baremetal deploy"
echo ""
echo "# Baremetal network config"
echo "export BAREMETAL_API_VIP=\"${API_VIP}\""
echo "export BAREMETAL_ISO_SERVER=\"${ISO_URL}\""
echo "export AGENT_BAREMETAL_ISO_SERVER=\"${ISO_URL}\""
[[ -n "${MACHINE_NETWORK}" ]] && echo "export EXTERNAL_SUBNET_V4=\"${MACHINE_NETWORK}\""
[[ -n "${INGRESS_VIP}" ]] && echo "export BAREMETAL_INGRESS_VIP=\"${INGRESS_VIP}\""

# BAREMETAL_MACS is required — agent-config needs data NIC MACs for hostname mapping
# AGENT_BAREMETAL_MACS is required — agent-config needs data NIC MACs for hostname mapping
local mac_list=""
for ((i = 0; i < ${#NODE_DATA_MACS[@]}; i++)); do
[[ -z "${NODE_DATA_MACS[$i]}" ]] && die "Node '${NODE_NAMES[$i]}': data_mac is required for baremetal deploy"
[[ -n "${mac_list}" ]] && mac_list+=","
mac_list+="${NODE_DATA_MACS[$i]}"
done
echo "export BAREMETAL_MACS=\"${mac_list}\""
echo "export AGENT_BAREMETAL_MACS=\"${mac_list}\""
Comment on lines +453 to +460

@coderabbitai coderabbitai Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Inspecting cited files...\n'
sed -n '440,470p' deploy/openshift-clusters/scripts/baremetal-adopt.sh
printf '\n---\n'
sed -n '1,80p' deploy/openshift-clusters/inventory_baremetal.ini.sample

printf '\nSearching for old/new variable references...\n'
rg -n 'BAREMETAL_MACS|AGENT_BAREMETAL_MACS|data_mac' deploy/openshift-clusters

Repository: openshift-eng/two-node-toolbox

Length of output: 7590

Update the inventory sample to use AGENT_BAREMETAL_MACS.
deploy/openshift-clusters/inventory_baremetal.ini.sample still documents data_mac as emitting BAREMETAL_MACS; that contract should match deploy/openshift-clusters/scripts/baremetal-adopt.sh.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/openshift-clusters/scripts/baremetal-adopt.sh` around lines 453 - 460,
Update deploy/openshift-clusters/inventory_baremetal.ini.sample so its data_mac
documentation references AGENT_BAREMETAL_MACS instead of BAREMETAL_MACS,
matching the export emitted by baremetal-adopt.sh.

} > "${output_file}"

info " → ${output_file}"
Expand Down