Skip to content

Fix remote workload restart port bind error when unauthenticated#3975

Open
amirejaz wants to merge 2 commits intomainfrom
remote-workload-restart-port-bind
Open

Fix remote workload restart port bind error when unauthenticated#3975
amirejaz wants to merge 2 commits intomainfrom
remote-workload-restart-port-bind

Conversation

@amirejaz
Copy link
Contributor

@amirejaz amirejaz commented Mar 3, 2026

Problem

When a remote workload becomes unauthenticated (e.g., token refresh fails when the laptop disconnects from the internet), the proxy process continues running and holding the port. If the user runs thv restart, it fails with "port already in use" because we never stopped the existing proxy before attempting to start a new one.

Solution

  1. Stop proxy before restart: Always call stopProxyIfNeeded before loading config and starting a new runner for any restart path.

  2. Unified restart flow: Consolidate the restart logic for all remote workload states (Running+dead, Unauthenticated, Unhealthy, Error). The flow now:

    • Stops the proxy first (so "Stopped" status matches actual state)
    • Sets Stopping → removeClientConfigurations → Stopped
    • Loads config and starts the new runner

    This removes special handling for Running+dead and keeps status consistent with reality.

Changes

  • pkg/workloads/manager.go: Refactor maybeSetupRemoteWorkload to use the unified restart flow.
  • pkg/workloads/manager_test.go: Update tests for the new flow; add a test for unauthenticated restart.

Testing

Unit tests updated and passing. Covers:

  • Remote workload with healthy supervisor (no-op)
  • Remote workload with dead supervisor (restart with unified flow)
  • Remote workload unauthenticated (restart with unified flow)

@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 3, 2026
@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.43%. Comparing base (7acf39c) to head (7d736ef).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/workloads/manager.go 14.28% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3975      +/-   ##
==========================================
- Coverage   68.44%   68.43%   -0.01%     
==========================================
  Files         437      437              
  Lines       44363    44362       -1     
==========================================
- Hits        30363    30358       -5     
- Misses      11632    11636       +4     
  Partials     2368     2368              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Mar 3, 2026
@eleftherias
Copy link
Member

The changes make sense. I'll wait for the PR checks to be green before approving. It looks like there's a flaky tests causing a failure now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants