Update/agent updater #3102

Merged

mfreeman451 merged 3 commits from refs/pull/3102/head into staging

2026-03-30 19:41:07 +00:00

mfreeman451 commented

2026-03-30 19:40:44 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #3104
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/3104
Original created: 2026-03-30T19:40:44Z
Original updated: 2026-03-30T19:46:37Z
Original head: carverauto/serviceradar:update/agent-updater
Original base: staging
Original merged: 2026-03-30T19:41:07Z by @mfreeman451

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

Imported from GitHub pull request. Original GitHub pull request: #3104 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/3104 Original created: 2026-03-30T19:40:44Z Original updated: 2026-03-30T19:46:37Z Original head: carverauto/serviceradar:update/agent-updater Original base: staging Original merged: 2026-03-30T19:41:07Z by @mfreeman451 --- ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test?

Copilot commented

2026-03-30 19:46:36 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852407
Original created: 2026-03-30T19:46:36Z
Original path: helm/serviceradar/templates/web.yaml
Original line: 250

DATASVC_ENABLED is now hard-coded to "true" for the web-ng deployment. This is a behavioral change from prior chart versions and forces datasvc client startup even when a datasvc endpoint isn't deployed/configured, which can lead to noisy reconnect loops and unnecessary resource usage. Consider making this value configurable via values.yaml (e.g., webNg.datasvcEnabled) and defaulting to the previous behavior, or gating it on datasvc connectivity being configured.

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852407 Original created: 2026-03-30T19:46:36Z Original path: helm/serviceradar/templates/web.yaml Original line: 250 --- `DATASVC_ENABLED` is now hard-coded to "true" for the web-ng deployment. This is a behavioral change from prior chart versions and forces datasvc client startup even when a datasvc endpoint isn't deployed/configured, which can lead to noisy reconnect loops and unnecessary resource usage. Consider making this value configurable via `values.yaml` (e.g., `webNg.datasvcEnabled`) and defaulting to the previous behavior, or gating it on datasvc connectivity being configured.

Copilot commented

2026-03-30 19:46:36 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852438
Original created: 2026-03-30T19:46:36Z
Original path: elixir/serviceradar_core/lib/serviceradar/data_service/client.ex
Original line: 600

disconnect_direct_channel/1 attempts to close the direct gRPC channel via adapter.disconnect/1, but the rest of the codebase consistently uses GRPC.Stub.disconnect/1 (e.g., elixir/serviceradar_core/lib/serviceradar/spiffe/workload_api.ex:175-181). If the adapter doesn't implement disconnect/1, the fallback path can leak connections. Prefer calling GRPC.Stub.disconnect(channel) (wrapped in rescue/catch like the existing safe_disconnect/1 helper) to reliably close channels.

  defp disconnect_direct_channel(%GRPC.Channel{} = channel) do
    try do
      GRPC.Stub.disconnect(channel)
    rescue
      _ -> :ok
    catch
      _, _ -> :ok

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852438 Original created: 2026-03-30T19:46:36Z Original path: elixir/serviceradar_core/lib/serviceradar/data_service/client.ex Original line: 600 --- `disconnect_direct_channel/1` attempts to close the direct gRPC channel via `adapter.disconnect/1`, but the rest of the codebase consistently uses `GRPC.Stub.disconnect/1` (e.g., `elixir/serviceradar_core/lib/serviceradar/spiffe/workload_api.ex:175-181`). If the adapter doesn't implement `disconnect/1`, the fallback path can leak connections. Prefer calling `GRPC.Stub.disconnect(channel)` (wrapped in rescue/catch like the existing `safe_disconnect/1` helper) to reliably close channels. ```suggestion defp disconnect_direct_channel(%GRPC.Channel{} = channel) do try do GRPC.Stub.disconnect(channel) rescue _ -> :ok catch _, _ -> :ok ```

Copilot commented

2026-03-30 19:46:37 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852468
Original created: 2026-03-30T19:46:37Z
Original path: elixir/serviceradar_core/test/serviceradar/data_service/client_test.exs
Original line: 10

This test assumes ServiceRadar.DataService.Client is not running (refute Process.whereis(Client)), but test/test_helper.exs starts the full core app whenever a test DB env var is set; in that mode the client is typically started (see ServiceRadar.Application.datasvc_client_child/0). This makes the test brittle for integration runs. Consider making the test deterministic by explicitly stopping/ensuring the client isn’t started in setup, or by isolating it under the same conditions as the "core not started" unit-test mode (e.g., tagging/excluding it when the full app is started).

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852468 Original created: 2026-03-30T19:46:37Z Original path: elixir/serviceradar_core/test/serviceradar/data_service/client_test.exs Original line: 10 --- This test assumes `ServiceRadar.DataService.Client` is not running (`refute Process.whereis(Client)`), but `test/test_helper.exs` starts the full core app whenever a test DB env var is set; in that mode the client is typically started (see `ServiceRadar.Application.datasvc_client_child/0`). This makes the test brittle for integration runs. Consider making the test deterministic by explicitly stopping/ensuring the client isn’t started in setup, or by isolating it under the same conditions as the "core not started" unit-test mode (e.g., tagging/excluding it when the full app is started).

Copilot commented

2026-03-30 19:46:37 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852496
Original created: 2026-03-30T19:46:37Z
Original path: CHANGELOG
Original line: 10

The changelog claims a new "manual GitHub Actions workflow" was added for deriving SERVICERADAR_AGENT_RELEASE_PUBLIC_KEY, but this PR doesn’t include any new/updated workflow under .github/workflows/ (and there are no workflow references to that env var). Please either include the workflow changes in this PR or remove this bullet so the release notes match what actually shipped.

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3104#discussion_r3011852496 Original created: 2026-03-30T19:46:37Z Original path: CHANGELOG Original line: 10 --- The changelog claims a new "manual GitHub Actions workflow" was added for deriving `SERVICERADAR_AGENT_RELEASE_PUBLIC_KEY`, but this PR doesn’t include any new/updated workflow under `.github/workflows/` (and there are no workflow references to that env var). Please either include the workflow changes in this PR or remove this bullet so the release notes match what actually shipped. ```suggestion ```