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

Adding simple dockerd config file to rotate logs from containers #74

Merged
merged 2 commits into from Nov 14, 2018

Conversation

Projects
None yet
10 participants
@max-rocket-internet
Copy link
Contributor

max-rocket-internet commented Oct 16, 2018

Related issue is here: #36

I myself have seen pods evicted due to disk pressure. Similar configuration is in place on other managed Kubernetes services already.

@micahhausler

This comment has been minimized.

Copy link
Member

micahhausler commented Oct 23, 2018

Hey thanks for this PR! We'll have to vet this and get back to you

@max-rocket-internet

This comment has been minimized.

Copy link
Contributor Author

max-rocket-internet commented Oct 24, 2018

We'll have to vet this and get back to you

Please do, I think it's important and there's already other users who have had issues with disk space. And I've had pods evicted due to disk pressure.

I actually tried to build the AMI myself to test it but packer fails to find the source AMI. Not sure how to fix that.

@rdubya16

This comment has been minimized.

Copy link

rdubya16 commented Oct 30, 2018

This seems like a must have in a default image. +1 from me

@ledor473

This comment has been minimized.

Copy link

ledor473 commented Oct 31, 2018

I've tested the PR but as is it's not working for me, I had to add:
sudo mkdir -p /etc/docker/

@max-rocket-internet

This comment has been minimized.

Copy link
Contributor Author

max-rocket-internet commented Oct 31, 2018

I've tested the PR but as is it's not working for me, I had to add:

OK done.

@max-rocket-internet

This comment has been minimized.

Copy link
Contributor Author

max-rocket-internet commented Oct 31, 2018

Why can't I build the AMI myself to test it?

$ make all
packer validate eks-worker-al2.json
Template validated successfully.
packer build -var source_ami_id=ami-0abb414482ac85a71 eks-worker-al2.json
amazon-ebs output will be in this color.

==> amazon-ebs: Prevalidating AMI Name: amazon-eks-node-1540976573
==> amazon-ebs: Error querying AMI: InvalidAMIID.NotFound: The image id '[ami-0abb414482ac85a71]' does not exist
==> amazon-ebs: 	status code: 400, request id: 6a22ead4-b494-42fd-a462-4d1071ac1613
Build 'amazon-ebs' errored: Error querying AMI: InvalidAMIID.NotFound: The image id '[ami-0abb414482ac85a71]' does not exist
	status code: 400, request id: 6a22ead4-b494-42fd-a462-4d1071ac1613
@max-rocket-internet

This comment has been minimized.

Copy link
Contributor Author

max-rocket-internet commented Oct 31, 2018

Ahh, I had to run export AWS_DEFAULT_REGION=us-west-2 first.

@ledor473

This comment has been minimized.

Copy link

ledor473 commented Oct 31, 2018

Yeah I ran into that issue too... to use us-east-1 you need to export AWS_DEFAULT_REGION and change this variable:
https://github.com/awslabs/amazon-eks-ami/blob/master/eks-worker-al2.json#L3

The other gotcha is that you can't update this variable to us-east-1 otherwise it don't work...
https://github.com/awslabs/amazon-eks-ami/blob/master/eks-worker-al2.json#L6

@max-rocket-internet

This comment has been minimized.

@willejs

This comment has been minimized.

Copy link

willejs commented Nov 14, 2018

@micahhausler would be great to see this released! 😸

@micahhausler
Copy link
Member

micahhausler left a comment

Thanks! We'll get this in the next release

@micahhausler micahhausler merged commit c281f32 into awslabs:master Nov 14, 2018

@max-rocket-internet max-rocket-internet deleted the max-rocket-internet:logrotate branch Nov 15, 2018

@gflarity

This comment has been minimized.

Copy link

gflarity commented Nov 15, 2018

Any eta when the image will be released? We're seeing prod issues that would be resolved by this.

@eswarbala

This comment has been minimized.

Copy link

eswarbala commented Nov 29, 2018

We are planning a release on Dec 5th with this patch.

@fcervantes2

This comment has been minimized.

Copy link

fcervantes2 commented Dec 7, 2018

@eswarbala is there a new date for the release which includes this patch? I was hoping to use official AMIs with this issue resolved.

@luispabon

This comment has been minimized.

Copy link

luispabon commented Dec 8, 2018

Has this been released yet?

@max-rocket-internet

This comment has been minimized.

Copy link
Contributor Author

max-rocket-internet commented Dec 12, 2018

Yes, it's in amazon-eks-node-1.10-v20181210 but still no release on this repo 🤔

@jpb

This comment has been minimized.

Copy link
Contributor

jpb commented Dec 12, 2018

Ah, it looks like the version/naming convention has changed. I was waiting for an AMI called amazon-eks-node-v26 (to match version 23 to 25, versions 18 to 22 were named slightly differently). It looks like amazon-eks-node-1.10-v20181210 is the one that is referenced in the documentation.

(Unrelated to this change – is it possible to have something like Subscribing to Amazon ECS-Optimized Amazon Linux AMI Update Notifications but for the EKS AMIs?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment