fix refresh race: wait for kustomize rollouts before restarting pods

[omer-vishlitzky]

Summary

Fixes a race condition in refresh-after-snapshot.sh where oc rollout restart in step [4/8] kills pods that were started by the kustomize apply in step [3/8], interrupting in-flight
DB migrations and leaving the database in a dirty state.

What happens:

1. Step [3/8] oc apply -k changes the fulfillment-service image → Kubernetes creates new pods that start running DB migration N
2. Step [4/8] waits for TLS certs, then does oc rollout restart → kills those pods mid-migration
3. DB is now version: N, dirty: true → every subsequent pod crashes with "Dirty database version N"
4. CrashLoopBackOff → rollout timeout → job fails

Evidence from this CI
run:

• Old pod (from snapshot): "version":43,"dirty":false — clean
• First new pod (from kustomize apply): started migration 44, killed after ~1 second by rollout restart
• Second new pod (from restart): "version":44,"dirty":true → Dirty database version 44. Fix and force version. → CrashLoopBackOff

Fix: Wait for kustomize-triggered rollouts in parallel with the TLS cert waits, so migrations complete before the restart. Zero duration impact since both wait groups overlap.

Test plan

• Retrigger the failing CI job on osac-test-infra PR Add secrets RBAC to hub-access Role #45 with this installer change
• Verify fulfillment-grpc-server completes migration before restart
• Verify no dirty DB state after refresh

Summary by CodeRabbit

• Chores
  • Post-snapshot refresh now waits for fulfillment service rollouts in parallel with TLS certificate readiness, reducing deployment flakiness and ensuring services become available before continuing.
  • Improved restart flow for the AAP controller: the controller is restarted and awaited to be ready before re-checking the gateway, with clearer timeout/error messaging for post-restart gateway checks.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: omer-vishlitzky

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [omer-vishlitzky]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Caution

Review failed

Failed to post review comments

Walkthrough

The script now runs background cert-manager certificate waits and parallel oc rollout status checks for four fulfillment deployments, waits for all spawned processes and fails with a combined "TLS certificates or fulfillment rollouts not ready" message; it also deletes and waits for the osac-aap-controller-task pod before rechecking the AAP gateway. Risk: Moderate — availability may be blocked if readiness or rollouts time out.

Changes

Deployment Readiness & AAP Restart

Layer / File(s)	Summary
Fulfillment rollout and cert-manager waits scripts/refresh-after-snapshot.sh	Starts background oc wait for cert-manager certificates and parallel oc rollout status checks for fulfillment-controller, fulfillment-grpc-server, fulfillment-rest-gateway, and fulfillment-ingress-proxy; waits for all PIDs and updates the timeout failure message to cover TLS certificates or fulfillment rollouts not ready.
AAP controller-task recycle and gateway recheck scripts/refresh-after-snapshot.sh	Deletes pods matching osac-aap-controller-task, waits for the controller-task pod to be Ready, then retries the AAP gateway HTTP readiness probe with a distinct post-restart timeout error message.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• osac-project/osac-installer#164: Modifies scripts/refresh-after-snapshot.sh with overlapping readiness and rollout wait changes.
• osac-project/osac-installer#95: Earlier updates to the script's post-snapshot wait/restart sequencing for fulfillment/AAP components.
• osac-project/osac-installer#122: Changes parallel stabilization/wait logic around cert-manager and rollout waits similar to these edits.

Suggested labels

lgtm

Suggested reviewers

• trewest
• akshaynadkarni
• adriengentil

Poem

✨ Pods and certs in quiet line,
The script counts each ready sign.
Rollouts hum, the controller sleeps—
A gateway waits while readiness keeps.
🎯 Moderate risk: timeouts may stall deploys.

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error, 2 warnings)

Check name	Status	Explanation	Resolution
No-Hardcoded-Secrets	❌ Error	Script contains hardcoded plaintext credentials: username=admin and password=admin in line 86 for Keycloak authentication via curl command.	Extract hardcoded credentials (username, password, client_id) from line 86 and pass them as environment variables, function parameters, or read from secure config files instead.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Ai-Attribution	⚠️ Warning	Commit uses Co-Authored-By trailer for Claude (Anthropic AI tool); Red Hat policy requires Assisted-by or Generated-by trailers for AI contributions.	Replace Co-Authored-By: Claude trailer with Assisted-by: Claude noreply@anthropic.com (or Generated-by if fully auto-generated).

✅ Passed checks (8 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and specifically describes the main change: waiting for kustomize rollouts before restarting pods to fix a race condition.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto	✅ Passed	No weak crypto found. MD5 usage is non-cryptographic (config file change detection, acceptable). No SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or insecure secret comparisons.
No-Injection-Vectors	✅ Passed	PR modifies only Bash script (refresh-after-snapshot.sh) with 19 new lines using hardcoded selectors and safe quoted variables—introduces none of the flagged injection vectors.
Container-Privileges	✅ Passed	PR modifies only bash orchestration script; no privileged containers, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation are introduced or modified.
No-Sensitive-Data-In-Logs	✅ Passed	All new logging statements in the PR (lines 206-217, 259-265) are safe and do not expose passwords, tokens, API keys, PII, session IDs, internal hostnames, or customer data.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[omer-vishlitzky]

/retest

[omer-vishlitzky]

/retest

[ori-amizur]

/lgtm