Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

NO-ISSUE: Fix CaaS boot failures caused by dirty database migrations #260

Closed
omer-vishlitzky wants to merge 1 commit into from
Closed

NO-ISSUE: Fix CaaS boot failures caused by dirty database migrations #260

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions scripts/refresh-after-snapshot.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ failed=0
wait ${pid_creds} || failed=1
if (( failed )); then echo "ERROR: Failed to create fulfillment credentials"; exit 1; fi

oc scale deploy/fulfillment-controller -n "${INSTALLER_NAMESPACE}" --replicas=0 2>/dev/null || true
echo "[3/9] Applying kustomize overlay..."
oc delete job -n "${INSTALLER_NAMESPACE}" --all --ignore-not-found
# Exclude only the bootstrap job — it's redundant on snapshot boot and races
Expand All @@ -173,8 +172,14 @@ oc delete job -n "${INSTALLER_NAMESPACE}" --all --ignore-not-found
# operator triggers into one reconciliation instead of a separate oc patch.
sed '/job\.yaml/d' base/osac-aap/config/base/kustomization.yaml > base/osac-aap/config/base/kustomization.yaml.tmp \
&& mv base/osac-aap/config/base/kustomization.yaml.tmp base/osac-aap/config/base/kustomization.yaml
# Deploy with fulfillment pods scaled to zero. The apply changes the database
# StatefulSet image ref (tag → digest), triggering a pod recreation. If the
# grpc-server were running, it could be mid-migration when the database is
# killed, leaving golang-migrate's schema dirty. Deploying at zero replicas
# eliminates the race entirely — pods are brought back at step [5/9] after
# the database rollout completes.
( cd "overlays/${INSTALLER_KUSTOMIZE_OVERLAY}" && kustomize edit set replicas fulfillment-controller=0 fulfillment-grpc-server=0 )
Comment thread
coderabbitai[bot] marked this conversation as resolved.
oc apply -k "overlays/${INSTALLER_KUSTOMIZE_OVERLAY}"
Comment on lines +175 to 182

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Major risk: kustomize edit leaves overlay with replicas: 0 permanently.

The kustomize edit set replicas command on line 181 modifies overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/kustomization.yaml on disk. While lines 322-323 restore replicas via oc scale in the cluster, the kustomization.yaml file retains replicas: 0.

Impact (Severity: Major):

  • Any subsequent oc apply -k overlays/${OVERLAY} (manual or automated) will scale fulfillment-controller and fulfillment-grpc-server to 0, causing immediate service outage
  • On shared clusters (development, hypershift2 per AGENTS.md), this creates a latent disruption risk for other developers
  • Leaves git working tree dirty, potentially masking other overlay changes

Recommended fix: Restore the replicas configuration at the end of the script or after the deployments are successfully rolled out.

🛡️ Proposed fix: Restore replicas in kustomization.yaml

Add this near the end of the script (e.g., after line 418's rollout verification):

 if (( failed )); then echo "ERROR: Fulfillment rollout failed after restart"; exit 1; fi
+# Restore overlay replicas so future applies don't scale to zero
+( cd "overlays/${INSTALLER_KUSTOMIZE_OVERLAY}" && kustomize edit set replicas fulfillment-controller=1 fulfillment-grpc-server=1 )
 ./scripts/prepare-tenant.sh

Alternatively, consider using oc scale --replicas=0 before the apply instead of modifying the overlay file, though this reintroduces the race window you're trying to avoid.

🤖 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/refresh-after-snapshot.sh` around lines 175 - 182, The script
currently mutates overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/kustomization.yaml
with "kustomize edit set replicas fulfillment-controller=0
fulfillment-grpc-server=0" and never restores it; change the script to read and
save the current replica values from the overlay before the edit, perform the
zero-replica kustomize edit and oc apply -k as now, then after rollout
verification restore the original replica values back into the same overlay (use
the saved values with kustomize edit set replicas fulfillment-controller=<saved>
fulfillment-grpc-server=<saved>), placing the restore step after the
deployment/rollout checks near the end of the script so the working tree is not
left modified (alternatively replace the edit-within-file approach by using oc
scale --replicas=0 before apply if you prefer to avoid mutating
kustomization.yaml).

oc scale deploy/fulfillment-controller -n "${INSTALLER_NAMESPACE}" --replicas=0 2>/dev/null || true

REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd -P)"
PULL_SECRET="${REPO_ROOT}/overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/quay-pull-secret.json"
Expand Down Expand Up @@ -306,22 +311,29 @@ for cert in "${fs_certs[@]}"; do
-n "${INSTALLER_NAMESPACE}" --timeout=300s &
pids+=($!)
done
failed=0
for pid in "${pids[@]}"; do wait "${pid}" || failed=1; done
if (( failed )); then echo "ERROR: TLS certificates not ready"; exit 1; fi
# Wait for the database StatefulSet rollout before scaling up pods that run
# migrations. The kustomize apply changed the image ref, triggering a
# recreation — the database must be accepting connections before grpc-server
# starts.
oc rollout status statefulset/fulfillment-database -n "${INSTALLER_NAMESPACE}" --timeout=300s
oc scale deploy/fulfillment-controller -n "${INSTALLER_NAMESPACE}" --replicas=1
oc scale deploy/fulfillment-grpc-server -n "${INSTALLER_NAMESPACE}" --replicas=1
# Envoy never re-reads its config after startup. Restart ingress-proxy before
# the rollout wait so that pods depending on new routes (e.g. JWKS) can start.
oc rollout restart deploy/fulfillment-ingress-proxy -n "${INSTALLER_NAMESPACE}"
# Kustomize apply may have changed deployment images, triggering new rollouts
# that run DB migrations. Wait for those to finish before restarting pods —
# otherwise the restart kills pods mid-migration and leaves the DB dirty.
pids=()
for deploy in "${FULFILLMENT_DEPLOYS[@]}"; do
oc rollout status "deploy/${deploy}" -n "${INSTALLER_NAMESPACE}" --timeout=300s &
pids+=($!)
done
failed=0
for pid in "${pids[@]}"; do wait "${pid}" || failed=1; done
wait ${pid_cdi} || failed=1
if (( failed )); then echo "ERROR: TLS certificates or fulfillment rollouts not ready"; exit 1; fi
if (( failed )); then echo "ERROR: Fulfillment rollouts not ready"; exit 1; fi
echo "[5/9] TLS certificates ready, restarting pods..."
oc scale deploy/fulfillment-controller -n "${INSTALLER_NAMESPACE}" --replicas=1
for deploy in "${FULFILLMENT_DEPLOYS[@]}"; do
oc rollout restart "deploy/${deploy}" -n "${INSTALLER_NAMESPACE}"
done
Expand Down
Loading