Skip to content

refactor(dockerd): replace deprecated ctxhttp with http.NewRequestWithContext - #716

Merged
gkeesh7 merged 2 commits into
uber:masterfrom
reginaldalfret:fix/dockerd-client-ctxhttp-deprecation
Oct 22, 2026
Merged

refactor(dockerd): replace deprecated ctxhttp with http.NewRequestWithContext#716
gkeesh7 merged 2 commits into
uber:masterfrom
reginaldalfret:fix/dockerd-client-ctxhttp-deprecation

Conversation

@reginaldalfret

@reginaldalfret reginaldalfret commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

What this does

  • Replaces the deprecated golang.org/x/net/context/ctxhttp call in lib/containerruntime/dockerdaemon/cli.go with standard library http.NewRequestWithContext(ctx, ...) and cli.client.Do(req).
  • Removes the unused ctxhttp import.
  • Adds comprehensive unit tests in lib/containerruntime/dockerdaemon/cli_test.go covering parseHost (TCP and Unix sockets), NewDockerClient, and PullImage (success, HTTP error status, and context cancellation).

Why

Fixes #709.

ctxhttp has been deprecated since Go 1.7 added native context support to
et/http. Furthermore, dockerdaemon lacked unit test coverage.

Verification

  • Added and passed unit tests via go test -v ./lib/containerruntime/dockerdaemon/...:
    • TestParseHost: verified TCP, TCP with path, Unix socket, and invalid format/length errors
    • TestNewDockerClient: verified client initialization
    • TestPullImageSuccess: verified request method, path, headers, query params
    • TestPullImageErrorResponse: verified non-200 status handling
    • TestPullImageContextCanceled: verified context cancellation propagation
  • Confirmed ctxhttp is no longer used anywhere in lib/containerruntime.

Copilot AI balanced review requested due to automatic review settings October 8, 2026 12:56
Copilot AI reviewed Oct 8, 2026

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@CLAassistant

CLAassistant commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@gkeesh7

gkeesh7 commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

@reginaldalfret Please address the comments by @sambhav-jain-16! Rest LGTM!

Copilot AI balanced review requested due to automatic review settings October 13, 2026 12:36
Copilot AI reviewed Oct 13, 2026

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@reginaldalfret

reginaldalfret commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@sambhav-jain-16 @gkeesh7 I have updated the branch with the requested table-driven refactoring and fixed the errcheck lint errors on w.Write. All tests pass locally.

sambhav-jain-16
sambhav-jain-16 approved these changes Oct 16, 2026
View reviewed changes

@sambhav-jain-16 sambhav-jain-16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution
LGTM

@gkeesh7
gkeesh7 merged commit 884429e into uber:master Oct 22, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@sambhav-jain-16 sambhav-jain-16 sambhav-jain-16 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dockerd client uses deprecated ctxhttp package

5 participants

@reginaldalfret @CLAassistant @gkeesh7 @sambhav-jain-16