Update/agent updater #3101

Merged

mfreeman451 merged 2 commits from refs/pull/3101/head into staging

2026-03-30 18:24:12 +00:00

mfreeman451 commented

2026-03-30 18:24:02 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #3103
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/3103
Original created: 2026-03-30T18:24:02Z
Original updated: 2026-03-30T18:29:47Z
Original head: carverauto/serviceradar:update/agent-updater
Original base: staging
Original merged: 2026-03-30T18:24:12Z 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: #3103 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/3103 Original created: 2026-03-30T18:24:02Z Original updated: 2026-03-30T18:29:47Z Original head: carverauto/serviceradar:update/agent-updater Original base: staging Original merged: 2026-03-30T18:24:12Z 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 18:29:46 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499424
Original created: 2026-03-30T18:29:46Z
Original path: helm/serviceradar/templates/web.yaml
Original line: 250

DATASVC_ENABLED is now hardcoded to "true" for web-ng, which removes the ability to disable the supervised DataService.Client via chart values and changes behavior for existing installs. Consider wiring this to an explicit Helm value (or gating off .Values.kv.enabled / a new webNg.datasvc.enabled flag) so operators can disable datasvc client startup when datasvc is not deployed or connectivity is intentionally not configured.

          value: {{ default true $datasvc.enabled | quote }}

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499424 Original created: 2026-03-30T18:29:46Z Original path: helm/serviceradar/templates/web.yaml Original line: 250 --- DATASVC_ENABLED is now hardcoded to "true" for web-ng, which removes the ability to disable the supervised DataService.Client via chart values and changes behavior for existing installs. Consider wiring this to an explicit Helm value (or gating off .Values.kv.enabled / a new webNg.datasvc.enabled flag) so operators can disable datasvc client startup when datasvc is not deployed or connectivity is intentionally not configured. ```suggestion value: {{ default true $datasvc.enabled | quote }} ```

Copilot commented

2026-03-30 18:29:46 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499460
Original created: 2026-03-30T18:29:46Z
Original path: elixir/serviceradar_core/lib/serviceradar/data_service/client.ex
Original line: 112

with_channel/2 sets connect_timeout_ms to the overall request timeout when falling back to a direct connection. That can make a single operation block significantly longer than expected (GenServer call timeout + connect timeout + RPC timeout) and diverges from the configured DATASVC_CONNECT_TIMEOUT_MS behavior. Prefer letting connect/1 use its configured/default connect_timeout_ms (or using a bounded min(timeout, default_connect_timeout)).

        with {:ok, channel} <- connect(opts) do

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499460 Original created: 2026-03-30T18:29:46Z Original path: elixir/serviceradar_core/lib/serviceradar/data_service/client.ex Original line: 112 --- with_channel/2 sets connect_timeout_ms to the overall request timeout when falling back to a direct connection. That can make a single operation block significantly longer than expected (GenServer call timeout + connect timeout + RPC timeout) and diverges from the configured DATASVC_CONNECT_TIMEOUT_MS behavior. Prefer letting connect/1 use its configured/default connect_timeout_ms (or using a bounded min(timeout, default_connect_timeout)). ```suggestion with {:ok, channel} <- connect(opts) do ```

Copilot commented

2026-03-30 18:29:46 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499478
Original created: 2026-03-30T18:29:46Z
Original path: CHANGELOG
Original line: 5

Heading has a typo: "Whats New" should be "What's New".

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499478 Original created: 2026-03-30T18:29:46Z Original path: CHANGELOG Original line: 5 --- Heading has a typo: "Whats New" should be "What's New".

Copilot commented

2026-03-30 18:29:47 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499495
Original created: 2026-03-30T18:29:47Z
Original path: elixir/serviceradar_core/test/serviceradar/data_service/client_test.exs
Original line: 10

This test assumes the supervised DataService.Client process is not running. That can become environment-dependent if DATASVC_HOST/PORT/ADDRESS are set (datasvc_connectivity_configured?/0 enables datasvc even in :test) or if other tests start the application, which would make Process.whereis(Client) non-nil and fail the assertion. To keep it deterministic, consider making the test non-async and explicitly forcing DATASVC_ENABLED to "false" for the test (with on_exit cleanup), or otherwise isolating the scenario where the client is guaranteed not to be started.

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3103#discussion_r3011499495 Original created: 2026-03-30T18:29:47Z Original path: elixir/serviceradar_core/test/serviceradar/data_service/client_test.exs Original line: 10 --- This test assumes the supervised DataService.Client process is not running. That can become environment-dependent if DATASVC_HOST/PORT/ADDRESS are set (datasvc_connectivity_configured?/0 enables datasvc even in :test) or if other tests start the application, which would make Process.whereis(Client) non-nil and fail the assertion. To keep it deterministic, consider making the test non-async and explicitly forcing DATASVC_ENABLED to "false" for the test (with on_exit cleanup), or otherwise isolating the scenario where the client is guaranteed not to be started.