Add refresh-after-snapshot script for cluster-tool clones - #95
openshift-merge-bot[bot] merged 1 commit into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR modifies the hub registration flow in Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/prepare-fulfillment-service.sh`:
- Line 21: The unconditional osac delete hub hub call causes the script to fail
under set -o errexit when the hub is absent; update the script to guard the
deletion by checking for the hub's existence (or run the delete and ignore the
specific "not found" failure) before calling osac delete hub hub so the command
is safe and idempotent. Locate the osac delete hub hub invocation in the script
and either wrap it with an existence test using the appropriate osac query/list
command, or execute it conditionally and suppress or handle the exit code 4 (Not
found) so the script does not abort when the hub is already absent. Ensure
existing errexit behavior remains for other failures.
In `@scripts/refresh-after-snapshot.sh`:
- Around line 101-105: The script restarts four fulfillment deployments but only
waits for two; add oc rollout status checks for both fulfillment-rest-gateway
and fulfillment-ingress-proxy (using the same --timeout=120s and -n
"${INSTALLER_NAMESPACE}" pattern) after you restart them so the script blocks
until all four rollouts are complete (keep the existing oc rollout restart lines
for fulfillment-rest-gateway and fulfillment-ingress-proxy and mirror the status
call pattern used for fulfillment-controller and fulfillment-grpc-server).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6e5f6ee-b5a5-42da-a0f9-2215c68181a2
📒 Files selected for processing (2)
scripts/prepare-fulfillment-service.shscripts/refresh-after-snapshot.sh
| # Login to fulfillment API and ensure hub exists with current kubeconfig | ||
| FULFILLMENT_API_URL=https://$(oc get route -n ${INSTALLER_NAMESPACE} fulfillment-api -o jsonpath='{.status.ingress[0].host}') | ||
| osac login --insecure --private --token-script "oc create token -n ${INSTALLER_NAMESPACE} admin" --address ${FULFILLMENT_API_URL} | ||
| osac delete hub hub |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the OSAC CLI, what is the exit-code behavior of osac delete hub when the hub does not exist, and is there an--ignore-not-found (or equivalent) flag?
💡 Result:
No public documentation or sources were found describing an "OSAC CLI" or the specific command osac delete hub <id>. Searches returned results for OSCAL CLI (oscal-cli), OCI OS Management Hub CLI (oci os-management-hub... delete), and osac-project/fulfillment-cli (a "Cloud-in-a-box fulfillment CLI" for managing infrastructure like clusters and hosts via gRPC, but its README mentions general delete commands without specifics on delete hub or exit codes [1][2]). No explicit mention of exit code behavior when the hub does not exist, nor an --ignore-not-found flag or equivalent. The command may be internal, unpublished, or from a private project under osac-project (e.g., Red Hat-related, given GitHub patterns [3]). OCI delete commands use --force for confirmation skipping but do not address not-found cases explicitly [4][5].
Citations:
- 1: https://github.com/osac-project/fulfillment-cli
- 2: https://github.com/osac-project/fulfillment-service
- 3: https://github.com/osac-project/fulfillment-common
- 4: https://docs.oracle.com/iaas/tools/oci-cli/latest/oci_cli_docs/cmdref/os-management-hub/management-station/delete.html
- 5: https://docs.public.oneportal.content.oci.oraclecloud.com/en-us/iaas/tools/oci-cli/3.56.0/oci_cli_docs/cmdref/os-management-hub/software-source/delete.html
🏁 Script executed:
#!/bin/bash
# Search for the script file mentioned in the review
find . -name "prepare-fulfillment-service.sh" -type f 2>/dev/nullRepository: osac-project/osac-installer
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Search for any osac-related documentation or code
find . -type f \( -name "*osac*" -o -name "*delete*hub*" \) 2>/dev/null | head -20Repository: osac-project/osac-installer
Length of output: 291
🏁 Script executed:
#!/bin/bash
# Look for documentation about osac command
find . -name "README*" -o -name "*.md" -type f 2>/dev/null | xargs grep -l "osac" 2>/dev/null | head -10Repository: osac-project/osac-installer
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Read the script to see the current state at line 21
cat -n ./scripts/prepare-fulfillment-service.sh | head -30Repository: osac-project/osac-installer
Length of output: 1935
🏁 Script executed:
#!/bin/bash
# Read the OSAC CLI documentation
cat ./OSAC-CLI-HOWTO.mdRepository: osac-project/osac-installer
Length of output: 50385
🏁 Script executed:
#!/bin/bash
# Search for delete command documentation and exit behavior
grep -n -A 10 -B 5 "osac delete" ./OSAC-CLI-HOWTO.md | head -100Repository: osac-project/osac-installer
Length of output: 2638
🏁 Script executed:
#!/bin/bash
# Search for exit code behavior
grep -n -i "exit.*code\|exit.*status" ./OSAC-CLI-HOWTO.mdRepository: osac-project/osac-installer
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Search for --ignore or --force flags
grep -n -E "\-\-ignore|\-\-force" ./OSAC-CLI-HOWTO.mdRepository: osac-project/osac-installer
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Check the API Reference section for delete command signature
sed -n '/## API Reference/,/^## /p' ./OSAC-CLI-HOWTO.md | grep -A 20 "delete hub"Repository: osac-project/osac-installer
Length of output: 495
🏁 Script executed:
#!/bin/bash
# Read the Exit Codes section
sed -n '1668,1690p' ./OSAC-CLI-HOWTO.mdRepository: osac-project/osac-installer
Length of output: 614
🏁 Script executed:
#!/bin/bash
# Search for "not found" or similar error messages
grep -n -i "not found\|does not exist\|not exist" ./OSAC-CLI-HOWTO.mdRepository: osac-project/osac-installer
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Look for error behavior in delete command documentation
sed -n '/## API Reference/,/^##/p' ./OSAC-CLI-HOWTO.md | grep -A 30 "delete hub"Repository: osac-project/osac-installer
Length of output: 53
Guard hub deletion to preserve idempotency.
With set -o errexit, an unconditional osac delete hub hub exits with code 4 ("Not found error") when the hub is absent, which aborts the script and breaks the idempotent flow this PR targets. The --force flag exists but is for bypassing confirmation prompts, not error handling.
Suggested fix
-osac delete hub hub
+osac delete hub hub || true
osac create hub --kubeconfig=/tmp/kubeconfig.hub-access --id hub --namespace ${INSTALLER_NAMESPACE}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| osac delete hub hub | |
| osac delete hub hub || true | |
| osac create hub --kubeconfig=/tmp/kubeconfig.hub-access --id hub --namespace ${INSTALLER_NAMESPACE} |
🤖 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 `@scripts/prepare-fulfillment-service.sh` at line 21, The unconditional osac
delete hub hub call causes the script to fail under set -o errexit when the hub
is absent; update the script to guard the deletion by checking for the hub's
existence (or run the delete and ignore the specific "not found" failure) before
calling osac delete hub hub so the command is safe and idempotent. Locate the
osac delete hub hub invocation in the script and either wrap it with an
existence test using the appropriate osac query/list command, or execute it
conditionally and suppress or handle the exit code 4 (Not found) so the script
does not abort when the hub is already absent. Ensure existing errexit behavior
remains for other failures.
5d6b7e9 to
5230a29
Compare
|
/retest |
5230a29 to
20e2eb1
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/prepare-fulfillment-service.sh (1)
24-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard hub deletion to keep idempotency under
errexit(Line 24).
osac delete hub hubcan terminate the script when the hub is already absent, which breaks the idempotent flow this PR is aiming for.Suggested minimal fix
-osac delete hub hub +osac delete hub hub || true osac create hub --kubeconfig=/tmp/kubeconfig.hub-access --id hub --namespace ${INSTALLER_NAMESPACE}🤖 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 `@scripts/prepare-fulfillment-service.sh` at line 24, The direct call to "osac delete hub hub" fails under errexit when the hub is already missing; change it to a guarded delete by first checking for the hub's existence (e.g., call "osac get hub hub" or equivalent) and only run "osac delete hub hub" if the check succeeds, ensuring the delete is a no-op when the hub is absent and preserving idempotency under errexit.
🤖 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.
Duplicate comments:
In `@scripts/prepare-fulfillment-service.sh`:
- Line 24: The direct call to "osac delete hub hub" fails under errexit when the
hub is already missing; change it to a guarded delete by first checking for the
hub's existence (e.g., call "osac get hub hub" or equivalent) and only run "osac
delete hub hub" if the check succeeds, ensuring the delete is a no-op when the
hub is absent and preserving idempotency under errexit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93cfd2ff-a4f1-49a5-8078-dcc7226d6ca9
📒 Files selected for processing (2)
scripts/prepare-fulfillment-service.shscripts/refresh-after-snapshot.sh
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/hold |
|
/retest |
When booting an OSAC cluster from a snapshot (via cluster-tool), the cluster gets a new domain but application-level resources still reference the old one. This script refreshes all domain-sensitive resources: - Deletes and recreates routes (kustomize-managed and AAP-managed) - Triggers AAP operator reconciliation to regenerate configs - Waits for AAP controller to be healthy - Recreates AAP API token with correct URL - Re-registers hub with fresh kubeconfig - Restarts fulfillment pods to pick up new hub config - Ensures tenant exists Also makes prepare-fulfillment-service.sh idempotent by deleting the existing hub before creating (osac delete hub returns 0 if not found).
20e2eb1 to
8f5707b
Compare
|
/unhold |
| exit 1 | ||
| } | ||
| AAP_ROUTE_HOST=$(oc get route osac-aap -n "${INSTALLER_NAMESPACE}" -o jsonpath='{.spec.host}') | ||
| retry_until 120 5 '[[ "$(curl -sk -o /dev/null -w %{http_code} https://'"${AAP_ROUTE_HOST}"'/api/gateway/v1/)" == "200" ]]' || { |
There was a problem hiding this comment.
if you want there's an healthcheck endpoint that expose the health of each components
| echo "[6/8] Configuring fulfillment service..." | ||
| ./scripts/prepare-fulfillment-service.sh | ||
|
|
||
| echo "[7/8] Restarting fulfillment pods..." |
There was a problem hiding this comment.
don't we have a common label ? so we don't rely on the name of the Deployments as the fulfilment-service might change again in the future.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adriengentil, akshaynadkarni, omer-vishlitzky 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 |
3 participants
Summary
scripts/refresh-after-snapshot.sh— refreshes all domain-sensitive OSAC resources after booting a cluster from a snapshot via cluster-toolprepare-fulfillment-service.shidempotent (deletes existing hub before creating)Context
cluster-tool boots OpenShift SNO clusters from golden snapshots in ~6 minutes via recert-based identity regeneration. After boot, the cluster has a new domain but OSAC application resources (routes, AAP configs, hub kubeconfig, fulfillment controller) still reference the old domain.
This script handles all the domain-sensitive cleanup:
Usage
Total time: ~10 minutes (6 min boot + 4 min refresh) vs 2+ hours from scratch.
Test plan
make test-vmaas— 8/8 tests passSummary by CodeRabbit
New Features
Chores