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

INSTALL: Document kube-proxy settings #391

Merged
merged 1 commit into from
Dec 9, 2019
Merged

INSTALL: Document kube-proxy settings #391

merged 1 commit into from
Dec 9, 2019

Conversation

sam-aws
Copy link
Contributor

@sam-aws sam-aws commented Oct 10, 2019

Issue #, if available:
#371

Description of changes:
The kube-proxy configuration must be updated to prevent it from
overriding Thar's sysctl change to nf_conntrack_max. Document the steps
to update the configuration in the install steps.

Fixes #371 for kernel settings.

Signed-off-by: Samuel Mendoza-Jonas samjonas@amazon.com

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sam-aws sam-aws requested a review from tjkirch October 10, 2019 17:23
@sam-aws sam-aws added this to In progress in Docs for public launch via automation Oct 10, 2019
Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Is this the only way? :(

INSTALL.md Outdated Show resolved Hide resolved
INSTALL.md Outdated
@@ -230,6 +230,28 @@ Save the file and confirm that the changes have been applied:
kubectl describe configmap -n kube-system aws-auth
```

## kube-proxy settings
By default `kube-proxy` will set the `nf_conntrack_max` kernel parameter to a
small value, overriding [Thar's default
Copy link
Contributor

Choose a reason for hiding this comment

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

I have mixed feelings about this wording since it suggests an issue with the default EKS configuration, when there may not really be an issue.

Maybe we need --conntrack-min 0 to mean "apply the new setting but only if it's an increase over the current system value" i.e. an upstream behavior change.

We could also ask EKS to consider changing --conntrack-min to align with our value.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bcressey Are you happier with the new wording?

@sam-aws
Copy link
Contributor Author

sam-aws commented Oct 11, 2019

Is this the only way? :(

I'll see if I can dig up some more k8s knowledge on this, I wonder if we can set it as part of teksctl?

I have mixed feelings about this wording since it suggests an issue with the default EKS configuration, when there may not really be an issue.

Good point, it would be better to say something more along the lines of "...if these values happen to be different".

@sam-aws
Copy link
Contributor Author

sam-aws commented Oct 15, 2019

In lieu of finding a nicer way to set this, pushed a change to the wording of the instructions.

@jahkeup
Copy link
Member

jahkeup commented Oct 15, 2019

I'll see if I can dig up some more k8s knowledge on this, I wonder if we can set it as part of teksctl?

I didn't see anything on a quick look over the code that handles this, but I do think we could insert the modification to the DaemonSet during the cluster bootstrapping phase before it starts a node. This would prevent it from making changes between the time we have the Scheduler launch these Pods on a Node and us changing the setting after the fact.

@iliana
Copy link
Contributor

iliana commented Nov 19, 2019

@bcressey can we merge this?

INSTALL.md Show resolved Hide resolved
@tjkirch tjkirch requested review from bcressey and removed request for bcressey November 21, 2019 20:43
The kube-proxy configuration must be updated to prevent it from
overriding Thar's sysctl change to nf_conntrack_max. Document the steps
to update the configuration in the install steps.

Fixes #371 for kernel settings.

Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com>
@sam-aws
Copy link
Contributor Author

sam-aws commented Dec 6, 2019

Rebased

@sam-aws sam-aws merged commit 1220108 into develop Dec 9, 2019
Docs for public launch automation moved this from In progress to Done Dec 9, 2019
@sam-aws sam-aws deleted the kube-proxy branch December 9, 2019 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

integrate recommended node settings for EKS
5 participants