-
Notifications
You must be signed in to change notification settings - Fork 49
Expose --conntrack-max-per-core kube-proxy flag #1187
Conversation
68aa5ca
to
b24a57c
Compare
@@ -42,6 +42,9 @@ spec: | |||
- --proxy-mode=iptables | |||
- --metrics-bind-address=$(HOST_IP) | |||
- --healthz-bind-address=$(HOST_IP) | |||
{{- if not (eq (int .Values.kubeProxy.conntrackMaxPerCore) 32768) }} |
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.
We can get rid of this if block if we provide the same default value as whatever is set in the node right now.
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.
This line allows kube-proxy
to steer the default value, allowing us to update it asynchronously if it ever changes.
It also reduces final manifest size, I don't see a value in specifying the flag which is exactly the same as default value. I'd prefer to keep it.
assets/terraform-modules/aws/flatcar-linux/kubernetes/variables.tf
Outdated
Show resolved
Hide resolved
assets/terraform-modules/packet/flatcar-linux/kubernetes/variables.tf
Outdated
Show resolved
Hide resolved
b24a57c
to
8d45554
Compare
@invidian I can't review this right now due to load, sorry. Please assign someone else. |
62f1d0a
to
03cfea2
Compare
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.
Nice work, some minor suggestions
03cfea2
to
5038d83
Compare
5038d83
to
a28c642
Compare
assets/terraform-modules/aws/flatcar-linux/kubernetes/variables.tf
Outdated
Show resolved
Hide resolved
a28c642
to
2b5583a
Compare
Also added configuration validation rules and unit tests for that. |
@invidian can you please check the CI errors? |
@knrt10 they were caused by Docker Hub limits, which should be now resolves, so I restarted them. |
2b5583a
to
6403492
Compare
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
6403492
to
cbd94cf
Compare
Have to re-run the tests again after rebasing for bare metal to pass 😢 |
Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Signed-off-by: knrt10 <kautilya@kinvolk.io>
This commit exposes --conntrack-max-per-core kube-proxy flag in kubernetes Helm chart and adds required plumbing to expose it to the user using HCL. It also adds sample usage to CI configuration and e2e tests to verify that settings are properly applied. Closes #1081 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
cbd94cf
to
ebf81dd
Compare
This PR exposes --conntrack-max-per-core kube-proxy flag in
kubernetes Helm chart and adds required plumbing to expose it to the
user using HCL.
It also adds sample usage to CI configuration and e2e tests to verify
that settings are properly applied.
Closes #1081
Signed-off-by: Mateusz Gozdek mateusz@kinvolk.io