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

Support bottlerocket ami types #93

Merged
merged 20 commits into from Nov 11, 2021
Merged

Support bottlerocket ami types #93

merged 20 commits into from Nov 11, 2021

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Nov 4, 2021

what

  • Support bottlerocket ami types
  • Removed unused null and template providers

why

  • This will allow us to support bottlerocket ami using an ami_type
  • template provider removal will allow OSX M1 to use this module

references

https://docs.aws.amazon.com/eks/latest/APIReference/API_Nodegroup.html

amiType

    If the node group was deployed using a launch template with a custom AMI, then this is CUSTOM. For node groups that weren't deployed using a launch template, this is the AMI type that was specified in the node group configuration.

    Type: String

    Valid Values: AL2_x86_64 | AL2_x86_64_GPU | AL2_ARM_64 | CUSTOM | BOTTLEROCKET_ARM_64 | BOTTLEROCKET_x86_64

    Required: No

notes

  • There is no GPU version of bottlerocket
Here are the following data source checks to verify backwards compatibility.

With a temporary output in ami.tf and examples/complete/outputs.tf

output "ami_regex" {
  value = local.ami_regex[local.ami_kind],
}

And setting after_cluster_joining_userdata = ["ls"] to trigger needing an ami.

ami_type = "AL2_x86_64" and ami_release_version = ["1.21.0-20211013"]

# matches amazon-eks-node-1.21-v20211013 in us-east-2
ami_regex = amazon-eks-node-1.21-v20211013

ami_type = "AL2_x86_64" and no ami_release_version

# matches amazon-eks-node-1.21-v20211013 in us-east-2
ami_regex = amazon-eks-node-1.21-*

ami_type = "BOTTLEROCKET_x86_64" and ami_release_version = ["1.2.0-ccf1b754"]

# matches bottlerocket-aws-k8s-1.21-x86_64-v1.2.0-ccf1b754 in us-east-2
ami_regex = bottlerocket-aws-k8s-1.21-x86_64-v1.2.0-ccf1b754

ami_type = "BOTTLEROCKET_x86_64" and no ami_release_version

# matches bottlerocket-aws-k8s-1.21-x86_64-v1.3.0-395b459c in us-east-2
ami_regex = bottlerocket-aws-k8s-1.21-x86_64-*

ami_type = "BOTTLEROCKET_ARM_64" and ami_release_version = ["1.2.0-ccf1b754"]

# matches bottlerocket-aws-k8s-1.21-aarch64-v1.2.0-ccf1b754 in us-east-2
ami_regex = "bottlerocket-aws-k8s-1.21-aarch64-v1.2.0-ccf1b754"

ami_type = "BOTTLEROCKET_ARM_64" and no ami_release_version

# matches bottlerocket-aws-k8s-1.21-aarch64-v1.3.0-395b459c in us-east-2
ami_regex = "bottlerocket-aws-k8s-1.21-aarch64-*"

variables.tf Outdated Show resolved Hide resolved
@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode changed the title Support ami_name_regex Support var.ami_name_regex override Nov 4, 2021
@Nuru Nuru added do not merge Do not merge this PR, doing so would cause problems wontfix This will not be worked on labels Nov 4, 2021
@Nuru
Copy link
Sponsor Contributor

Nuru commented Nov 4, 2021

I am closing this as wontfix because I don't like it, which means it can be reopened if a consensus develops that there are good reasons to do it even though I don't like it. :-)

I don't like it because I think this is dangerous feature creep, and not necessary to achieve desired goals. Users who are not happy with the standard AMI regex should select the AMI outside of this module and then feed in the desired AMI ID via the ami_image_id input. Regular expressions are notoriously problematic, and this PR does not further parameterize the most_recent or owners inputs to the aws_ami data source, nor do I want to add those parameters. (variables.tf is already over 400 lines and I don't want to add even more options, because every option adds to the complexity of understanding and using the module and the complexity of supporting it.)

@Nuru Nuru closed this Nov 4, 2021
@nitrocode nitrocode changed the title Support var.ami_name_regex override Support bottlerocket Nov 5, 2021
@nitrocode nitrocode reopened this Nov 5, 2021
@nitrocode nitrocode marked this pull request as draft November 5, 2021 19:54
@nitrocode nitrocode changed the title Support bottlerocket Support bottlerocket (wip) Nov 5, 2021
@nitrocode nitrocode removed wontfix This will not be worked on do not merge Do not merge this PR, doing so would cause problems labels Nov 5, 2021
@nitrocode nitrocode changed the title Support bottlerocket (wip) Support bottlerocket ami types Nov 5, 2021
@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode marked this pull request as ready for review November 5, 2021 21:55
@nitrocode
Copy link
Member Author

/test all

Nuru
Nuru previously requested changes Nov 5, 2021
ami.tf Outdated Show resolved Hide resolved
@mergify mergify bot dismissed Nuru’s stale review November 5, 2021 23:59

This Pull Request has been updated, so we're dismissing all reviews.

@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode requested a review from Nuru November 6, 2021 14:10
Nuru
Nuru previously requested changes Nov 10, 2021
main.tf Outdated Show resolved Hide resolved
Co-authored-by: Nuru <Nuru@users.noreply.github.com>
@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode requested a review from Nuru November 10, 2021 21:58
@mergify mergify bot dismissed Nuru’s stale review November 10, 2021 21:59

This Pull Request has been updated, so we're dismissing all reviews.

@nitrocode
Copy link
Member Author

/test test/terratest

1 similar comment
@nitrocode
Copy link
Member Author

/test test/terratest

@nitrocode nitrocode merged commit 34b800d into master Nov 11, 2021
@nitrocode nitrocode deleted the ami_name_regex branch November 11, 2021 04:23
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

4 participants