Repository navigation
fix(dashboard): show LoadBalancer external IP on the app Services tab - #3172
Conversation
The per-app Services tab queried Services with a spec-only type, so it never read status.loadBalancer.ingress and showed only the cluster-internal IP for external: true (type: LoadBalancer) services. This regressed the pre-1.4.0 behaviour (#2789). Add a ServiceStatus type to the useK8sList query and surface an "External IP" column (ip / hostname / Pending for LoadBalancer, — otherwise), mirroring what ExternalIpsPage already does. Fixes #3111 Assisted-By: Claude <[email protected]> Signed-off-by: Алексей Артамонов <[email protected]>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a regression in the application dashboard where the external IP address for LoadBalancer services was no longer visible in the Services tab. By updating the data fetching logic to include service status information and adding a dedicated column to the UI, users can now easily identify the connection address for their exposed applications. The changes are accompanied by comprehensive unit tests to ensure future stability. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughAdds a ChangesServicesTab External IP Display
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ServicesTab
participant K8sClient
participant ServiceRow
ServicesTab->>K8sClient: useK8sList query ServiceSpec + ServiceStatus
K8sClient-->>ServicesTab: services list with status.loadBalancer.ingress
ServicesTab->>ServiceRow: render row per service
ServiceRow->>ServiceRow: check spec.type === LoadBalancer
ServiceRow->>ServiceRow: derive external = ingress.ip or hostname or Pending
ServiceRow-->>ServicesTab: render External IP cell or em dash
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request adds support for displaying the External IP of LoadBalancer services in the dashboard's ServicesTab component. It introduces an "External IP" column to the services table, resolving the IP or hostname from the service status (or displaying "Pending" if not yet assigned), and falls back to an em dash for non-LoadBalancer services. Additionally, a comprehensive test suite has been added to verify this behavior under various scenarios. I have no feedback to provide as the implementation is clean and well-tested.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the PR template requires a screenshot for UI changes; this PR ships without one.
Business context: Cozystack 1.4.0 dropped the LoadBalancer external IP from the per-app Services tab (it was visible pre-1.4.0), and this PR restores it by threading status.loadBalancer.ingress[] through the existing query, mirroring the logic already used on the Administration → External IPs page.
Blockers
B1: No screenshot for a UI change
File: PR description, "Screenshots" section
Issue: The PR adds a new "External IP" column to the Services tab UI, but the Screenshots section still contains the unfilled placeholder <!-- TODO: attach a screenshot of the Services tab showing the External IP column for a LoadBalancer service. -->. No image is attached anywhere in the PR body or comments.
Evidence: .github/PULL_REQUEST_TEMPLATE.md states: "REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged." This PR modifies ServicesTab.tsx's rendered table — a UI change squarely within the template's stated scope.
Impact: Reviewers can't visually verify the rendered column (alignment, font-mono styling, hostname truncation, Pending/— states) without checking out the branch. Merging as-is bypasses the project's own completeness gate for UI PRs.
Fix: Attach a screenshot of the Services tab showing the new External IP column for a LoadBalancer service (ideally also the Pending/— states) and remove the TODO placeholder.
Non-blocking follow-ups
ServiceSpec/ServiceStatusand theip ?? hostname ?? "Pending"fallback logic are now duplicated verbatim betweenServicesTab.tsxandExternalIpsPage.tsx. Two occurrences fits this project's "three similar widgets over a premature factory" convention — worth extracting only if a third call site shows up.- The failing "E2E Tests" check on this PR is a
cozy-vertical-pod-autoscalerHelm upgrade/liveness-probe failure during cluster install — unrelated to this frontend-only change.
IvanHunters
left a comment
There was a problem hiding this comment.
Review: LGTM with one non-blocking note
Correct minimal frontend fix for a real regression.
Non-blocking
ServicesTab.test.tsxClusterIP test:expect(screen.getAllByText("—").length).toBeGreaterThan(0)does not actually pin the External IP behavior. All three fixtures omitcreationTimestamp, andformatAge(undefined)returns"—"(lib/status.ts:11), so the Age column already renders an em dash on its own. The assertion passes on the Age cell regardless of the External IP column, and would still pass if the External IP rendering were broken. Suggest scoping the query to the External IP cell, or giving the fixture acreationTimestampso the em-dash count is deterministic. The other two tests use specific string matches and are robust.
Verified
- Reading
status.loadBalancerneeds no new RBAC: thestatusfield of a core Service is returned byget/list/watch services(not a separate subresource), and the dashboard already list/watches services (ExternalIpsPage does the identical read). - The
lb?.ip ?? lb?.hostname ?? "Pending"fallback matches ExternalIpsPage; ServicesTab correctly gates ontype === "LoadBalancer"and renders—otherwise.
Upgrade/fresh-install impact: none beyond a dashboard image rebuild. Purely additive UI column.
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — correct, minimal, additive fix for a real 1.4.0 regression, and the screenshot is now attached, which resolves my earlier request for changes.
The change threads status.loadBalancer.ingress[] through the existing useK8sList query and renders ip ?? hostname ?? "Pending" for LoadBalancer services and — otherwise, mirroring ExternalIpsPage.tsx. No new RBAC: status ships with a core Service list/watch, and the dashboard already reads it on the External IPs page. Existing columns are unchanged; no install/upgrade impact beyond a dashboard image rebuild.
Non-blocking (already raised in review, adding evidence): the ClusterIP test asserts getAllByText("—").length > 0, but all fixtures omit creationTimestamp and formatAge(undefined) returns "—" (lib/status.ts:11), so the Age cell alone satisfies the assertion — it would still pass if the External IP cell were broken. Scope the query to the External IP cell, or give the fixture a creationTimestamp, so the test pins the intended behavior. The IP and hostname→Pending tests use exact string matches and are solid.
What this PR does
For an application exposed with
external: true(which creates atype: LoadBalancerService), the app detail → Services tab showed the service Type and Cluster IP but not the external IP assigned by MetalLB — so there was no way to learn the connection address from the per-app view. The pre-1.4.0 dashboard showed this, so it read as a regression (reported in #2789).Root cause:
ServicesTab.tsxqueried Services with a spec-only local type ({ type, clusterIP, ports }) and never readstatus.loadBalancer.ingress[]. The data was already reachable —ExternalIpsPage.tsxrenders it — so this was a wiring gap in the per-app tab, not missing data.Changes:
ServiceStatustype and extend theuseK8sListquery toK8sResource<ServiceSpec, ServiceStatus>.ip ?? hostname ?? "Pending"forLoadBalancerservices,—otherwise — mirroringExternalIpsPage.ServicesTab.test.tsx) covering the IP, hostname→Pending fallback, and the ClusterIP no-external case.Verified:
pnpm typecheckclean,pnpm testgreen (320 tests, incl. 3 new),eslintclean on the changed files.Fixes #3111
Screenshots
Release note
Summary by CodeRabbit
New Features
Bug Fixes