Skip to content

start partner install conditionally only if file exists#29

Merged
maorfr merged 2 commits intomainfrom
partner-start-conditional
Mar 6, 2026
Merged

start partner install conditionally only if file exists#29
maorfr merged 2 commits intomainfrom
partner-start-conditional

Conversation

@maorfr
Copy link
Copy Markdown
Collaborator

@maorfr maorfr commented Mar 5, 2026

related to https://github.com/gori-project/GoRI/issues/816

replaces #25

in this PR we uncomment the partner install start script execution and make the execution conditional only if the file exists. this will facilitate testing and production needs.

Summary by CodeRabbit

  • Chores
    • Deployment wrapper now performs a runtime check before attempting partner installation and provides clear progress and fallback messages when the installer is absent.
    • Removed an obsolete placeholder installation script that was no longer used.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: abfffabc-b49c-49a3-ac98-a4577cf3457f

📥 Commits

Reviewing files that changed from the base of the PR and between 9f3de34 and 15ab05a.

📒 Files selected for processing (2)
  • bootstrap.sh
  • partner-install/start.sh
💤 Files with no reviewable changes (1)
  • partner-install/start.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • bootstrap.sh

Walkthrough

bootstrap.sh now conditionally attempts Partner OverLay deployment: it prints a prompt, checks for partner-install/start.sh, runs it (logging output) if present, or prints a not-found message; the partner-install script file (partner-install/start.sh) was removed.

Changes

Cohort / File(s) Summary
Bootstrap Script Activation
bootstrap.sh
Replaced commented/no-op block with guarded runtime check: prints deployment prompt, tests for partner-install/start.sh, executes it with output redirected to the log if found, otherwise prints a not-found message; retains final "Done" progress output.
Partner Install Script Removal
partner-install/start.sh
Removed the entire script file (POSIX shebang, banner, KUBECONFIG-from-arg behavior, placeholder implementation and export were deleted).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly summarizes the main change: making partner install start conditional based on file existence, which matches the primary modification in bootstrap.sh.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch partner-start-conditional

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator

@kbsingh kbsingh left a comment

Choose a reason for hiding this comment

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

just two minor comments, looks good otherwise

Comment thread bootstrap.sh Outdated
#
echo -p "Deploying Partner OverLay .. " -n1 -s
if [ -f ./partner-install/start.sh ]; then
./partner-install/start.sh ${workingDir}/ocp-cluster/auth/kubeconfig ${global_vars} ${certs_vars} 2>&1 >> ${log}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is it worth checking the script is +x, or maybe force it with a /usr/bin/bash ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

minor other point would be if the global_vars and certs_var's are absolute references (ie, consuming when needed the working_dir prefix )

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

executing the script with bash. the absolute reference to vars i'll keep for a future PR.

@maorfr maorfr force-pushed the partner-start-conditional branch from 9f3de34 to 15ab05a Compare March 6, 2026 08:10
@maorfr maorfr enabled auto-merge March 6, 2026 17:32
@maorfr maorfr disabled auto-merge March 6, 2026 17:33
@maorfr maorfr merged commit 20f258a into main Mar 6, 2026
14 checks passed
@maorfr maorfr deleted the partner-start-conditional branch March 6, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants