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

terraform: fix docker ulimit configuring for latest EKS AMI #1349

Merged
merged 8 commits into from
Dec 19, 2019

Conversation

aylei
Copy link
Contributor

@aylei aylei commented Dec 17, 2019

Signed-off-by: Aylei rayingecho@gmail.com

close #1344
close #1302

The latest AMI of EKS has a break change that it runs containerd as a separate systemd service, for which we have not adjusted the ulimit. Configuring ulimit for containers is a more general method, ref: https://docs.docker.com/engine/reference/commandline/dockerd/

Tested on AWS with the latest EKS AMI.

Does this PR introduce a user-facing change?:

Terraform: fix docker ulimit configuring for latest EKS AMI.

@aylei aylei requested review from weekface, cofyc and tennix December 17, 2019 06:36
cofyc
cofyc previously approved these changes Dec 17, 2019
Copy link
Contributor

@cofyc cofyc left a comment

Choose a reason for hiding this comment

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

LGTM

@aylei
Copy link
Contributor Author

aylei commented Dec 17, 2019

/run-e2e-test

weekface
weekface previously approved these changes Dec 17, 2019
@aylei aylei dismissed stale reviews from weekface and cofyc via 1fb1bf6 December 17, 2019 06:52
Signed-off-by: Aylei <rayingecho@gmail.com>
@aylei
Copy link
Contributor Author

aylei commented Dec 17, 2019

fix a mistake in command order @cofyc @tennix @weekface PTAL again

@aylei aylei requested review from cofyc, tennix and weekface December 17, 2019 06:57
Signed-off-by: Aylei <rayingecho@gmail.com>
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Aylei <rayingecho@gmail.com>
Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

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

LGTM

@aylei aylei requested a review from DanielZhangQD December 18, 2019 11:12
@aylei
Copy link
Contributor Author

aylei commented Dec 18, 2019

/run-e2e-test

@aylei
Copy link
Contributor Author

aylei commented Dec 18, 2019

Comments addressed @weekface @DanielZhangQD @cofyc PTAL again

Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

@aylei
Copy link
Contributor Author

aylei commented Dec 18, 2019

/run-e2e-test

@aylei
Copy link
Contributor Author

aylei commented Dec 19, 2019

/run-e2e-test

2 similar comments
@aylei
Copy link
Contributor Author

aylei commented Dec 19, 2019

/run-e2e-test

@aylei
Copy link
Contributor Author

aylei commented Dec 19, 2019

/run-e2e-test

@aylei aylei merged commit df1755e into pingcap:master Dec 19, 2019
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.

Terraform script broken on latest AWS AMI Considering adjust containerd systemd service ulimit on EKS
5 participants