feat: add merge queue to osac-test-infra, grant wg-infra bypass on github-config - #186
Merged
omer-vishlitzky merged 2 commits intoAug 11, 2026
Conversation
WalkthroughThe changes add conditional merge queue bypass and pull-request enforcement to the repository ruleset. Repository configurations grant ChangesMerge queue rulesets
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MergeQueue
participant RepositoryRuleset
participant GitHubRepository
MergeQueue->>RepositoryRuleset: Push rebased commit
RepositoryRuleset->>GitHubRepository: Apply Integration bypass
GitHubRepository-->>MergeQueue: Accept commit
RepositoryRuleset->>GitHubRepository: Require pull request
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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 `@modules/common_repository/main.tf`:
- Around line 150-153: Separate the update restriction from the ruleset that
uses ruleset_bypass_team_ids: create or reuse a dedicated ruleset for update
with only approved bypass actors, and remove update from the shared ruleset
controlled by the update expression in the Terraform configuration. Preserve
merge-queue behavior while ensuring wg-infra cannot bypass the update
restriction.
In `@repositories.tf`:
- Around line 315-334: Do not enable the merge_queue configuration in the
ruleset until osac-test-infra deploys workflows with merge_group triggers that
emit all four required contexts: e2e-vmaas-gate, e2e-bmaas-gate, e2e-caas-gate,
and check-labels. Keep the queue settings unchanged once those workflow changes
are available.
🪄 Autofix
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: osac-project/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: da619d4d-bbaf-4b9f-9397-a81d7cd46158
📒 Files selected for processing (2)
modules/common_repository/main.tfrepositories.tf
omer-vishlitzky
added a commit
to osac-project/osac-test-infra
that referenced
this pull request
Aug 11, 2026
…348) Add workflows required for GitHub merge queue support: - label-gate.yml: required status check that validates lgtm, approved, and jira/valid-reference labels are present - auto-queue.yml: enables auto-merge for collaborator PRs, respects do-not-merge/hold and needs-rebase blocking labels - Add merge_group trigger to all three e2e caller workflows so they run on merge queue entries - Add gate jobs (e2e-vmaas-gate, e2e-bmaas-gate, e2e-caas-gate) as required status checks for the merge queue Depends on: osac-project/github-config#186 Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]>
…thub-config osac-test-infra: - Add merge queue with same settings as osac (REBASE, ALLGREEN, batch 4) - Add required status checks (e2e gates + check-labels) - Add wg-infra as ruleset bypass team - Remove push_allowances (conflicts with merge queue bot) - Depends on: label-gate, e2e gate jobs, and auto-queue workflows being added to osac-test-infra repo first github-config: - Add wg-infra team with admin permission - Add wg-infra as ruleset bypass team (can force merge) Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]>
The update rule blocks ALL ref updates including auto-merge evaluation,
causing "Cannot update this protected ref" on every PR. The pull_request
rule blocks direct pushes ("Changes must be made through a pull request")
without interfering with the merge queue or auto-merge.
required_approving_review_count = 0 because native GitHub reviews are
not used — approval is handled by Prow labels and the check-labels gate.
Assisted-by: Claude Code <[email protected]>
Signed-off-by: omer-vishlitzky <[email protected]>
omer-vishlitzky
force-pushed
the
fix/merge-queue-bypass
branch
from
August 11, 2026 14:18
8e92f7f to
b78026a
Compare
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 `@modules/common_repository/main.tf`:
- Around line 150-159: Move the dynamic pull_request rule out of the ruleset
that grants wg-infra bypass_mode = "always", placing it in a separate ruleset
without that team bypass. Preserve the merge_queue condition and
required_approving_review_count = 0 so direct pushes remain blocked while
merge-queue behavior is unchanged.
🪄 Autofix
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: osac-project/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 32dba2ce-f757-4334-8d13-88a12902d684
📒 Files selected for processing (1)
modules/common_repository/main.tf
eranco74
approved these changes
Aug 11, 2026
This was referenced Aug 11, 2026
danmanor
added a commit
to danmanor/osac-test-infra
that referenced
this pull request
Sep 7, 2026
* NO-ISSUE: fix Keycloak identity mismatch after snapshot restore (#340) After restoring from a caas-4-22 snapshot on a new cluster, the Keycloak deployment retains the old cluster's KC_HOSTNAME, TLS certificate SANs, and OIDC issuer URL. This causes fulfillment-controller to crash (token issuer not trusted) and fulfillment-grpc-server to reject requests (TLS verification failure). Fix by patching three things in prep-refresh-osac, before the Helm upgrade runs: 1. KC_HOSTNAME env var on the Keycloak deployment — so tokens carry the correct issuer claim for the new cluster domain. 2. keycloak-tls Certificate dnsNames — adds the new external route hostname and forces cert-manager to reissue, preventing TLS verification failures. 3. Helm values file (auth.issuerUrl, idp.url) — so the Helm upgrade renders the correct --grpc-authn-trusted-token-issuers, --auth-issuer-url, and --idp-url args on the fulfillment deployments. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * NO-ISSUE: Add fork PR authorization to e2e-caas-netris-full-install (#341) * NO-ISSUE: Add fork PR authorization to e2e-caas-netris-full-install Every other e2e reusable workflow (e2e-bmaas/vmaas/caas-full-install) gates fork PR execution behind the authorize-fork-pr composite action (ok-to-test label or org-membership check via a Vault-fetched token) before touching any real secrets or infra. This one had no such gate at all -- it accepted fork-pr-author-association/-author/pr-number inputs from nowhere and never checked them, so any fork PR could trigger real EC2 provisioning and Vault secret fetches unauthorized. Adds the same 3 inputs and the same Authorize fork PR step, in the same position (right after setup, before any Vault secret fetch, so an unauthorized fork PR fails fast) as the sibling workflows already use. * Gate provision on fork-PR authorization, not just push-secrets authorize-fork-pr lived inside push-secrets, which runs after provision (needs: provision) -- an unauthorized fork PR could still trigger real EC2 provisioning before ever hitting the authorization check, exactly the cost/blast-radius this gate exists to prevent. Extract checkout + validate-and-bootstrap + authorize-fork-pr into a new authorize job that runs first, and make provision need it. It's lightweight (local AppRole files already on the orchestrator + a couple of GitHub API calls), no dependency on the box existing. Signed-off-by: Elior Erez <[email protected]> --------- Signed-off-by: Elior Erez <[email protected]> Co-authored-by: Elior Erez <[email protected]> * NO-ISSUE: Add metadata to setup-caas API payloads (#346) * NO-ISSUE: Add fork PR authorization to e2e-caas-netris-full-install Every other e2e reusable workflow (e2e-bmaas/vmaas/caas-full-install) gates fork PR execution behind the authorize-fork-pr composite action (ok-to-test label or org-membership check via a Vault-fetched token) before touching any real secrets or infra. This one had no such gate at all -- it accepted fork-pr-author-association/-author/pr-number inputs from nowhere and never checked them, so any fork PR could trigger real EC2 provisioning and Vault secret fetches unauthorized. Adds the same 3 inputs and the same Authorize fork PR step, in the same position (right after setup, before any Vault secret fetch, so an unauthorized fork PR fails fast) as the sibling workflows already use. * Gate provision on fork-PR authorization, not just push-secrets authorize-fork-pr lived inside push-secrets, which runs after provision (needs: provision) -- an unauthorized fork PR could still trigger real EC2 provisioning before ever hitting the authorization check, exactly the cost/blast-radius this gate exists to prevent. Extract checkout + validate-and-bootstrap + authorize-fork-pr into a new authorize job that runs first, and make provision need it. It's lightweight (local AppRole files already on the orchestrator + a couple of GitHub API calls), no dependency on the box existing. Signed-off-by: Elior Erez <[email protected]> * NO-ISSUE: Add metadata to setup-caas API payloads The fulfillment-service now requires a metadata object with a name field on all resource creation requests (OSAC-3263). The setup-caas host_type and cluster_catalog_item POST requests were sending the old top-level id field, causing 400 "metadata is required" errors in the e2e-caas-netris periodic runs. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> --------- Signed-off-by: Elior Erez <[email protected]> Signed-off-by: Dan Manor <[email protected]> Co-authored-by: Elior Erez <[email protected]> * OSAC-3264: Add e2e test for name validation (#333) Tests creating objects with valid/invalid names to ensure that the validation works correctly. Assisted-by: Claude Code <[email protected]> * OSAC-3402: Add E2E test for bare metal instance restart (#302) * OSAC-3402: Add E2E test for bare metal instance restart Add a dedicated test for BMI restart (power cycle) that verifies the full restart_trigger flow: increment spec.restart_trigger via Update RPC, observe RESTART_IN_PROGRESS condition, wait for status.restart_trigger echo, confirm host comes back online, and verify BMI returns to RUNNING state. Also adds update_baremetal_instance_restart_trigger() helper to GRPCClient for field-masked restart_trigger updates. Note: This test depends on OSAC-3548 (fulfillment controller premature trigger echo race condition) being fixed to pass. Assisted-by: Claude Code <[email protected]> Signed-off-by: MENNY ABOUSH <[email protected]> * OSAC-3402: Remove transient RESTART_IN_PROGRESS poll The RESTART_IN_PROGRESS condition is transient and cannot be reliably observed — it may flip TRUE→FALSE between poll intervals. Use status.restart_trigger echo as the authoritative restart completion signal instead. Assisted-by: Claude Code <[email protected]> Signed-off-by: MENNY ABOUSH <[email protected]> * OSAC-3402: Address review — observe RESTART_IN_PROGRESS, remove redundant assertion Add back poll for RESTART_IN_PROGRESS == CONDITION_STATUS_TRUE. With the IsRestartComplete fix (osac/PR #129) the operator now waits for poweredOn=true before completing, so the TRUE window is wide enough (~10-20s) to reliably observe. Remove the redundant final_trigger assertion — the trigger echo poll already guarantees status.restart_trigger == new_trigger. Assisted-by: Claude Code <[email protected]> Signed-off-by: MENNY ABOUSH <[email protected]> --------- Signed-off-by: MENNY ABOUSH <[email protected]> Co-authored-by: MENNY ABOUSH <[email protected]> * feat: add merge queue workflows (label-gate, auto-queue, e2e gates) (#348) Add workflows required for GitHub merge queue support: - label-gate.yml: required status check that validates lgtm, approved, and jira/valid-reference labels are present - auto-queue.yml: enables auto-merge for collaborator PRs, respects do-not-merge/hold and needs-rebase blocking labels - Add merge_group trigger to all three e2e caller workflows so they run on merge queue entries - Add gate jobs (e2e-vmaas-gate, e2e-bmaas-gate, e2e-caas-gate) as required status checks for the merge queue Depends on: osac-project/github-config#186 Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * OSAC-3434: Add VMaaS metering lifecycle E2E tests (#301) * OSAC-3434: Add cross-cutting concern test pattern and metering infrastructure Establish the composable fixture pattern for testing features that span multiple domains (metering, storage, networking, catalog): - Verifier class in tests/core/<concern>.py - Fixture + marker + skip logic in tests/conftest.py - Tests annotate with @pytest.mark.<concern> and inject fixture Metering implementation: - kafka-python-ng dependency for Kafka consumption - MeteringCollector: background Kafka consumer with expect/verify. Validates CloudEvent structure (specversion, source, tenant, billing dimensions, schema version) on every matched event. - metering fixture: starts collector, yields, verifies + stops - @pytest.mark.metering marker, auto-skip without KAFKA_BOOTSTRAP_SERVERS - test-metering Makefile target (pytest -m metering) Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> OSAC-3434: Add metering verification to VMaaS lifecycle tests Annotate existing VMaaS tests with @pytest.mark.metering and inject the metering fixture. No new test files — metering verification is composable on top of existing lifecycle tests. Annotated tests: - test_compute_instance_lifecycle: created.v1 + started.v1 + deleted.v1 - test_compute_instance_delete_during_provision: created.v1 + deleted.v1 - test_compute_instance_restart: created.v1 + started.v1 When KAFKA_BOOTSTRAP_SERVERS is not set, these tests run without metering verification (marker auto-skips the fixture). Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> OSAC-3434: Fix consumer race condition and billing_dimensions assertion - Wait for Kafka partition assignment before returning from start(). First poll() triggers partition assignment; threading.Event gates start() until consumer is ready. Prevents missing early events. - Assert billing_dimensions contains instance_type, image_ref, and boot_disk_size_gib per OSAC-3434 AC, not just key existence. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> OSAC-3434: Add missing metering ACs — restart events and short-lived VM - Restart test: add suspended.v1 + started.v1 expectations during the restart cycle (VM stops then starts — both transitions must produce metering events) - Short-lived VM test (CAP-4): create and immediately delete a VM without waiting for Running. Verify created.v1 + deleted.v1 appear on Kafka — validates sub-minute billing granularity Note: AC2 checks suspended.v1 event type but not previous_state field (always null in Phase 1, no state tracking). Full AC2 coverage in Phase 2 when previous_state is populated. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> OSAC-3434: Fail hard when metering tests run without Kafka If @pytest.mark.metering tests are collected but KAFKA_BOOTSTRAP_SERVERS is not set, fail immediately instead of silently skipping. Missing Kafka when metering is expected is a configuration bug, not a graceful degradation. Use -m 'not metering' to explicitly exclude metering tests. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> OSAC-3434: Fix review findings — event dedup, consumer robustness, docstring - Track matched events: _matched set prevents the same Kafka event from satisfying multiple expectations. Restart test expecting two started.v1 events now requires two distinct events. - Propagate consumer errors: if KafkaConsumer constructor fails (bad creds, TLS, unreachable), the exception is saved and re-raised from start() instead of a generic timeout message. - Wait for partition assignment: loop on consumer.assignment() with bounded retries instead of a single poll(timeout_ms=0). - Guard leaked thread: fixture wraps start() in try/except, calls stop() on failure before re-raising. - Fix displaced docstring in pytest_configure. - Fix unused loop variable (records.values() not records.items()). Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> OSAC-3434: Signal ready on both success and failure paths - Set _ready on exception path so start() surfaces the error immediately instead of waiting 30s timeout - Check assignment after poll loop — fail if no partitions assigned instead of silently proceeding with no assignment Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> OSAC-3434: Pass Kafka credentials to VMaaS E2E test container Extract Kafka bootstrap address, SASL password, and CA cert from the deployed cluster and pass them to the test container. Without these, the MeteringCollector can't connect to Kafka and metering tests fail at collection time. Only set when Kafka is deployed (kafka cluster exists in osac-kafka namespace). SASL password masked in workflow logs. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix(pr-review): switch to test adapter HTTP API, add heartbeat test Replace direct Kafka consumer with HTTP client querying the metering test adapter's /events endpoint. The test adapter runs inside the cluster — CI creates an OpenShift Route to make it externally accessible. Eliminates the cluster-internal DNS resolution issue that caused all metering tests to ERROR on teardown. MeteringCollector: - HTTP-based: queries GET /events?type=...&resource_id=...&since=... - No background thread, no Kafka dependency, no SSL config - Per-expectation timeout (default 60s, heartbeat uses 120s) - Enhanced structure validation: numeric boot_disk_size_gib, project_id presence, RFC3339 transition_time, previous_state/duration_seconds for state transition events Tests: - New: test_compute_instance_heartbeat — verifies heartbeat events appear for a RUNNING VM within 120s (Phase 2 heartbeat generator) - Fix: restart test expects resumed.v1 (not started.v1) for STOPPED→RUNNING transition - Rename: test_metering_short_lived_vm → test_compute_instance_short_lived_metering (AGENTS.md naming convention) CI: - Replace Kafka credential extraction with oc expose route for test adapter; single METERING_ADAPTER_URL env var replaces 5 Kafka vars - Remove kafka-python-ng dependency Review feedback (masayag): - Remove unused config param from pytest_collection_modifyitems - Fix AGENTS.md stale path (tests/concerns/conftest.py → tests/conftest.py) Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix(pr-review): HTTP resilience, stop-VM test, structure validation HTTP resilience: - Wrap _fetch_events in try/except inside poll_until find() closure. Transient HTTP errors (URLError, JSONDecodeError, OSError) are logged and retried instead of crashing the test immediately. Stop-VM metering test (AC 2): - New test_compute_instance_stop_metering: creates VM, waits for RUNNING, patches runStrategy to Halted, waits for Stopped, verifies suspended.v1 event. Covers the "stop VM" acceptance criterion that was missing (only restart exercised suspended.v1 before). Structure validation: - Assert osacresourceid extension matches expected resource_id (previously only used for matching, not validated) Heartbeat test: - Call metering.verify() explicitly before deleting VM to ensure heartbeat assertion is checked while VM is still RUNNING Cleanup: - Remove phantom kafka-python-ng from uv.lock - Restore grpcurl health check explanatory comments in CI workflow - Add name=unique_name() to short-lived test for consistency Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> feat: comprehensive metering E2E coverage — heartbeat stop, reconciliation, deep validation Metering collector enhancements: - assert_no_events: negative assertion for verifying events do NOT appear within a time window (heartbeat stops after non-billable) - get_event: returns event data for value assertions beyond structure - verify() clears expectations after completion for multi-phase tests - suspended.v1 deep validation: previous_state must be RUNNING, duration_seconds must be positive (closes a billing interval) New tests: - test_compute_instance_stop_metering: enhanced with heartbeat-stops verification — after stopping VM, asserts no heartbeat events appear within 90s (design doc line 815) - test_compute_instance_reconciliation_metering: restarts metering pod, waits for readiness (startup reconciliation), verifies heartbeats resume for still-running VM (design doc CAP-15, line 839) CI workflow: - Remove conditional metering exclusion — metering is mandatory for vmaas, missing adapter is an infra bug that should fail immediately. Always pass METERING_ADAPTER_URL to test container. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix(pr-review): contract mismatches, validation depth, robustness Contract fixes (blocking): - Skip transition_time assertion for heartbeat events (heartbeatData has no transition_time field — generator.go:179) - Accept "osac-metering/reconciler" as valid source (synthetic heartbeats from reconciler use different source — reconciler.go:389) Validation depth: - osacresourcetype asserts exact value "compute_instance" (was presence) - resumed.v1 previous_state asserts value in (STOPPED, PAUSED) - assert_no_events uses datetime parsing instead of string comparison (Z vs +00:00 format difference) - assert_no_events error includes event IDs and timestamps - verify() warns when called with no expectations Robustness: - Workflow: check route host non-empty before setting URL; emit ::error:: if adapter unavailable; quote METERING_ADAPTER_URL - Heartbeat timeout 180s (was 120s) — 3x interval for margin - Reconciliation test VM cleanup in try/finally block Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix: get_event after verify, billing dimensions truthiness get_event() after verify() was dead code — verify() added matched event IDs to _matched, then get_event() skipped them and timed out. The billing dimensions value check in test_compute_instance_lifecycle never executed. Fix: verify() now stores matched events in _verified dict; get_event() returns from cache if available. billing_dimensions: instance_type and image_ref now checked for truthiness (not just key presence), catching empty string values. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix: enable metering test adapter in CI install Add --set metering.testAdapter.enabled=true to Helm install args. The test adapter pod is required for metering E2E tests — without it, the route creation step fails and pytest_collection_modifyitems correctly aborts the session (metering is mandatory for vmaas). Also fix get_event after verify (verified events cached in _verified dict) and billing_dimensions truthiness checks. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> Revert "fix: enable metering test adapter in CI install" This reverts commit 2d0d014f71ab1a37d78c97b50ad62b750676f330. fix: suspended.v1 previous_state accepts transient states, K8sClient API fix suspended.v1 validation: - Accept RUNNING, STOPPING, STARTING as valid previous_state values (was: RUNNING only). A VM goes RUNNING→STOPPING→STOPPED, so the suspended.v1 event may capture STOPPING as previous_state. Reconciliation test: - Use runner.run_unchecked instead of K8sClient.run (which doesn't exist) for pod deletion and readiness polling. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix: allow null duration_seconds in suspended.v1 When a VM transitions RUNNING→STOPPING→STOPPED, the billing interval closes on the RUNNING→STOPPING transition (updated.v1 with duration). The subsequent STOPPING→STOPPED transition emits suspended.v1 with null duration_seconds because no billing interval was open. This is correct behavior — the assertion should accept null OR positive. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix: relax instance_type value check (proto mismatch), stop previous_state instance_type value check removed from lifecycle test — metering service proto needs regeneration after PR #85 changed instance_type from optional string to InstanceTypeReference. The generic _validate_structure still checks instance_type is present and non-empty. Value check can be restored after metering proto is regenerated. Stop test: accept STOPPING as valid previous_state alongside RUNNING. VM transitions RUNNING→STOPPING→STOPPED; the suspended.v1 event may capture either as previous_state depending on timing. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> Revert "fix: relax instance_type value check (proto mismatch), stop previous_state" This reverts commit 1e7aae7b1c9cf0dbf11bfd75b68e493731237266. fix: resolve echo-adapter Service by label, fail fast if missing oc expose svc/osac-metering-test-adapter targets a name that no longer exists after the test-adapter to echo-adapter rename — the Service never gets exposed, and the swallowed errors (2>/dev/null || true, || echo "") let the job limp forward with an empty METERING_ADAPTER_URL instead of failing at the source. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix: match resource_id field from echo-adapter response The echo-adapter returns resource_id at the top level (extracted from the osacresourceid extension). Add it as first match check alongside osacresourceid and data.resource_id. Depends on osac-project/osac#250 Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> fix: heartbeat race in stop metering test, ensure cleanup on failure Wait one heartbeat interval after suspended.v1 before asserting no more heartbeats — the projection DB lags behind Kafka, so one stale heartbeat sweep can fire after the stop is confirmed. Wrap post-stop assertions in try/finally so the compute instance is always cleaned up, matching test_compute_instance_reconciliation_metering. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: remove reconciliation test, restore stop_metering original timing Remove test_compute_instance_reconciliation_metering — it restarts the metering pod, which disrupts all other metering tests running in parallel via xdist (Watch stream replay causes spurious resumed.v1 events on stopped VMs in other workers). Will re-enable once a disruptive test isolation pattern is established. Tracked in OSAC-3918. Restore test_compute_instance_stop_metering to original form — the heartbeat race was caused by the reconciliation test's pod restart, not by projection lag. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: wait for heartbeat drain before asserting silence after VM stop The heartbeat generator queries the projection DB, which lags behind Kafka publication (publishAndUpsert publishes first, updates DB second). One more heartbeat sweep can fire against the stale projection after suspended.v1 is confirmed. Wait 90s (> heartbeat interval) to let in-flight heartbeats drain before starting the no-heartbeat assertion window. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: stop VM via gRPC API instead of K8s CR patch The test patched the ComputeInstance K8s CR directly to set runStrategy=Halted. The fulfillment-service API record still had run_strategy=Always, so the controller reconciled the discrepancy and restarted the VM — causing a spurious resumed.v1 event and legitimate heartbeats that failed the assert_no_events check. Use the private gRPC UpdateComputeInstance RPC with an update mask on spec.run_strategy instead. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> --------- Signed-off-by: omer-vishlitzky <[email protected]> * fix: sync auto-queue with osac — dismiss reviews, remove lgtm on push Sync auto-queue.yml from osac monorepo: - Dismiss all CHANGES_REQUESTED reviews on every PR event - Remove lgtm label on new push/rebase (synchronize) - Add pull_request_review trigger for immediate dismiss - Add scheduled requeue job (every 10 min) for ejected PRs - Add workflow_dispatch for manual requeue - Check all blocking labels (do-not-merge/*, needs-rebase) Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * NO-ISSUE: fix E2E flakes — retry metering HTTP, serialize BMaaS tests Add retry logic to MeteringCollector._fetch_events to handle transient OpenShift router connection drops during HAProxy reloads. Serialize BMaaS tests with xdist_group to avoid concurrent Ironic provisioning through single-threaded sushy-emulator, which caused ~19% timeout failures. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * NO-ISSUE: revert bmaas xdist_group — loadfile ignores group markers Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * Revert "OSAC-3402: Add E2E test for bare metal instance restart (#302)" This reverts commit 302996daa83e4a10b00e3a83518305a7a72055b9. * OSAC-1330: add references E2E test suite infrastructure Add shared fixtures, helper methods, and Makefile target for the typed resource reference E2E test suite (OSAC-3095/3100/3105/3110/3114). - tests/references/conftest.py: session-scoped fixtures for network class, virtual network, subnet, and security group with cleanup - tests/core/helpers.py: assert_grpc_field_violation for reference validation error assertions - tests/core/grpc_client.py: methods for NATGateway, RoleBinding, ProjectMembership CRUD and generic filtered list - Makefile: test-references target Assisted-by: Claude Code <[email protected]> Signed-off-by: Haim Tayrie <[email protected]> * OSAC-3095: add E2E tests for networking resource references Test VirtualNetwork→NetworkClass, Subnet→VN, and SecurityGroup→VN name-based references, invalid reference field path errors, CEL filter by nested reference path, and cross-tenant NetworkClass access. Assisted-by: Claude Code <[email protected]> Signed-off-by: Haim Tayrie <[email protected]> * OSAC-3100: add E2E tests for compute resource references Test full CI provisioning chain with name-based catalog item, subnet, and security group references, CI reaches Running state, invalid SG name returns array-indexed field path, cross-tenant template reference, and instance type deprecation replacement reference resolution. Assisted-by: Claude Code <[email protected]> Signed-off-by: Haim Tayrie <[email protected]> * OSAC-3105: add E2E tests for IP management resource references Test ExternalIP→Pool by name, NATGateway→VN+ExternalIP by name, invalid attachment target returns field path error, and cross-tenant pool reference access. Assisted-by: Claude Code <[email protected]> Signed-off-by: Haim Tayrie <[email protected]> * OSAC-3110: add E2E tests for cluster and bare metal resource references Test ClusterCatalogItem→Template and Cluster→CatalogItem by name, BareMetalInstanceCatalogItem→Template and BMI→CatalogItem by name, cross-tenant cluster template reference, and invalid template name error reporting. Assisted-by: Claude Code <[email protected]> Signed-off-by: Haim Tayrie <[email protected]> * OSAC-3114: add E2E tests for IAM resource references Test RoleBinding→Role+Users by name with reference resolution, ProjectMembership→Users by name, invalid role name error reporting, and multi-user role binding reference resolution. Fix create_project_membership to match proto spec (role enum + users list). Assisted-by: Claude Code <[email protected]> Signed-off-by: Haim Tayrie <[email protected]> * OSAC-1330: address validation review findings Restructure cleanup in test finally blocks to delete via API first (always runs) then best-effort wait for CR deletion, preventing resource leaks when wait helpers timeout. Fix cross-tenant compute test to move CI delete into finally block. Use proper cr_name=None initialization instead of dir() check. Fix Generator return type annotation on yield fixture. Assisted-by: Claude Code <[email protected]> Signed-off-by: Haim Tayrie <[email protected]> * OSAC-1330: add references E2E step to vmaas workflows Run tests/references/ as a follow-up step in both the snapshot and full-install E2E workflows, reusing the same deployed cluster. The step runs when test-suite is 'vmaas' or empty (the default PR/schedule paths) and is skipped on cancellation. Co-Authored-By: Claude Opus 4.6 <[email protected]> * OSAC-1330: collect references JUnit XML in gather artifacts Pick up any junit-*.xml files alongside the primary junit.xml so the references test results are included in the uploaded artifact. Co-Authored-By: Claude Opus 4.6 <[email protected]> * OSAC-1330: fix references E2E test failures and CI workflow - Remove references step from e2e-vmaas (full-install covers it) - Fix result output in full-install to include references outcome - Add instance_type to compute reference tests (now required) - Add version to cluster creation (required by API) - Use SA token for IAM tests (JWT tenant lacks permissions) - Skip IP management tests when no ExternalIPPools deployed - Fix networking tests to use Get after Create for ref validation Co-Authored-By: Claude Opus 4.6 <[email protected]> * OSAC-1330: fix references test failures and merge into main E2E step - Fix camelCase response keys (catalogItem, networkAttachments, securityGroups) - Use CLI for Clusters/Create (version field is a message type, not string) - Add user auto-registration fixture and skip-on-not-found for IAM tests - Catch TimeoutError in cleanup blocks across networking and compute tests - Merge references tests into existing "Run E2E tests" step (vmaas suite) Co-Authored-By: Claude Opus 4.6 <[email protected]> * OSAC-1330: discover cluster version from environment for cluster creation tests Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * NO-ISSUE: Prune dangling podman images and orphaned sushy boot ISOs in teardown Every E2E run leaves two kinds of garbage behind that teardown.sh never cleaned up: untagged intermediate layers from multi-stage podman builds, and sushy-tools' own tempfile.mkdtemp() cache of Redfish virtual-media boot ISOs for BMaaS. Neither is scoped to a container or a tracked path, so nothing removed them -- fleet-wide this had grown to multiple TB of pure garbage across all runner hosts, one host over 70% full. Both cleanups are safe to run unconditionally on every job: image prune is dangling-only (never touches a tagged or in-use image), and the ISO sweep only removes a directory if lsof finds no live process still holding its ISO open. * NO-ISSUE: Make image/ISO cleanup job-scoped, move podman prune to a schedule The previous commit's blanket 'podman image prune -f' in teardown.sh and its unscoped /tmp sweep were both too broad: every host's runner instances share one podman store and /tmp, so either could touch another concurrent job's still-in-progress build or still-attached boot ISO. - Move the podman prune out of teardown.sh entirely into a new scheduled workflow (fleet-podman-cleanup.yml), matrixed one job per host, gated on --filter until=6h -- well past the longest E2E run on record -- so it runs independently of any specific job and can never touch an image a real build could still need. The workflow_dispatch override is a fixed choice list (6h/12h/24h/48h) rather than free text, so a manual run can't be given an unsafe value; it also declares empty top-level permissions since it never touches the GitHub API. - Scope the sushy boot-ISO cleanup deterministically instead of guessing: setup-virtual-bmh.sh points TMPDIR at a directory named for CLONE_NAME before starting sushy-emulator (it resolves its virtual-media cache through tempfile.gettempdir(), which honors TMPDIR), so teardown.sh can just remove that directory by name -- no marker files, timestamp comparisons, or lsof-based liveness checks needed. - Register the fleet's per-host runner labels in actionlint.yaml -- needed now that fleet-podman-cleanup.yml is the first workflow to reference them as literal matrix values instead of through an expression-typed runner-label input. * NO-ISSUE: Make fleet podman cleanup skip busy images instead of aborting The first-ever run of fleet-podman-cleanup.yml (#319) failed on 8 of 11 hosts: - osac-6 failed fast: `podman image prune` removes its whole candidate list as one operation, so one dangling image still referenced by a live job's in-progress build aborted the entire host's prune, leaving every other safely-removable image on that host untouched too. - The other 7 hosts hit the job's 10-minute timeout outright, working through months of accumulated backlog (thousands of images, 600+GB on some hosts) -- ordinary for a first run against unbounded prior growth, but the timeout wasn't sized for it. Fixed both: - Prune images one at a time instead of as a single batch. A busy image just gets skipped and logged -- it's still dangling and still gated by the same `until` filter, so it's picked up on the very next run instead of blocking every other image on that host in the meantime. - Raised timeout-minutes from 10 to 60 to get through the one-time backlog; steady-state runs only have ~6h of new dangling images to catch up on and should finish in a small fraction of that ceiling. Verified: dry-ran the dangling-image listing and the full removal loop live against osac-9's real podman store. * NO-ISSUE: Branch podman rmi on exit code, detect a failed image listing Two robustness gaps in the per-image removal loop from #356: - podman rmi's exit code was collapsed to a plain success/skip boolean. Verified live: exit 2 specifically means "still referenced by a container" (the expected, safe-to-defer case this loop exists to handle), exit 1 means the image is already gone (e.g. removed alongside a shared parent layer earlier in the same loop -- the goal is already achieved, so this counts as removed), and anything else (125, or an unrecognized code) is a real failure that was being silently logged as just another "skipped" instead of surfacing. - The dangling-image listing used `mapfile -t x < <(podman images ...)`: process substitution, whose exit status is invisible to the consuming command -- $? reflects mapfile's own read, not podman's. A broken `podman images` call would silently look identical to "0 dangling images found" instead of the real error it is. Switched to command substitution so the failure is actually checked. Verified live against osac-9's real podman store: exit 2 correctly hit for a genuinely in-use image, a nonexistent image ID correctly counted as already-removed (exit 1), script exits 0 under set -euo pipefail despite handling nonzero exit codes throughout the loop. Skipped one review comment after verifying it against real data: adding `-a` to the `podman images` dangling query. Tested on osac-9 -- identical count (75) with and without `-a`, since `dangling=true` already selects every untagged, unreferenced image regardless of that flag; `-a` only affects visibility of intermediate layers belonging to still-tagged images, which by definition aren't dangling. * Reduce org-runner-exporter poll interval and pin rootless podman storage paths org-runner-exporter defaulted to a 20s poll interval (unset REFRESH_INTERVAL), far more aggressive than this dashboard data needs and sharing PRIVATE_GITHUB_TOKEN's rate-limit budget with workflow-exporter's 90s interval; this was implicated in a sustained 502/503 failure pattern against GitHub's runners API. Set it to 60s to match that same order of magnitude. Separately, podman 6.0.0 was observed on osac-ci-1 failing to auto-detect rootless storage paths on service restart, falling back to root-mode defaults and breaking any quadlet unit that restarted after the upgrade. machine-init.sh now writes an explicit storage.conf with the correct rootless runroot/graphroot so this can't silently regress again; this is a no-op on machines where auto-detection already works correctly. * NO-ISSUE: Harden storage.conf creation against symlink attacks Reject existing symlinks (including dangling ones) and use atomic tmpfile + mv -T instead of direct cat-redirect, so root-owned writes cannot follow a runner-controlled path. * NO-ISSUE: Pin Infinity datasource plugin version to fix broken Grafana panels GF_INSTALL_PLUGINS re-downloads from scratch on every container restart (every deploy restarts this service), so an unpinned version drifts to whatever's newest at that exact moment with zero soak time. 3.11.2 (published only hours before a routine restart picked it up) ships a broken build that bundles react/jsx-runtime in a way Grafana 12.0.1's import map can't resolve, breaking every panel on this datasource ("Could not load plugin ... SystemJS Error#7"). Pinned to 3.11.1, which had two weeks of real-world use with no such report. Verified live: reinstalling with the pin (after clearing the persisted plugin directory, which Grafana won't overwrite on its own) resolves to 3.11.1 and the plugin loads without error. * NO-ISSUE: Document relay migration and add a read-only relay audit script Prompted by an actual relay-replacement check: the relay machine turned out to be a shared lab box, not a dedicated host -- it had two unrelated personal accounts and an nginx reverse proxy (predating the tunnel setup by months, not mentioned anywhere in this doc) still actively forwarding a port to Prometheus. None of that was a problem on its own, but finding it ad hoc, by hand, right before a planned host replacement is exactly what this change is meant to prevent next time. - Add audit-relay-host.sh: a read-only survey of a candidate/existing relay host -- tunnel identities and their systemd services, any other user accounts, and any nginx/httpd config already bound to the well-known tunnel ports. Never modifies anything. - Add a "Replacing the relay machine" section covering the actual gaps found: the DNS CNAME pointing directly at the specific host being replaced (confirmed live -- osac-ci.redhat.com resolves straight to the relay's own hostname, so it has to be repointed, not left alone), the cert fingerprint table needing a matching update, and the Grafana OAuth canonical-host order already documented elsewhere in this file. - Point at the audit script from the existing one-time setup section too, since checking a candidate host before first use is just as relevant as checking one before decommissioning it. * NO-ISSUE: Reap stale buildah working containers blocking image cleanup The real answer to "why do the same images get skipped every single cycle": leaked buildah "working containers" from podman build stages that never reached their own cleanup. These are invisible to a plain `podman ps -a` (need --external), and each one pins its image layer exactly like a real container would -- confirmed live on osac-9: 33 dangling images were skipped on every cleanup run regardless of how many cycles passed, and 743 stale external containers (some over 6 weeks old) were sitting there the whole time as the actual blocker. Removing them (force required -- plain `podman rm` fails with "container is mounted") immediately let all 33 images be removed cleanly on the very next prune. Surveyed the rest of the fleet: 2,749 more stale (>6h old) external containers across the other 10 hosts, from 0 on the just-cleared osac-9 up to 579 on osac-7. Added as a third step, age-gated the same way as the other two (reused `until`, so anything from a build still genuinely in progress is left alone), running before the image prune so containers freed up in this same pass make their images immediately eligible for removal in the same run rather than waiting a full cycle. Verified live end-to-end on osac-9: reaped 743/743 with 0 failures, `podman system df`'s reclaimable jumped from 92% to 100% (active image count 428 -> 3), and the previously-always-skipped 33 dangling images were removed cleanly on the very next prune (0 skipped, versus 33 skipped on every prior run). * NO-ISSUE: Fix reaped-count bug and node-exporter removal risk in cleanup Two issues found reviewing the stale-container reap steps, both confirmed live before fixing: - The E2E-container reap counted a container as reaped even when its podman rm -f failed, inflating the reported count. Now only counts on success, matching the pattern the external-container step already used. - --external is not an external-only view -- it's a union of genuine buildah artifacts with every regular container -a would already show, and on every host tested this included node-exporter ("Up 3 hours", meant to run for weeks). Age alone would eventually have marked it stale and force-removed the fleet's own monitoring container. Now filters on Command==buildah + Status==Storage as well as age, which correctly identifies genuine orphaned buildah containers across both podman 5.8.3 and 6.0.x hosts in this fleet (the full version spread currently deployed). * NO-ISSUE: Fix active-runs list silently capping at 20 queued/in-progress runs _fetch_active_runs hardcoded per_page=20 with no pagination, on the assumption ("active runs are few") that no repo would ever have more queued/in-progress runs than one page. During today's GitHub Actions incident, a single repo queued 67 runs -- everything past the first 20 was silently dropped from the active-runs list that /api/counts and every "Queued Now"/"In Progress Now" stat panel are built from, while the Prometheus queue-depth graph (built from GitHub's own total_count field, unaffected by this cap) correctly showed the real number. Fixed by paginating through every page instead of stopping after the first. Also stopped fetching per-run job details for queued runs specifically: a queued run has no runner assigned yet, so that call could only ever return a jobs list with no useful runner_name -- and repeating it for every item in a large queued backlog, every 90s, was needless API traffic in exactly the scenario (a real backlog) it's most likely to happen in. in_progress runs, where runner_name is actually meaningful, are unaffected. Verified live: dry-ran the fix inside the running container against the real GitHub API, confirmed it correctly returned all 69 active runs for the affected repo (up from 20); deployed and confirmed /api/counts now matches the Prometheus gauge (66/66) instead of being capped. * OSAC-3438: Add CaaS metering lifecycle E2E tests - Generalize MeteringCollector._validate_structure for both compute_instance and cluster_order resource types - Extract validate_vmaas_billing / validate_caas_billing helpers for resource-type-specific billing dimension checks - Add get_all_events() for N+1 record count assertions Tests: - test_cluster_metering_lifecycle: full lifecycle (create, started, heartbeat N+1 decomposition, delete) with CaaS billing validation - test_cluster_metering_event_structure: CloudEvent fields, resource type, cluster_template in billing dimensions CI: - Add echo-adapter route + METERING_ADAPTER_URL to CaaS workflow Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: restore billing validation, merge CaaS tests into existing lifecycle - Restore previous_state value validation in _validate_structure (suspended.v1 and resumed.v1 allowed values) - Dispatch billing dimension validation by resource type inside _validate_structure — fixes VMaaS billing regression where instance_type/image_ref/boot_disk_size_gib checks were dropped - Remove standalone CaaS metering test — add metering fixture and expectations to existing test_cluster_create instead (saves one full cluster provision cycle ~30min) Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: validate topLevelDims for CaaS created/deleted events created.v1 and deleted.v1 use topLevelDims (cluster_template + release_image, no per-component fields). Validate both fields instead of returning early after cluster_template. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: address review — N+1 count assertion, PR description accuracy - Assert heartbeat count matches 1 control_plane + N worker node sets (derived from cluster spec), not just component set membership - Use >= to allow multiple heartbeat ticks during the test window Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: add scaling test with updated.v1 assertion Add scale_cluster() CLI wrapper and scaling verification to test_cluster_create: scale a worker node set up by 1, verify osac.resource.updated.v1 carries correct node_set and node_count in billing_dimensions, then scale back before deletion. Covers OSAC-3438 AC: "Scale a worker node set, verify osac.resource.updated.v1 event is emitted." Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: use correct proto field name for node set size ClusterNodeSet proto field is 'size' not 'count'. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: verify deleted.v1 explicitly after deletion completes Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix: suppress spurious warning on fixture teardown verify() Only warn when verify() is called with no expectations AND nothing was previously verified. A second verify() after all expectations were already checked is normal (fixture teardown safety net). Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * OSAC-1684: Add pre-upload gitleaks scan to gather-artifacts Reduce secret exposure window by running gitleaks + redact.py on collected artifacts before upload. Post-upload scanner still runs unchanged as defense-in-depth. * OSAC-1684: Skip purging raw logs/artifacts for non-PR runs Periodic (non-PR) e2e runs now retain their original logs and artifacts after the audit scanner redacts them, so developers can still download the originals for debugging. PR-triggered runs continue to have tainted content deleted. PURGE_OK is set to false when purge is skipped and leaks were found, so tracking issues correctly report the exposure window as open rather than falsely showing a successful purge. * OSAC-1684: Fix empty else syntax error, purge merge_group runs Drop empty else body in gather-artifacts pre-scan that caused a bash parse error on every run. Also treat merge_group events like pull_request for purge purposes since they are pre-merge validation, not periodic debugging runs. * fix(audit): parallelize scan loop with matrix strategy The monorepo migration dramatically increased completed e2e run count (144+ runs/27h from osac-test-infra alone), causing the serial scan loop to time out at 2h31m. Split the single audit job into three phases: 1. discover: list runs, chunk into batches of 20 2. scan: matrix job (max-parallel 5) scans each chunk independently 3. report: aggregate counters/summaries, file issue, notify Slack Estimated wall time drops from 2.5h to ~20 min. Key design decisions: - Scan logic (process_one_run, validation) preserved verbatim - Per-chunk counters.json for authoritative aggregation (not JSONL line counting which misses failed jq appends) - Fail-closed: download failure + successful scan = incomplete audit - Clean pass headline only when SKIPPED_TOTAL == 0 - actions:write on scan (upload), actions:read on report (download) Assisted-by: Cursor <[email protected]> Signed-off-by: Min Zhang <[email protected]> * fix CI flakiness: SHA-pin component clones and retry downloads Two fixes for the top CI failure modes: 1. SHA-pin component clones in the build loop: the first clone of each repo resolves the branch ref to a commit SHA; all subsequent clones of the same repo reuse that SHA via git-fetch instead of cloning by branch name. This eliminates two race conditions: - merge-queue branches (gh-readonly-queue/main/pr-*) being deleted by GitHub between sequential clones (31% of all E2E failures) - main advancing between clones, producing divergent SHAs that fail the installer-pin check The AAP project SCM override now passes the resolved SHA instead of the branch name, so AAP's git-checkout survives ephemeral refs too. 2. Add --retry 5 --retry-delay 2 to all curl downloads in workflows and Containerfile to survive transient network errors (truncated downloads, CDN hiccups). Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix JWT token expiry in gRPC test clients GRPCClient now accepts an optional token_factory callable that refreshes the JWT on every request instead of using a static token acquired once at session start. The jwt_grpc_tenant1/2 fixtures use this to get fresh tokens from Keycloak, eliminating "token expired N seconds ago" flakes during long test sessions. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * fix review findings: TTL cache, pipefail, SHELL directive Address code review findings: - Add 60s TTL cache to GRPCClient token_factory to avoid hitting Keycloak on every gRPC call during polling loops - Add set -o pipefail to bmaas and e2e-vmaas.yml deploy steps so curl failures in pipes propagate instead of being silently swallowed - Add SHELL ["/bin/bash", "-o", "pipefail", "-c"] to Containerfile so curl | tar pipe failures are caught during image build Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * remove unused ref variable from install steps The AAP override now uses sha instead of ref, leaving the ref extraction as dead code that fails shellcheck SC2034. Assisted-by: Claude Code <[email protected]> Signed-off-by: omer-vishlitzky <[email protected]> * NO-ISSUE: Add osac-11 through osac-15 to the fleet inventory * NO-ISSUE: Track the merge queue and label gate in CI monitoring The merge queue (ALLGREEN batching, up to 4 PRs) and PR-label gate rolled out recently on osac/osac-test-infra, and time-to-merge has visibly shifted since -- but nothing distinguished queue/gate overhead from review latency, and merge-queue runs were invisible to per-PR tracking. workflow-exporter.py: - Link merge_group runs to their PR by parsing the PR number straight out of the gh-readonly-queue/<base>/pr-<N>-<sha> branch name (no extra API calls) -- unlocks per-PR job counts and retest tracking for merge-queue-triggered e2e runs, which previously vanished from those panels entirely. - Add queued_at/queue_wait_seconds/approval_to_queue_seconds/ via_merge_queue to pr_merges, computed from already-stored jobs (no new API calls), backfilled for existing rows. approval_to_merge_seconds is kept as-is for continuity, but now has a decomposition: how long a PR waited after approval before entering the queue (still a real review/process signal) vs. how long the queue itself added (a new, distinct cost from batching and ALLGREEN rechecks). - Add a merge_queue job_type bucket for the Grafana dropdown. Daily digest (post-ci-digest.py + ci-digest-report-template.html): - Time to Merge chart now shows 24h and 7d side by side (was 7d-only), each with median and average, so a rollout-day shift is visible instead of only showing up once it's absorbed into the 7-day window. - New Time in Merge Queue chart, same 24h/7d/median/avg treatment. - Both charts color by time window (24h vs 7d) rather than by repo -- with only two repos left (see below) a per-repo hue added a legend without adding information the row labels didn't already give. - Top 10 PRs table now links each PR number to its GitHub page. - HTML report now always opens in light mode -- previously followed the viewer's OS/browser dark-mode preference, which nobody had asked for and made screenshots/Slack previews inconsistent. The toggle still switches to dark on demand. Repo scope: osac-aap, osac-operator, fulfillment-service, osac-installer, and bare-metal-fulfillment-operator are archived and no longer produce CI activity worth tracking. Trimmed the exporter's REPOS list, the Grafana repo_filter/repo_regex dropdowns, and the digest's repo-color list down to osac/osac-test-infra. Existing history for the archived repos ages out on its own via the exporter's existing 60-day retention -- not purged. * Address CodeRabbit review findings on #366 Verified each finding against current code; fixed the ones that were still valid, skipped the rest with reasons below. Fixed: - MERGE_QUEUE_BRANCH_RE used [^/]+ for the base-branch segment, so a base branch containing its own "/" (e.g. a release branch) would shift the match short and miss the PR number entirely. Anchored on the pr-<N>-<sha> suffix instead. Neither repo queues against anything but "main" today, but the fix is free. - EVENT_TYPE_LABELS had no merge_group entry, so the avg-duration-by- type panel would show a raw "merge_group" row instead of "Merge Queue" like the other three job types. Also updated the job_type docstring line in get_jobs_json to mention merge_queue. - _seconds_between's docstring promises None for "falsy/unparseable" input, but a non-string truthy value would raise AttributeError uncaught. Every current caller only ever passes None/""/a real ISO string, so this was latent, not live -- still a one-line fix to match the docstring. - _backfill_queue_data_if_needed opened one SQLite write connection per row instead of one for the whole pass. Also fixed stale docstring wording ("startup") left over from before this got moved to run once from collect() instead. - post-ci-digest.py's queue-wait line indexed merge_time directly; since the digest and exporter are separate deploys, a digest run against a not-yet-upgraded exporter would KeyError the whole PNG build instead of degrading one line. Switched to .get() with the existing "no merge-queue PRs yet" fallback. - Extracted the merge-time and queue-wait charts' near-duplicate rendering code (same geometry, same two-bar loop, same tooltip wiring) into one shared renderWindowChart() function. Skipped: - "via_merge_queue could get a permanent wrong 0 if a PR's merge_group jobs aged out of the jobs table before the one-time backfill ran." Checked: pr_merges and jobs share the same JOBS_HISTORY_DAYS retention, pruned together every collect() cycle, and a merge_group job's created_at is always within minutes/hours of the PR's merged_at (not 60 days apart) -- so a pr_merges row can't outlive its own merge_group jobs in practice. The pr_display backfill for merge_group rows also has no PR-map staleness limit (unlike the pull_request case), so by the time this runs, every merge_group job still in the jobs table already has its PR resolved. - "GitHub org hardcoded in the PR link." True, but osac-project is already hardcoded elsewhere in this same template (the header's "osac-project org · generated ..." text) -- fixing just the new instance wouldn't fix the existing one, and threading pr_url through get_jobs_per_pr_json is a bigger change than this trivial/low-value finding calls for. Re-validated after every change: py_compile on both Python files, node --check on the extracted HTML report JS, a synthetic-data render covering every code path, and a fresh run of the real-data pull against osac/osac-test-infra confirming the branch-parsing fix and refactor didn't change any real output. * OSAC-2166: E2E tests for version-based cluster provisioning Assisted-by: Claude <[email protected]> * OSAC-2166: adapt version tests to OSAC-3675 ClusterVersionReference rename OSAC-3675 renamed ClusterSpec.version_name (string) to ClusterSpec.version (ClusterVersionReference message). Update gRPC request payloads, response assertions, and catalog-item field_definition paths to match the new schema. Assisted-by: Claude <[email protected]> * OSAC-2166: verify ClusterVersion deletion rejected while referenced Move version-to-image e2e propagation check (ClusterOrder releaseImage -> HostedCluster release image) into test_cluster_create which already waits for full provisioning. Lighten test_cluster_create_with_version by removing wait_for_cluster_ready and the HostedCluster check, cutting its runtime from ~25 min to ~5 min. Assisted-by: Claude <[email protected]> * Add BMaaS variables and update capabilities Add sushy-tools and BMaaS configuration variables to group_vars/all.yml to support bare metal host provisioning. Update capabilities file to advertise bmaas support alongside existing caas suite, enabling multi-suite test infrastructure. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Add deploy-lab role with sushy-tools on br-mgmt Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Add setup-bmaas role for virtual BMH provisioning on Netris Implements 7-phase BMaaS setup workflow: 1. Enable Ironic via Provisioning CR 2. Repurpose HGX VMs as virtual bare-metal hosts 3. Create BareMetalHost resources 4. Wait for BMH availability 5. Label BMHs with host type 6. Configure BMF operator secrets 7. Register host type, template, and catalog item in fulfillment service Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Add destroy-bmaas and gather-bmaas roles Implemented destroy-bmaas role: - Find BMaaS VMs matching patterns - Delete BareMetalHost CRs and BMC secrets - Delete BMF config secrets - Remove Provisioning CR - Shut down VMs and remove disks - Delete BMH namespace Implemented gather-bmaas role: - Gather BMH status and BareMetalInstance CRs - Collect Ironic and BMF operator logs - Gather sushy-tools logs - Collect fulfillment API BMI data Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Add deploy-lab, setup-bmaas, destroy-bmaas, gather-bmaas Makefile targets Added four new Makefile targets: - deploy-lab: Deploy sushy-tools BMC emulator - setup-bmaas: Register BMaaS resources with OSAC - destroy-bmaas: Clean up BMaaS VMs, CRs, and secrets - gather-bmaas: Collect BMaaS diagnostic artifacts Updated .PHONY declaration to include all new targets. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Pass osac_values_file to Ansible via ANSIBLE_EXTRA prep-osac copies license.zip and pull-secret to the values directory derived from osac_values_file. Without this, it always used the default from group_vars/all.yml (caas-ci), ignoring the Makefile's OSAC_VALUES_FILE override. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Move BMF secrets to prep-bmaas, convert k8s module to oc apply BMF operator mounts inventory/management config secrets as volumes. Without them, the pod stays in ContainerCreating and Helm --wait times out. Move secret creation to a new prep-bmaas role that runs before deploy-osac. Also convert all kubernetes.core.k8s calls to oc apply to avoid Python kubernetes library dependency issues on fresh servers. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Set Ironic externalIPs to br-mgmt for sushy-tools connectivity Ironic auto-detects the node's primary IP (Netris VNet) for virtual media URLs. sushy-tools on br-mgmt can't reach that IP. Discover the OCP node's br-mgmt IP and set it as externalIPs in the Provisioning CR so Ironic serves images from the management network. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Fix BMH loop and use disableVirtualMediaTLS Remove duplicate loop directive that produced list items instead of strings for BMH names. Use disableVirtualMediaTLS instead of virtualMediaViaExternalNetwork (which has no effect when Disabled). Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Use dedicated bmc-net libvirt network for Ironic/sushy-tools The host's br-mgmt has a VPN route conflict: OpenVPN pushes a route for 192.168.16.0/20 via tun0 that prevents direct L2 communication between the host and VMs on the bridge. Instead of fighting the VPN, create a dedicated libvirt network (bmc-net / br-bmc / 10.99.0.0/24) with DHCP. The OCP VM gets a static lease at 10.99.0.2, sushy-tools listens on 10.99.0.1. This gives Ironic and sushy-tools a clean L2 path with no VPN interference. Also enables virtstoraged.socket which sushy-tools needs for virtual media disk operations. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Attach bmc-net NIC to BMH VMs for IPA callback IPA ramdisk inside the BMH VMs needs to reach Ironic's callback URL on the bmc-net (10.99.0.2:6385). Without a NIC on bmc-net, the VMs can only reach br-mgmt addresses. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Use bmc-net NIC MAC as bootMACAddress for IPA connectivity IPA's BOOTIF kernel param must match the NIC on the network where rootfs is served. Previously bootMACAddress pointed at the br-mgmt NIC, so IPA only configured that interface and couldn't reach the rootfs URL at 10.99.0.2 on bmc-net. Now we use the bmc-net NIC MAC so IPA gets a DHCP lease on bmc-net and can download rootfs + callback on the same L2 segment — matching dev-scripts' single-network pattern. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Rename deploy-lab to setup-bmc and run it from deploy-infra The bmc-net + sushy-tools setup only needs libvirt VMs to exist, not OCP installed. Move it into deploy-infra (after verify) so it runs earlier in the pipeline. Rename deploy-lab → setup-bmc to better reflect what the role does. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Enable virtqemud.socket in setup-bmc for virsh connectivity virsh defaults to connecting via the modular virtqemud-sock, which isn't present when only the legacy libvirtd is running. Enable virtqemud.socket alongside virtstoraged and virtnetworkd so virsh commands work regardless of which libvirt mode the host uses. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Merge prep-bmaas into prep-osac, gate on bare_metal_services BMC network, sushy-tools, and BMF secrets are shared infrastructure for any bare metal service (BMaaS, CaaS). Gate them on a single bare_metal_services variable (default: true) instead of checking the values file path. Remove prep-bmaas role/playbook/target and the hardcoded bmf.enabled=false override. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Prefer non-secboot OVMF firmware for sushy-tools The netris-lab VMs use pc-i440fx machine type which doesn't support SMM required by OVMF_CODE.secboot.fd. When sushy-tools adds the UEFI loader for virtual media boot, the secboot firmware causes the VM to hang at UEFI initialization. Prefer OVMF_CODE.fd (non-secboot) which works with i440fx. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Clean stale UEFI loader from BMH VMs in setup-bmaas sushy-tools adds an OVMF loader to VMs when Ironic inserts virtual media. On re-runs, the stale loader (potentially secboot, which hangs on i440fx) must be removed so sushy-tools can re-add the correct non-secboot firmware. Clean up loader, nvram XML elements and the NVRAM vars file after shutting down VMs. Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Create osac CLI config directory before writing config Assisted-by: Claude Code <[email protected]> Signed-off-by: Dan Manor <[email protected]> * Fix BMI template host_type field to be a plain string The fulfillment API expects host_type as a string ID, not a nested obje…
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
2 participants
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
osac-test-infra:
github-config:
Dependencies
Depends on osac-test-infra PR adding label-gate, e2e gate jobs, and auto-queue workflows.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes