Skip to content

Discover the agent address from BLACKSMITH_AGENT_ADDR - #38

Merged
piob-io merged 1 commit into
mainfrom
agent-addr-discovery
Jul 30, 2026
Merged

Discover the agent address from BLACKSMITH_AGENT_ADDR#38
piob-io merged 1 commit into
mainfrom
agent-addr-discovery

Conversation

@piob-io

@piob-io piob-io commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Discovers the Blacksmith agent (git-mirror gRPC and internal metrics endpoints) through the BLACKSMITH_AGENT_ADDR env var the agent advertises on every platform (FastActions/fa#4816); the action contains no hardcoded agent IPs, and agent problems never fail the job:

  • shouldUseBlacksmithCache() requires BLACKSMITH_AGENT_ADDR in addition to BLACKSMITH_VM_ID, so runners without agent services use upstream actions/checkout behavior with a single info line and zero agent dials.
  • setupCache() RPCs (up, getStickyDisk) are bounded by a 45s AbortController.
  • Any agent error (Unimplemented on platforms without the service, refused or timed-out dials, anything else) warns and falls back to a standard checkout.
if (!getAgentAddr()) {
  core.info('[blacksmith] BLACKSMITH_AGENT_ADDR is not set; ... falling back to actions/checkout behavior')
  return false
}

Requires the agent change (FastActions/fa#4816) to be rolled out fleet-wide before this is released.


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled. (Staging)


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@piob-io
piob-io force-pushed the agent-addr-discovery branch 3 times, most recently from c41a53b to 9e66025 Compare July 29, 2026 19:00
@piob-io
piob-io marked this pull request as ready for review July 29, 2026 20:36
@piob-io
piob-io requested a review from taha-au July 29, 2026 21:40
@piob-io
piob-io force-pushed the agent-addr-discovery branch 2 times, most recently from 65bb662 to 0810c59 Compare July 30, 2026 11:51

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0810c59. Configure here.

Comment thread __test__/blacksmith-cache.test.ts
Comment thread src/blacksmith-cache.ts
const GRPC_PORT = process.env.BLACKSMITH_STICKY_DISK_GRPC_PORT || '5557'
// Without a deadline, a black-holed dial stalls the checkout until the OS
// gives up on the TCP handshake.
const AGENT_RPC_TIMEOUT_MS = 45000

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this feels excessively large, but useblacksmith/checkout is the legacy approach so not strongly opinionated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We discussed this and decided to keep 45s. Context: this deadline covers more than the dial; the same AbortController bounds up() plus the getStickyDisk RPC, and agent-side that RPC synchronously does the Ceph expose (bounded at 60s in the agent) and the rbd-nbd map (another 60s), so a legitimately slow mount can run tens of seconds under Ceph load. 45s also matches the long-standing stickyDiskTimeoutMs=45000 in the stickydisk, setup-docker-builder, and setup-bazel actions, so all actions hitting this RPC share one number. Before this PR checkout had no deadline at all here (a wedged agent stalled checkout until the step timeout), so this is a strict improvement either way. On timeout the action falls back to a plain clone, so the worst case is one cache-less checkout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This actually matches the timeout for other sticky disk actions - the 45s includes exposing and mounting the sticky disk, not just reachability. I considered splitting this into two deadlines, but I don't expect dialing to be slow.

… to standard checkout when unavailable

Co-authored-by: Codesmith Staging <codesmith-bot@users.noreply.github.com>
@piob-io
piob-io force-pushed the agent-addr-discovery branch 3 times, most recently from ab56bde to cd21553 Compare July 30, 2026 14:48
@piob-io
piob-io merged commit 846eab2 into main Jul 30, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants