aws-hypervisor: restructure instance-data into per-node subdirectories#72
Open
fonta-rh wants to merge 3 commits into
Open
aws-hypervisor: restructure instance-data into per-node subdirectories#72fonta-rh wants to merge 3 commits into
fonta-rh wants to merge 3 commits into
Conversation
Move per-node files (aws-instance-id, public_address, ssh_user, etc.) into instance-data/node-0/ while keeping shared files (network_stack_name, to_be_removed_cf_stack_list) at the root. This prepares the layout for multi-node support (OCPEDGE-2608). Add get_node_dir/get_shared_dir helpers to common.sh with a migration fallback: if node-0/ doesn't exist but flat files do, get_node_dir returns the flat path so unported scripts keep working. Update openshift-clusters scripts with dual-path existence checks (node-0/ first, flat fallback) for backward compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
WalkthroughThis PR restructures multi-node deployment support by introducing ChangesMulti-node deployment infrastructure
🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fonta-rh 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 |
openshift-ci
Bot
added
the
approved
Scripts using hardcoded flat instance-data paths failed after create.sh moved per-node files into node-0/ subdirectories. The migration fallback in get_node_dir only helps scripts that call it — these didn't. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Cache get_node_dir/get_shared_dir in create.sh for consistency with all other scripts in the PR - Move .done marker write inside directory guard in destroy.sh so cleanup is idempotent when instance-data was already removed - Fix missing trailing newline in destroy.sh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@deploy/openshift-clusters/scripts/clean-spoke.sh`:
- Around line 42-43: The INSTANCE_IP assignment can cause the script to exit
under set -o errexit if both files are missing; update the command that sets
INSTANCE_IP to fall back to an empty string instead of letting the subshell fail
(e.g. change the substitution to use "|| echo \"\"" so it becomes
INSTANCE_IP=$(cat ... 2>/dev/null || cat ... 2>/dev/null || echo "")). This
mirrors the SSH_USER fallback pattern and ensures later validation logic (used
after INSTANCE_IP) runs instead of the script exiting prematurely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7674ec47-98d7-41e1-92b7-3aeb56a3aadb
📒 Files selected for processing (17)
deploy/aws-hypervisor/instance.env.templatedeploy/aws-hypervisor/scripts/common.shdeploy/aws-hypervisor/scripts/create.shdeploy/aws-hypervisor/scripts/destroy.shdeploy/aws-hypervisor/scripts/force-stop.shdeploy/aws-hypervisor/scripts/init.shdeploy/aws-hypervisor/scripts/inventory.shdeploy/aws-hypervisor/scripts/print_instance_data.shdeploy/aws-hypervisor/scripts/ssh.shdeploy/aws-hypervisor/scripts/start.shdeploy/aws-hypervisor/scripts/stop.shdeploy/openshift-clusters/scripts/clean-spoke.shdeploy/openshift-clusters/scripts/clean.shdeploy/openshift-clusters/scripts/deploy-cluster.shdeploy/openshift-clusters/scripts/deploy-fencing-assisted.shdeploy/openshift-clusters/scripts/full-clean.shdeploy/openshift-clusters/scripts/patch-nodes.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
get_node_dir/get_shared_dirhelpers tocommon.shwith migration fallback for backward compatibilityaws-instance-id,public_address,ssh_user, etc.) intoinstance-data/node-0/subdirectory; shared files (network_stack_name,to_be_removed_cf_stack_list) stay at rootnode-0/first, flat fallback)Part of the multi-node groundwork series (OCPEDGE-2608). Follow-up to #70 (split CF template).
Test plan
make shellcheckpassesmake createwrites per-node files toinstance-data/node-0/and shared files toinstance-data/make start/make stopstill work via migration fallback (unported scripts)make inforeads instance data correctly via fallbackmake destroytears down both stacks and cleans upinstance-data/including node subdirsmake clean,make deploy, etc.) find instance data in both layouts🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores