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

kubelet, containerd: replace 'containerd.sock' with 'dockershim.sock' #796

Merged
merged 2 commits into from
Feb 27, 2020

Conversation

etungsten
Copy link
Contributor

@etungsten etungsten commented Feb 27, 2020

Issue number: Fixes #747 (comment)

Description of changes:
Let containerd create the container runtime endpoint as
/run/dockershim.sock instead of /run/containerd/containerd.sock

Pass the new container runtime endpoint to kubelet as /run/dockershim.sock

Add changelog entry for v0.3.0

Testing done:
Built image, launched bottlerocket instance, connects to my cluster fine, pods can run fine.
Restarting aws-node pods and they still works fine.
Checking ipamd shows IP addresses are being allocated successfully:

2020-02-27T20:49:59.344Z [DEBUG]        GetLocalPods start ...                                                                                       [58/4445]
2020-02-27T20:49:59.344Z [DEBUG]        getLocalPodsWithRetry() found 0 local pods
2020-02-27T20:49:59.344Z [INFO]         Serving RPC Handler on 127.0.0.1:50051
2020-02-27T20:49:59.344Z [INFO]         Serving introspection endpoints on 127.0.0.1:61679
2020-02-27T20:49:59.344Z [INFO]         Serving metrics on port 61678
2020-02-27T20:49:59.344Z [INFO]         Setting up shutdown hook.
2020-02-27T20:49:59.549Z [INFO]         Add/Update for CNI pod aws-node-xzq82
2020-02-27T20:50:01.844Z [DEBUG]        IP pool stats: total = 9, used = 0, c.maxIPsPerENI = 9
2020-02-27T20:50:01.844Z [DEBUG]        IP pool stats: total = 9, used = 0, c.maxIPsPerENI = 9
2020-02-27T20:50:04.344Z [DEBUG]        Reconciling ENI/IP pool info...
2020-02-27T20:50:04.345Z [DEBUG]        Total number of interfaces found: 1
2020-02-27T20:50:04.345Z [DEBUG]        Found ENI mac address : 02:8a:99:2c:e9:a4
2020-02-27T20:50:04.346Z [DEBUG]        Using device number 0 for primary eni: eni-0fd6b8b6380328418
2020-02-27T20:50:04.346Z [DEBUG]        Found ENI: eni-0fd6b8b6380328418, MAC 02:8a:99:2c:e9:a4, device 0
2020-02-27T20:50:04.346Z [DEBUG]        Found CIDR 192.168.0.0/19 for ENI 02:8a:99:2c:e9:a4
2020-02-27T20:50:04.347Z [DEBUG]        Found IP addresses [192.168.10.57 192.168.2.42 192.168.4.27 192.168.12.107 192.168.31.76 192.168.7.62 192.168.17.222 1
92.168.17.111 192.168.15.34 192.168.3.69] on ENI 02:8a:99:2c:e9:a4
2020-02-27T20:50:04.387Z [DEBUG]        Reconcile existing ENI eni-0fd6b8b6380328418 IP pool
2020-02-27T20:50:04.387Z [DEBUG]        Reconcile and skip primary IP 192.168.10.57 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.2.42 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.4.27 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.12.107 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.31.76 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.7.62 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.17.222 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.17.111 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.15.34 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Reconciled IP 192.168.3.69 on ENI eni-0fd6b8b6380328418
2020-02-27T20:50:04.387Z [DEBUG]        Successfully Reconciled ENI/IP pool
....
2020-02-27T21:06:49.197Z [DEBUG]        Getting running pod sandboxes from "unix:///var/run/dockershim.sock"
2020-02-27T21:06:49.198Z [DEBUG]        Found pod(nginx-2-568ffd7464-7f9hd)'s sandbox ID: c11e9e2270efb82d4684b3102ed48f7862b528b8408af80b3abced9a3779a26d
2020-02-27T21:06:49.198Z [DEBUG]        Found pod(nginx-2-568ffd7464-frl62)'s sandbox ID: a2e42c03c5cfcaf376209dab03e64d95d894eed23b911120ac34c672e267151b
2020-02-27T21:06:49.198Z [DEBUG]        Found pod(nginx-2-568ffd7464-vwg9w)'s sandbox ID: f12e6c5b7f264bbaeb2731b40e8d3fc8cc660492b6e5075aaac8e12e43e39861
2020-02-27T21:06:49.198Z [DEBUG]        Found pod(nginx-2-568ffd7464-g4fhj)'s sandbox ID: a22dc68c0d5367a246bac81e075dabb80a2915876d3fe1f5e6c3ba0ade10539f
2020-02-27T21:06:49.198Z [DEBUG]        getLocalPodsWithRetry() found 4 local pods
2020-02-27T21:06:49.198Z [INFO]         Recovered AddNetwork for Pod nginx-2-568ffd7464-7f9hd, Namespace default, Sandbox c11e9e2270efb82d4684b3102ed48f7862b5
28b8408af80b3abced9a3779a26d
2020-02-27T21:06:49.198Z [DEBUG]        AssignIPv4Address: IP address pool stats: total: 18, assigned 0
2020-02-27T21:06:49.198Z [INFO]         AssignPodIPv4Address: Reassign IP 192.168.7.62 to pod (name nginx-2-568ffd7464-7f9hd, namespace default)
2020-02-27T21:06:49.198Z [INFO]         Update Rule List[[ip rule -1: from <nil> table 255 ip rule 512: from <nil> table 254 ip rule 512: from <nil> table 254
 ip rule 512: from <nil> table 254 ip rule 512: from <nil> table 254 ip rule 1024: from <nil> table 254 ip rule 1536: from 192.168.28.158/32 table 2 ip rule 3
2766: from <nil> table 254 ip rule 32767: from <nil> table 253]] for source[{192.168.7.62 ffffffff}] with toCIDRs[[192.168.0.0/16]], excludeSNATCIDRs[[]], req
uiresSNAT[true]

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@bcressey
Copy link
Contributor

bcressey commented Feb 27, 2020

I would like to see an issue opened to track:

  • make containerd socket path configurable via API
  • set default socket path something else in aws-k8s-1.16 and beyond

Can either be containerd.sock or cri.sock if we feel the latter is closer to being a standard interface.

@etungsten
Copy link
Contributor Author

Addresses @bcressey 's comments. Removed the change from aws-dev variant and docker.service

@jahkeup
Copy link
Member

jahkeup commented Feb 27, 2020

Built image, launched bottlerocket instance, connects to my cluster fine, pods can run fine.
Restarting aws-node pods and they still works fine.

To confirm, this was done with the default aws-node.yaml where the cluster had no trace of the modified/patched deployment, right?

Let containerd create the container runtime endpoint as
'/run/dockershim.sock' instead of '/run/containerd/containerd.sock'

Pass the new container runtime endpoint to kubelet as '/run/dockershim.sock'
@etungsten
Copy link
Contributor Author

Built image, launched bottlerocket instance, connects to my cluster fine, pods can run fine.
Restarting aws-node pods and they still works fine.

To confirm, this was done with the default aws-node.yaml where the cluster had no trace of the modified/patched deployment, right?

That is correct!

@etungsten
Copy link
Contributor Author

Force push above rebases on develop

@etungsten etungsten requested a review from tjkirch February 27, 2020 21:24
Copy link
Member

@jahkeup jahkeup left a comment

Choose a reason for hiding this comment

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

Changes lgtm! All that's left now is to figure out how to undo 😎

CHANGELOG.md Outdated Show resolved Hide resolved
@etungsten
Copy link
Contributor Author

Addresses @tjkirch 's comments.

@jaypipes
Copy link

jaypipes commented Apr 4, 2022

@bcressey @jahkeup @etungsten I know this is an old commit, but I'm wondering about the reasoning behind it. Was there a specific bug that changing the containerd sock path to something other than /run/containerd/containerd.sock solved? 2 years later and we're trying to get bottlerocket+ContainerInsights working together properly.

@bcressey
Copy link
Contributor

bcressey commented Apr 4, 2022

At the time, this was done for compatibility with the VPC CNI plugin, which didn't include the containerd socket in its mounts.

It avoided the need for an edit to the CNI config as a prerequisite to getting started with Bottlerocket and EKS.

We could likely switch back to the default path now, but would need to check how long the relevant fixes in the VPC CNI plugin have been deployed.

@jaypipes
Copy link

jaypipes commented Apr 4, 2022

At the time, this was done for compatibility with the VPC CNI plugin, which didn't include the containerd socket in its mounts.

It avoided the need for an edit to the CNI config as a prerequisite to getting started with Bottlerocket and EKS.

We could likely switch back to the default path now, but would need to check how long the relevant fixes in the VPC CNI plugin have been deployed.

Gotcha, OK that makes sense (and I vaguely remember the conversation in VPC CNI around this... :) ).

I think it would be good to align back to the /run/containerd/containerd.sock path but it's not super urgent.

etungsten added a commit to etungsten/bottlerocket that referenced this pull request Jun 2, 2022
This changes containerd's container runtime socket to '/run/containerd/containerd.sock'
from '/run/dockershim.sock'.

This change reverts changes done in bottlerocket-os#796

More K8s deployments are now containerd-aware so customers are having to
do daemonset edits for solutions that assume containerd as the
underlying runtime.

In the containerd systemd service, we create a symlink for the
containerd socket at `/run/dockershim.sock` just so other deployments
that assume docker as underlying runtime can still work.
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.

default aws-k8s-cni config doesn't work
5 participants