Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump docker/docker to master commit; cri-dockerd to 0.3.4 #8092

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

brandond
Copy link
Member

@brandond brandond commented Aug 1, 2023

Proposed Changes

Bump docker/docker to master commit; cri-dockerd to 0.3.4

Fixes issue with invalid HTTP host headers over unix sockets caused by recent releases of golang rejecting invalid header values.

Types of Changes

bugfix, version bump

Verification

See linked issue

Testing

Linked Issues

User-Facing Change

Bump docker/docker module version to fix issues with cri-dockerd caused by recent releases of golang rejecting invalid host headers sent by the docker client.

Further Comments

@brandond brandond requested a review from a team as a code owner August 1, 2023 20:35
@brandond brandond marked this pull request as draft August 1, 2023 20:38
@mgabeler-lee-6rs
Copy link

mgabeler-lee-6rs commented Aug 1, 2023

Is the Docker 24 API backwards compatible? I recall running into some issues with the 24.x client not being able to talk to 20.10 servers in a project of my own. Might want to grab the latest commit from the 20.10 branch instead, which was v20.10.26-0.20230730020612-0ad2952dc7f3+incompatible at least as of a couple days ago.

@brandond
Copy link
Member Author

brandond commented Aug 1, 2023

@mgabeler-lee-6rs it looks like that got backported to v20.10 in moby/moby#45972 so that should work if it does end up being incompatible - I'll see how it goes.

@brandond
Copy link
Member Author

brandond commented Aug 1, 2023

@mgabeler-lee-6rs seems to be OK:

root@ip-172-31-5-176:~# kubectl get node -o wide
NAME              STATUS   ROLES                  AGE   VERSION                INTERNAL-IP    EXTERNAL-IP   OS-IMAGE             KERNEL-VERSION    CONTAINER-RUNTIME
ip-172-31-5-176   Ready    control-plane,master   11m   v1.27.4+k3s-3ca6ce77   172.31.5.176   <none>        Ubuntu 22.04.2 LTS   5.19.0-1027-aws   docker://20.10.24

root@ip-172-31-5-176:~# kubectl get pod -A
NAMESPACE     NAME                                     READY   STATUS      RESTARTS   AGE
kube-system   local-path-provisioner-957fdf8bc-tl8vw   1/1     Running     0          13s
kube-system   coredns-77ccd57875-9g4dw                 1/1     Running     0          13s
kube-system   metrics-server-648b5df564-q5sb5          0/1     Running     0          13s
kube-system   traefik-64f55bb67d-5whfx                 0/1     Running     0          6s
kube-system   svclb-traefik-16b18966-jwbml             2/2     Running     0          6s
kube-system   helm-install-traefik-crd-lqkw6           0/1     Completed   0          13s
kube-system   helm-install-traefik-pcxcj               0/1     Completed   1          13s

root@ip-172-31-5-176:~# kubectl exec -it -n kube-system   svclb-traefik-16b18966-jwbml -c lb-tcp-80 -- /bin/sh -c "echo hello, world"
hello, world

Fixes issue with invalid HTTP host headers over unix sockets caused by
recent releases of golang rejecting invalid header values.

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
@brandond brandond marked this pull request as ready for review August 1, 2023 22:00
@brandond
Copy link
Member Author

brandond commented Aug 1, 2023

@mgabeler-lee-6rs if you'd like to test this on your nodes, try:

curl -sL get.k3s.io | INSTALL_K3S_COMMIT=647f1028614200d576645c58916703f033fed197 sh -s - server --docker

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Patch coverage has no change and project coverage change: +31.49% 🎉

Comparison is base (a87b183) 20.05% compared to head (647f102) 51.55%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8092       +/-   ##
===========================================
+ Coverage   20.05%   51.55%   +31.49%     
===========================================
  Files          83      143       +60     
  Lines        7694    14556     +6862     
===========================================
+ Hits         1543     7504     +5961     
+ Misses       5921     5863       -58     
- Partials      230     1189      +959     
Flag Coverage Δ
e2etests 49.23% <ø> (?)
inttests 44.56% <ø> (?)
unittests 20.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 119 files with indirect coverage changes

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

@mgabeler-lee-6rs
Copy link

it looks like that got backported to v20.10 in moby/moby#45972 so that should work if it does end up being incompatible

Yes, I can confirm the backport does work if it's needed based on usage in my projects.

if you'd like to test this on your nodes

Gave it a whirl, success! I think the API version compatibility issues I ran into with upgrading the docker client previously may have been me not understanding how to tell it to do version negotiation :)

Thanks!

@brandond brandond merged commit a0da8ed into k3s-io:master Aug 2, 2023
6 checks passed
@brandond brandond changed the title Bump docker/docker to master commit Bump docker/docker to master commit; cri-dockerd to 0.3.4 Aug 2, 2023
@brandond brandond deleted the fix-docker-host-header branch June 6, 2024 21:15
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.

5 participants