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

nginx: don't wait for deployment ready, it needs a minute#186

Closed
mangelajo wants to merge 1 commit into
mainfrom
mangelajo-patch-2
Closed

nginx: don't wait for deployment ready, it needs a minute#186
mangelajo wants to merge 1 commit into
mainfrom
mangelajo-patch-2

Conversation

@mangelajo

@mangelajo mangelajo commented Oct 31, 2025

Copy link
Copy Markdown
Member

It will happen in the background, and we avoid some failures that happen on the kubectl wait

Summary by CodeRabbit

  • Chores
    • Modified ingress controller initialization and readiness verification during system startup. Changes to the verification mechanism may affect overall startup duration and failure detection timing.

It will happen in the background, and we avoid some failures
that happen on the kubectl wait
@coderabbitai

coderabbitai Bot commented Oct 31, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The change removes an explicit 90-second kubectl wait timeout for the ingress-nginx controller pod readiness check in the install_nginx_ingress function. The verification now relies solely on a polling loop using kubectl get pods to check for pod availability.

Changes

Cohort / File(s) Summary
Ingress nginx controller readiness check refactoring
hack/utils
Removed explicit kubectl wait command with 90s timeout for ingress-nginx controller pod readiness. Readiness verification now depends exclusively on the polling loop using kubectl get pods against the ingress-nginx namespace controller selector.

Sequence Diagram

sequenceDiagram
    participant install as install_nginx_ingress
    participant kubectl as kubectl
    participant pods as ingress-nginx pods

    rect rgb(200, 220, 255)
    Note over install: Previous behavior
    install->>kubectl: kubectl wait --timeout=90s<br/>(explicit timeout wait)
    kubectl->>pods: Check readiness
    pods-->>kubectl: Ready or timeout
    kubectl-->>install: Success or timeout error
    end

    rect rgb(200, 255, 220)
    Note over install: New behavior
    install->>install: Polling loop
    loop Until pod found
        install->>kubectl: kubectl get pods (ingress-nginx)
        kubectl->>pods: Query pod status
        pods-->>kubectl: Pod info
        kubectl-->>install: Pod data or empty
    end
    install->>install: Continue when pod found
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the polling loop has adequate timeout and retry mechanisms to prevent indefinite hangs
  • Confirm the 90-second timeout was not a critical safety mechanism for detecting stuck deployments
  • Check if removal changes the expected startup duration or affects CI/CD reliability

Poem

🐰 No more waiting for that ninety-second chime,
We poll and peek instead, just taking our time!
The ingress controller hops into the fray,
No timeout to rush—we'll know when it's ready to play!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "nginx: don't wait for deployment ready, it needs a minute" directly and clearly relates to the main change in the changeset. The raw summary indicates the modification removes an explicit kubectl wait with a 90-second timeout for the ingress-nginx controller pod, replacing it with just a polling loop. The title captures this primary change by stating that the PR stops waiting for the deployment to be ready, which aligns with the PR's objective to avoid the explicit wait and allow deployment to happen in the background. The title is specific enough for a teammate scanning the repository history to understand the core modification without requiring knowledge of implementation details.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch mangelajo-patch-2

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9109552 and 4c0c2ee.

📒 Files selected for processing (1)
  • hack/utils (0 hunks)
💤 Files with no reviewable changes (1)
  • hack/utils

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.

@mangelajo mangelajo changed the title nginx: not wait for deployment ready, it needs a minute nginx: don't wait for deployment ready, it needs a minute Oct 31, 2025
@mangelajo

Copy link
Copy Markdown
Member Author

@bennyz and I agree that waiting is better, because then if nginx fails to deploy, we know it's nginx.

@mangelajo mangelajo closed this Nov 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant