Skip to content

OSAC-3352: upload snapshot creation scripts to scripts/ - #35

Closed
omer-vishlitzky wants to merge 3 commits into
osac-project:mainfrom
omer-vishlitzky:feat/OSAC-3352-upload-snapshot-scripts
Closed

OSAC-3352: upload snapshot creation scripts to scripts/#35
omer-vishlitzky wants to merge 3 commits into
osac-project:mainfrom
omer-vishlitzky:feat/OSAC-3352-upload-snapshot-scripts

Conversation

@omer-vishlitzky

Copy link
Copy Markdown
Contributor

Summary

  • Add Python scripts (scripts/) that automate the full OSAC snapshot flavor pipeline: boot base SNO, install OSAC components via Helm, validate cluster health, strip credentials, snapshot VM, and push to OCI registry
  • Includes snapshot_base.py (base SNO), snapshot_caas.py (CaaS flavor), snapshot_vmaas.py (VMaaS flavor), shared snapshot_lib.py, and kubeletconfig.yaml
  • Hardcoded paths replaced with environment variables (INSTALLER_DIR, KUBELETCONFIG, PULL_SECRET) for portability; sys.path hacks removed in favor of same-directory imports

Test plan

  • Verify snapshot_lib.py resolves INSTALLER_DIR and KUBELETCONFIG correctly via env vars and defaults
  • Run snapshot_base.py with SOURCE and KUBECONFIG set against a live SNO
  • Run snapshot_caas.py / snapshot_vmaas.py end-to-end on a baremetal server
  • Confirm pushed images appear in quay.io/osac-project/cluster-flavors

Add Python scripts that automate the full OSAC snapshot flavor pipeline:
boot SNO, install OSAC via Helm, validate health, strip credentials,
snapshot, and push to OCI registry. Includes base, CaaS, and VMaaS
flavor scripts plus shared library and kubeletconfig manifest.

Paths are configurable via environment variables instead of hardcoded.

Signed-off-by: Omer Vishlitzky <ovishliz@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: omer-vishlitzky <omer.vishlitzky@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@omer-vishlitzky, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 82e894a3-5345-4122-b06c-f3e68dc3cc0a

📥 Commits

Reviewing files that changed from the base of the PR and between ade62ac and ebde99f.

📒 Files selected for processing (6)
  • README.md
  • scripts/kubeletconfig.yaml
  • scripts/snapshot_base.py
  • scripts/snapshot_caas.py
  • scripts/snapshot_lib.py
  • scripts/snapshot_vmaas.py

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.

@omer-vishlitzky

Copy link
Copy Markdown
Contributor Author

Code Review Findings

MAJOR — run() hardcodes cwd=INSTALLER_DIR for all subprocess calls

scripts/snapshot_lib.py:58 — Every oc and cluster-tool invocation uses cwd=str(INSTALLER_DIR). This means snapshot_base.py (which imports oc(), run(), cluster_tool()) will crash with FileNotFoundError on any machine without an osac-installer checkout — even though snapshot_base.py has no dependency on osac-installer.

Fix: Make cwd a parameter of run() defaulting to None. Only pass cwd=str(INSTALLER_DIR) at the call site that needs it (run(["make", "install"], ...) in create_snapshot() step 4).

MAJOR — Must use OSAC baremetal infrastructure

These snapshot creation scripts require libvirt, cluster-tool, and bare metal hardware — they boot VMs, run oc commands against live clusters, snapshot disks, and push to OCI registries. OSAC already has self-hosted GitHub Actions runners on baremetal machines registered at the org level with the osac-ci label. These machines have everything pre-installed via machine-init.sh (libvirt, qemu-kvm, podman, cluster-tool, pigz, skopeo, etc.) and are connected to Vault for secrets.

Any GHA workflows invoking these scripts should use runs-on: [self-hosted, osac-ci] and fetch secrets from Vault via AppRole (see the fetch-secrets composite action in osac-test-infra), matching the existing E2E workflow convention.

MINOR — Dead code in run_health_checks()

scripts/snapshot_lib.py:163 — The else branch is unreachable. All HealthCheck instances use command="rollout" or command="wait". If reached, .split() on a list raises AttributeError.

Fix: Replace with raise ValueError(f"Unknown health check command: {hc.command}").

MINOR — callable vs Callable type annotation

scripts/snapshot_lib.py:38 — Uses callable (builtin function) instead of Callable from typing. Static type checkers won't validate this.

MINOR — Missing init container health checks

scripts/snapshot_lib.py:112check_no_crashing_pods() only inspects containerStatuses, ignoring initContainerStatuses. A pod stuck in init CrashLoopBackOff passes the check. A snapshot could be created from a cluster with failing init containers.

Fix: Also iterate over initContainerStatuses.

NIT — Scripts not executable

All three entry-point scripts have shebangs but 100644 mode. Run git update-index --chmod=+x scripts/snapshot_base.py scripts/snapshot_caas.py scripts/snapshot_vmaas.py.

NIT — Missing kubeconfig existence check

After boot, kubeconfig path is assumed without verification. If boot writes it elsewhere or fails silently, subsequent oc commands fail with a misleading error.


Overall design is solid — good separation between shared library and per-flavor scripts, proper credential stripping with verification, clean dataclass-based configuration.

Signed-off-by: omer-vishlitzky <omer.vishlitzky@gmail.com>
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: omer-vishlitzky <omer.vishlitzky@gmail.com>
Signed-off-by: omer-vishlitzky <omer.vishlitzky@gmail.com>
Assisted-by: Claude Code <noreply@anthropic.com>
@omer-vishlitzky

Copy link
Copy Markdown
Contributor Author

Superseded by #38 (unified snapshot pipeline PR)

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.

1 participant