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: Use amazon alias for worker ami owner instead of owner id #1038

Merged
merged 2 commits into from
Oct 6, 2020

Conversation

barryib
Copy link
Member

@barryib barryib commented Oct 5, 2020

PR o'clock

Description

Use amazon alias for for worker ami owner instead of setting the owner id. Those owner ids are different by region, but the alias remain the same.

Resolves #1037
Resolves #480

#824
#480

Checklist

@barryib barryib mentioned this pull request Oct 5, 2020
1 task
@barryib barryib changed the title fix: Use AWS alias for worker ami oner instead of owner id fix: Use amazon alias for worker ami oner instead of owner id Oct 5, 2020
@barryib
Copy link
Member Author

barryib commented Oct 6, 2020

@terraform-aws-modules/triage-supporters Someone can help me review this ? I'm not a ninja of AMI filtering, so I would like have your feedback on this change. I don't know why we're using the AWS owner ID instead of alias since the beginning. I probably missed something.

@antonbabenko antonbabenko changed the title fix: Use amazon alias for worker ami oner instead of owner id fix: Use amazon alias for worker ami owner instead of owner id Oct 6, 2020
@antonbabenko
Copy link
Member

I usually debug this is by running TF_LOG=debug terraform plan and see what kind of API call terraform sends to AWS. Example code should be very small to actually understand the verbose response.

@dpiddock
Copy link
Contributor

dpiddock commented Oct 6, 2020

The Amazon EKS node images from the container team are not in the same account as standard images from the EC2 amazon linux team. Although it appears that they both have the amazon ImageOwnerAlias. Maybe in the before times when the module was written this was not true so the account ID had to be specified?

Interesting that account aliases are not a one-to-one mapping to account IDs.

For example grab the, at time of writing, latest image for Amazon Linux 2 AMI (HVM) SSD 64-bit x86 (in eu-west-1 it's ami-0bb3fad3c0286ebd5), and query for the OwnerId and ImageOwnerAlias:

aws ec2 describe-images --image-ids ami-0bb3fad3c0286ebd5 --query Images[].[Name,OwnerId,ImageOwnerAlias]
[
    [
        "amzn2-ami-hvm-2.0.20200917.0-x86_64-gp2",
        "137112412989",
        "amazon"
    ]
]

The current EKS 1.17 AMZN2 image in eu-west-1 is ami-0c557e2a5ede1bae3:

aws ec2 describe-images --image-ids ami-0c557e2a5ede1bae3 --query Images[].[Name,OwnerId,ImageOwnerAlias]
[
    [
        "amazon-eks-node-1.17-v20201002",
        "602401143452",
        "amazon"
    ]
]

Existing module query:

$ aws ec2 describe-images --owner 602401143452 --filter 'Name=name,Values=amazon-eks-node-1.17-v*' --query 'sort_by(Images, &CreationDate)[].[Name,ImageId]' --output text | tail -n1
amazon-eks-node-1.17-v20201002	ami-0c557e2a5ede1bae3

$ aws ec2 describe-images --owner 801119661308 --filter 'Name=name,Values=Windows_Server-2019-English-Core-EKS_Optimized-1.17-**' --query 'sort_by(Images, &CreationDate)[].[Name,ImageId]' --output text | tail -n1
Windows_Server-2019-English-Core-EKS_Optimized-1.17-2020.09.09	ami-059dd50aa82b04356

New query:

$ aws ec2 describe-images --owner amazon --filter 'Name=name,Values=amazon-eks-node-1.17-v*' --query 'sort_by(Images, &CreationDate)[].[Name,ImageId]' --output text | tail -n1
amazon-eks-node-1.17-v20201002	ami-0c557e2a5ede1bae3

$ aws ec2 describe-images --owner amazon --filter 'Name=name,Values=Windows_Server-2019-English-Core-EKS_Optimized-1.17-**' --query 'sort_by(Images, &CreationDate)[].[Name,ImageId]' --output text | tail -n1
Windows_Server-2019-English-Core-EKS_Optimized-1.17-2020.09.09	ami-059dd50aa82b04356

Looks good to me.

@barryib barryib merged commit 094e363 into terraform-aws-modules:master Oct 6, 2020
@barryib barryib deleted the tba/aws-ami-alias branch October 6, 2020 12:26
@barryib
Copy link
Member Author

barryib commented Oct 6, 2020

Thanks @dpiddock.

barryib added a commit to Polyconseil/terraform-aws-eks that referenced this pull request Oct 25, 2020
@philomory
Copy link

philomory commented Oct 29, 2020

This doesn't actually resolve #480, because for some reason, the account that owns the EKS AMIs in ap-east-1 has no alias whatsoever:

$ aws ec2 describe-images --region ap-east-1 --image-ids ami-0824aac4c54c763d3 --query 'Images[].Name,OwnerId,ImageOwnerAlias]' --output text
amazon-eks-node-1.18-v20201007  800184023465    None

$ aws ec2 describe-images --region ap-east-1 --owner 800184023465 --filter 'Name=name,Values=amazon-eks-node-1.18-v*' --query 'Images[].[Name,ImageId]' --output text
amazon-eks-node-1.18-v20201007  ami-0824aac4c54c763d3

$ aws ec2 describe-images --region ap-east-1 --owner amazon --filter 'Name=name,Values=amazon-eks-node-1.18-v*' --query 'Images[].[Name,ImageId]' --output text

$

That being the case, I don't think there's any way to really fix this other than either hard-coding a default list of multiple owner IDs, hard-coding a default mapping between regions and owner ids, or just expecting the module user to figure it out on their own. Or, I guess, bugging Amazon to add an account alias to 800184023465.

Just to be clear, ami-0824aac4c54c763d3 is the AMI officially listed by Amazon for Hong Kong in their documentation: https://docs.aws.amazon.com/eks/latest/userguide/eks-optimized-ami.html

@barryib
Copy link
Member Author

barryib commented Oct 29, 2020

Hummm... Maybe the right way to fix this is to use the SSM datasource to get the AMI ID. See https://docs.aws.amazon.com/eks/latest/userguide/retrieve-ami-id.html. But I don't know how to do that without breaking all the AMI filtering logic in this module.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Module doesn't work on eu-south-1 data aws_ami "eks_worker" is failing in ap-east-1
4 participants