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.

Sign up for GitHub

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

Jump to bottom

Allow to provide instance requirements options to ASG module #831

Closed
wants to merge 9 commits into from
Closed

Allow to provide instance requirements options to ASG module #831

wants to merge 9 commits into from

Conversation

stefanhorning
Copy link
Contributor

@stefanhorning stefanhorning commented Dec 10, 2021 •
edited
Loading

... to let AWS pick instance types based on machine specs.

SUMMARY

Just allows to pass through more arguments in the mixed_instances_policy section, namely the one for specifying instance requirements (instead of providing specific instance types).

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_asg

@stefanhorning stefanhorning changed the title Also allow to provide instance requirements to ASG module Allow to provide instance requirements options to ASG module Dec 10, 2021
@stefanhorning
Copy link
Contributor Author

stefanhorning commented Dec 13, 2021 •
edited
Loading

PR is ready to be merged now (from my side).

@markuman markuman mentioned this pull request Feb 1, 2022
Closed
1 task
@ansibullbot
Copy link

cc @garethr @jillr @markuman @s-hertel @tremble
click here for bot help

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request has_issue integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Feb 9, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@stefanhorning
Copy link
Contributor Author

Latest change is still green with recent CI. So I guess this can be merged now?

@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Feb 22, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

@azure-pipelines
Copy link

There was an error handling pipeline event 4d0d2b0a-3461-4c22-910a-93925224aff5.

@ansibullbot ansibullbot added community_review and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 28, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@stefanhorning
Copy link
Contributor Author

Looks like CI/bots got confused by the rebase?

@marknet15
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@mandar242
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@mandar242
Copy link
Contributor

mandar242 commented Apr 14, 2022 •
edited
Loading

Hi @stefanhorning, could you please rebase with main?
With recent merge of #1061, a bug causing env_cleanup in integration tests to fail is resolved.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@stefanhorning
Copy link
Contributor Author

stefanhorning commented Apr 14, 2022 •
edited
Loading

There is still two failures in the ASG tests (not related to my changes):

  • error in the Task TASK [ec2_asg : Create internet gateway for use in testing]: in file "/tmp/ansible_ec2_vpc_igw_payload_lgqsbh2i/ansible_ec2_vpc_igw_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_vpc_igw.py", line 162, in get_igw_info : TypeError: 'NoneType' object is not subscriptable First seen that today
  • botocore.exceptions.ClientError: An error occurred (AccessDenied) when calling the UpdateAutoScalingGroup operation: You are not authorized to use launch template: lt-0b27953098b749950 Somehow popped up yesterday afternoon and since stayed. despite me leaving reverting any change in laucn template setup...

@mandar242
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@alinabuzachis
Copy link
Contributor

alinabuzachis commented Apr 19, 2022 •
edited
Loading

  • botocore.exceptions.ClientError: An error occurred (AccessDenied) when calling the UpdateAutoScalingGroup operation: You are not authorized to use launch template: lt-0b27953098b749950 Somehow popped up yesterday afternoon and since stayed. despite me leaving reverting any change in laucn template setup...

For this one, you probably have to add the missing permissions against the https://github.com/mattclay/aws-terminator.

@alinabuzachis
Copy link
Contributor

  • error in the Task TASK [ec2_asg : Create internet gateway for use in testing]: in file "/tmp/ansible_ec2_vpc_igw_payload_lgqsbh2i/ansible_ec2_vpc_igw_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_vpc_igw.py", line 162, in get_igw_info : TypeError: 'NoneType' object is not subscriptable First seen that today

@jatorcasso was working on this one.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@stefanhorning
Copy link
Contributor Author

  • botocore.exceptions.ClientError: An error occurred (AccessDenied) when calling the UpdateAutoScalingGroup operation: You are not authorized to use launch template: lt-0b27953098b749950 Somehow popped up yesterday afternoon and since stayed. despite me leaving reverting any change in laucn template setup...

For this one, you probably have to add the missing permissions against the https://github.com/mattclay/aws-terminator.

@alinabuzachis I thought about this too. But figured that neither updating the ASG nor access to the launch template is s.th. I added to the tests. Just one more task to basically requiring the same permissions as all the others already in the ASG test suite.

But anyway just double-checked in https://github.com/mattclay/aws-terminator/blob/master/aws/policy/compute.yaml#L11 and the permission(s) already seem to be there. So I am a little lost, especially since this error popped up only after rebasing my branch. 🤔

@markuman
Copy link
Member

markuman commented Apr 20, 2022 •
edited
Loading

  • botocore.exceptions.ClientError: An error occurred (AccessDenied) when calling the UpdateAutoScalingGroup operation: You are not authorized to use launch template: lt-0b27953098b749950 Somehow popped up yesterday afternoon and since stayed. despite me leaving reverting any change in laucn template setup...

For this one, you probably have to add the missing permissions against the https://github.com/mattclay/aws-terminator.

@alinabuzachis I thought about this too. But figured that neither updating the ASG nor access to the launch template is s.th. I added to the tests. Just one more task to basically requiring the same permissions as all the others already in the ASG test suite.

But anyway just double-checked in https://github.com/mattclay/aws-terminator/blob/master/aws/policy/compute.yaml#L11 and the permission(s) already seem to be there. So I am a little lost, especially since this error popped up only after rebasing my branch. thinking

Maybe the error message is missleading

The integration test requires

       v_cpu_count: { min: 2, max: 8 }
       memory_mi_b: { min: 1024, max: 4096 }

but the terminator policies allows only

  • t3.nano (2 v cpus, ~512 memory)
  • t3.micro (2 v cpus, ~1024 memory)

a nitro micro instance got 943Mi after booting. So the ASG tries to request the LT with a large instance, what is not allowed.
maybe you can try to lower the memory min memory and see if it solves this issue.

@mandar242
Copy link
Contributor

Maybe the error message is missleading

The integration test requires

       v_cpu_count: { min: 2, max: 8 }
       memory_mi_b: { min: 1024, max: 4096 }

but the terminator policies allows only

  • t3.nano (2 v cpus, ~512 memory)
  • t3.micro (2 v cpus, ~1024 memory)

a nitro micro instance got 943Mi after booting. So the ASG tries to request the LT with a large instance, what is not allowed. maybe you can try to lower the memory min memory and see if it solves this issue.

I tried reducing the memory here, but ends up with same error.

@stefanhorning
Copy link
Contributor Author

Yes, I also doubted that this will change anything, as the terminator AWS policy states the following condition

  Condition:
      StringEqualsIfExists:
        ec2:InstanceType:
...

So it does an exact string match on the given instance type string (not on s.th. that will implicitly be chosen).
Just wondering if this condition is the problem at all, as we are not providing an instance type at all here and StringEqualsIfExists sounds like it would only match if such a param would exist.

@stefanhorning stefanhorning mentioned this pull request Oct 16, 2023
Open
1 task
@stefanhorning
Copy link
Contributor Author

opened a ticket for this in the project it now belongs to ansible-collections/amazon.aws#1807

Closing this PR for good as it won't be merged here anymore.

abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
@tremble
module_utils.batch remove unused AWSConnection (ansible-collections#831)
165a881 
module_utils.batch remove unused AWSConnection

SUMMARY
Remove deprecated and unused AWSConnection class from ansible_collections.amazon.aws.plugins.module_utils.batch
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/module_utils/batch.py
ADDITIONAL INFORMATION
See also ansible/ansible#67191

Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alinabuzachis alinabuzachis alinabuzachis left review comments

@mandar242 mandar242 mandar242 left review comments

@marknet15 marknet15 marknet15 left review comments

@markuman markuman markuman left review comments

@tremble tremble tremble left review comments

Assignees
No one assigned
Labels
community_review feature This issue/PR relates to a feature request has_issue integration tests/integration module module plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@stefanhorning @ansibullbot @marknet15 @mandar242 @alinabuzachis @markuman @tremble