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

Add mixed instance policy support #32

Merged
merged 4 commits into from
Feb 19, 2020

Conversation

bnutt
Copy link
Contributor

@bnutt bnutt commented Feb 13, 2020

cloudposse/terraform-aws-ec2-autoscale-group#19 adds support for the ability to specify different instances types and spot/on demand configurations for a launch template. This extends this functionality to eks nodes

cloudposse/terraform-aws-ec2-autoscale-group#19 adds support for the ability to specify different instances  types and spot/on demand configurations for a launch template. This extends this functionality to eks nodes
@bnutt bnutt requested a review from aknysh February 13, 2020 21:15
@aknysh aknysh requested a review from maximmi February 13, 2020 21:18
@maximmi
Copy link
Contributor

maximmi commented Feb 15, 2020

/codefresh run test

@bnutt
Copy link
Contributor Author

bnutt commented Feb 16, 2020

Any thoughts here @maximmi ? It looks like upstream module for asg's mixed_instances_policy is defined as a variable but it looks like terraform is unable to pick it up in codefresh, also experiencing the same on my local machine.

Error: Unsupported argument

  on main.tf line 208, in module "autoscale_group":
 208:   mixed_instances_policy                  = var.mixed_instances_policy

An argument named "mixed_instances_policy" is not expected here.

@bnutt
Copy link
Contributor Author

bnutt commented Feb 16, 2020

Edit, I just saw that there were two tags published and I was using the outdated one. It was able to resolve mixed_instance_policy after I bumped it.

@bnutt
Copy link
Contributor Author

bnutt commented Feb 18, 2020

Ran:

make init
make readme/deps
make readme

Didn't see any deltas unless there is something else I need to add. Could you take a look again @maximmi ? Thanks!

@maximmi
Copy link
Contributor

maximmi commented Feb 19, 2020

/codefresh run test

@maximmi
Copy link
Contributor

maximmi commented Feb 19, 2020

/codefresh run test

Copy link
Contributor

@maximmi maximmi left a comment

Choose a reason for hiding this comment

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

thanks @bnutt , you've missed quotes around variable name, but I've fixed it

@maximmi maximmi merged commit 7cfd7c1 into cloudposse:master Feb 19, 2020
@bnutt
Copy link
Contributor Author

bnutt commented Feb 19, 2020

ahh, thanks @maximmi , explains why the doc didn't generate it. i've added it locally to test and can see that it properly generated the docs. thanks for the change/review!

@maximmi
Copy link
Contributor

maximmi commented Feb 19, 2020

@bnutt thanks for your contribution! please, check for recent release: https://github.com/cloudposse/terraform-aws-eks-workers/releases/tag/0.13.0

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.

2 participants