-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add ability to set the MTU size of the docker in docker container #385
Conversation
Out of interest do you know why your MTU is 1450? From my knowledge 1500 is super duper standard unless you are doing something like jumbo frames |
This decision was made before my time, but from what I am told, it is because of OpenShift's SDN was having packet loss do to largely the same issue, so they jacked the MTU size for our cluster's nodes down to 1450. I get this isn't a common use case, but I imagine it will crop up from time to time. |
controllers/runner_controller.go
Outdated
@@ -530,6 +530,10 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { | |||
}, | |||
} | |||
|
|||
if dockerdInRunner { | |||
pod.Spec.Containers[0].Command = []string{"startup.sh", fmt.Sprintf("--mtu %d", *runner.Spec.DockerMTU)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like to fail when DockerMTU is nil. Probably we'd better enclose this inside a if block?
pod.Spec.Containers[0].Command = []string{"startup.sh", fmt.Sprintf("--mtu %d", *runner.Spec.DockerMTU)} | |
if mtu := runner.Spec.DockerMTU; mtu != nil { | |
pod.Spec.Containers[0].Command = []string{"startup.sh", fmt.Sprintf("--mtu %d", *runner.Spec.DockerMTU)} | |
} |
controllers/runner_controller.go
Outdated
@@ -600,6 +604,7 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { | |||
MountPath: "/certs/client", | |||
}, | |||
}, | |||
Args: []string{fmt.Sprintf("--mtu %d", *runner.Spec.DockerMTU)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be easier to just set MTU
via an envvar, not over --mtu
flag?
Env: []corev1.EnvVar{
{
Name: "MTU",
Value: fmt.Sprintf("%d", *Zrunner.Spec.DockerMTU),
},
},
@callum-tait-pbx @bkimbrough88 I can imagine where this feature become handy- I've experienced a similar issue running drone in some on-prem environment and had to use mtu of 1450. I've also found that there are a few folks experiencing similar issues in https://discourse.drone.io/t/docker-mtu-problem/1207 |
Probably should update the additional tweaks section of the docs to include an example maybe? |
@callum-tait-pbx Yeah that would be great 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks a lot for your support @bkimbrough88 and @callum-tait-pbx
It would be great if you could also add some doc in our README. But that's another story!
I have encountered a problem where the Max Transmission Unit (MTU) in the docker in docker container is set higher than my ethernet interface's MTU. This is causing packets to get dropped and my runs to hang indefinitely. To fix this, I need to be able to set the MTU to a value lower than my ethernet interface's.
Here is the output from ifconfig that clued me into what my problem actually was, as you can see,
docker0
has a higher MTU value thaneth0
and that is known to cause packet loss.