Skip to content

test: wait for registration-service deployment to be created in manage-devsandbox-dashboard.sh#1246

Merged
rsoaresd merged 3 commits into
codeready-toolchain:masterfrom
rsoaresd:wait_for_deployment_rs_to_be_created
Jan 7, 2026
Merged

test: wait for registration-service deployment to be created in manage-devsandbox-dashboard.sh#1246
rsoaresd merged 3 commits into
codeready-toolchain:masterfrom
rsoaresd:wait_for_deployment_rs_to_be_created

Conversation

@rsoaresd
Copy link
Copy Markdown
Contributor

@rsoaresd rsoaresd commented Jan 6, 2026

Description

I thought oc wait --for=condition=Available deployment/registration-service -n toolchain-host-06215102 --timeout=5m was failing due to short timeout, but afterall it was failing because, by the time we check if deployment registration-service is available, the deployment was not created yet (Error from server (NotFound): deployments.apps "registration-service" not found), so we need to first wait for the deployment to be created.

Sorry!

job example: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/72470/rehearse-72470-pull-ci-codeready-toolchain-devsandbox-dashboard-master-e2e/2008644101292953600#1:build-log.txt%3A1270

+ HOST_NS=toolchain-host-06215102
+ oc wait --for=condition=Available deployment/registration-service -n toolchain-host-06215102 --timeout=10m
Error from server (NotFound): deployments.apps "registration-service" not found

More context: #1245

Summary by CodeRabbit

  • Chores
    • Improved dev sandbox dashboard CI reliability with a two-phase readiness check that distinguishes resource creation from full availability.
    • Faster detection of creation failures and clearer timeout messages.
    • Enhanced failure reporting with diagnostic listings to aid troubleshooting and reduce flaky initialization.

✏️ Tip: You can customize this high-level summary in your review settings.

@openshift-ci openshift-ci Bot requested review from fbm3307 and rajivnathan January 6, 2026 22:43
@openshift-ci openshift-ci Bot added the approved label Jan 6, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 6, 2026

Walkthrough

Replaces the single wait-for-availability call for registration-service with a two-phase approach: (1) poll for creation of the deployment/registration-service (5 min timeout, progress dots, list deployments on failure), (2) wait for the deployment to become Available (2 min timeout). Route wait unchanged.

Changes

Cohort / File(s) Summary
CI script: registration-service wait
scripts/ci/manage-devsandbox-dashboard.sh
Replaced single availability wait with two-phase flow: poll for creation of deployment/registration-service (5 min total via MAX_ITERATIONS with 2s intervals, dot progress, lists deployments and emits error on timeout), then wait up to 2 min for deployment condition Available. Adjusted NEXT_WAIT_TIME increment to pre-increment; route-wait logic and final timeouts retained.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CI as CI script
  participant K8s as Kubernetes API
  participant Deployment as registration-service (Deployment)
  participant Route as registration-service (Route)

  rect rgb(230,245,255)
  Note over CI,K8s: Phase 1 — poll for Deployment resource creation (<= 5m)
  CI->>K8s: GET Deployment `registration-service` in host namespace
  alt not present
    K8s-->>CI: 404 / not found
    CI->>CI: print dots, ++counter, sleep 2s
    loop until timeout
    end
    rect rgb(255,230,230)
    Note right of CI: on timeout -> list deployments, print error, exit
    end
  else present
    K8s-->>CI: 200 / deployment exists
  end
  end

  rect rgb(230,255,230)
  Note over CI,Deployment: Phase 2 — wait for condition Available (<= 2m)
  CI->>Deployment: wait-for condition=Available
  alt becomes Available
    Deployment-->>CI: status=Available
  else timeout
    Deployment-->>CI: status!=Available (timeout)
  end
  end

  rect rgb(245,245,220)
  Note over CI,Route: Route sync (unchanged)
  CI->>K8s: existing route wait/poll logic
  K8s-->>CI: route status / timeout as before
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

lgtm

Suggested reviewers

  • xcoulon
  • alexeykazakov
  • mfrancisc
  • MatousJobanek

Poem

🐇 I hopped through CI with careful cheer,

I counted dots and watched each tier,
First I looked for a newborn deploy,
Then I waited for it to stand with joy,
A gentle hop—now the route is clear.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding a wait mechanism for registration-service deployment creation in the script.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c565dc and bcd0729.

📒 Files selected for processing (1)
  • scripts/ci/manage-devsandbox-dashboard.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: GolangCI Lint
  • GitHub Check: Unit Tests
  • GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🔇 Additional comments (3)
scripts/ci/manage-devsandbox-dashboard.sh (3)

179-193: Excellent fix: two-phase approach solves the original timing issue.

The creation-wait loop correctly addresses the root cause—attempting to wait for availability before the deployment resource exists. The logic is sound:

  • 5-minute timeout (150 × 2s) is reasonable for deployment creation in CI
  • Pre-increment ((++COUNTER)) safely avoids the set -e exit trap
  • Error output lists available deployments for effective debugging

194-196: LGTM: Clean availability check after creation.

The oc wait command is the appropriate tool for this phase, and the 2-minute timeout is reasonable for a deployment to reach Available status.


208-208: Good consistency: pre-increment aligns with the deployment creation loop.

Switching to ((++NEXT_WAIT_TIME)) maintains the same set -e-safe pattern established in the deployment creation loop.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI Agents
In @scripts/ci/manage-devsandbox-dashboard.sh:
- Around line 179-192: The polling loop uses arithmetic post-increment
((COUNTER++)) which returns exit status 1 when COUNTER is 0 and causes the
script (with set -e enabled) to exit immediately; replace the increment with a
safe form that always succeeds (e.g., COUNTER=$((COUNTER+1)) or
COUNTER=$((COUNTER + 1)) or ((COUNTER+=1))) and keep the rest of the loop logic
around the deployment check for registration-service and TIMEOUT/COUTNER
variables; additionally, after the while loop completes successfully, add an
echo or newline to move output to the next line so the final messages aren’t
printed on the same line as the progress dots.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6612961 and b6e67d5.

📒 Files selected for processing (1)
  • scripts/ci/manage-devsandbox-dashboard.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: GolangCI Lint
  • GitHub Check: Unit Tests
  • GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🔇 Additional comments (1)
scripts/ci/manage-devsandbox-dashboard.sh (1)

193-194: Approve the two-phase wait approach.

The solution correctly addresses the root cause described in the PR objectives by ensuring the deployment resource exists before waiting for the Available condition. The 2-minute timeout for the availability check is reasonable.

Once the critical ((COUNTER++)) bug is fixed, this implementation will properly solve the "deployments.apps 'registration-service' not found" error.

Comment thread scripts/ci/manage-devsandbox-dashboard.sh
Comment thread scripts/ci/manage-devsandbox-dashboard.sh Outdated
@rsoaresd rsoaresd requested a review from rajivnathan January 6, 2026 23:18
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jan 6, 2026

Copy link
Copy Markdown
Contributor

@xcoulon xcoulon left a comment

Choose a reason for hiding this comment

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

yeah, it's too bad that the oc wait command fails when the resource to check doesn't exist yet :/

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jan 7, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, fbm3307, rsoaresd, xcoulon

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 [alexeykazakov,fbm3307,rsoaresd,xcoulon]

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

@rsoaresd rsoaresd merged commit 6e1f4ec into codeready-toolchain:master Jan 7, 2026
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants