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

fix(eks-cluster-example): Add eks_worker_ami_name_filter variable to the example #32

Merged
merged 4 commits into from Nov 11, 2019

Conversation

vymarkov
Copy link
Contributor

Unfortunately, most_recent variable does not work as expected.

Enforce usage of eks_worker_ami_name_filter variable to set the right kubernetes version for EKS workers,
otherwise will be used the first version of Kubernetes supported by AWS (v1.11) for EKS workers but
EKS control plane will use the version specified by kubernetes_version variable.

+ kubectl version --short
Client Version: v1.16.0
Server Version: v1.14.6-eks-5047ed
+ kubectl get nodes
NAME                                              STATUS   ROLES    AGE   VERSION
ip-172-16-109-48.eu-central-1.compute.internal    Ready    <none>   12m   v1.11.10-eks-17cd81
ip-172-16-116-136.eu-central-1.compute.internal   Ready    <none>   99m   v1.11.10-eks-17cd81
ip-172-16-143-206.eu-central-1.compute.internal   Ready    <none>   99m   v1.11.10-eks-17cd81
ip-172-16-157-119.eu-central-1.compute.internal   Ready    <none>   12m   v1.11.10-eks-17cd81

Vitaly Markov and others added 3 commits November 10, 2019 21:35
Unfortunately, most_recent (https://github.com/cloudposse/terraform-aws-eks-workers/blob/34a43c25624a6efb3ba5d2770a601d7cb3c0d391/main.tf#L141)
variable does not work as expected, if you are not going to use custom ami you should
enforce usage of eks_worker_ami_name_filter variable to set the right kubernetes version for EKS workers,
otherwise will be used the first version of Kubernetes supported by AWS (v1.11) for EKS workers but
EKS control plane will use the version specified by kubernetes_version variable.
@vymarkov
Copy link
Contributor Author

Also I've updated readme with copy-paste to avoid the differences with an example 😄

@osterman osterman requested a review from aknysh November 11, 2019 03:11
@aknysh
Copy link
Member

aknysh commented Nov 11, 2019

/codefresh run test

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @vymarkov , looks good

Please run terraform fmt on all files including the examples.
And please rebuild README:

make init
make readme/deps
make readme

@vymarkov
Copy link
Contributor Author

/codefresh run test

@vymarkov
Copy link
Contributor Author

@aknysh done

@aknysh
Copy link
Member

aknysh commented Nov 11, 2019

/codefresh run test

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @vymarkov

@aknysh aknysh merged commit 992c667 into cloudposse:master Nov 11, 2019
@vymarkov vymarkov deleted the fix-aws-ami-eks-worker branch November 11, 2019 17:26
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.

None yet

2 participants