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

Added modules to manage ec2 autoscaling groups #6233

Merged
merged 3 commits into from Mar 11, 2014

Conversation

garethr
Copy link
Contributor

@garethr garethr commented Mar 1, 2014

Includes management of Launch Configuration and related Autoscaling
Groups.

Based on a complete working example here: https://github.com/garethr/ansible-provisioner-aws

Includes management of Launch Configuration and related Autoscaling
Groups
@mpdehaan
Copy link
Contributor

mpdehaan commented Mar 1, 2014

Excellent, glad to see this. We're going to do a push to merge a LOT of the cloud stuff in a little over 1-2 weeks. This will be a great addition.

@garethr
Copy link
Contributor Author

garethr commented Mar 1, 2014

Excellent. Any feedback/changes/etc. just let me know

- register or deregister the instance
required: true
choices: ['present', 'absent']
group:
Copy link
Contributor

Choose a reason for hiding this comment

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

group does not match group_name on line 78, and does not match name on line 111 and 137.

My preference is to change all of them to simple name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've corrected the documentation and the example. I agree, name provides a better interface.

I've left the internal variable name as group_name however as that maps to the actual AWS name. Making this more explicit if you're in the code feels useful to me.

@pas256
Copy link
Contributor

pas256 commented Mar 2, 2014

I would like to see this support 2 more things

Otherwise, great work.

print "failed=True msg='boto required for this module'"
sys.exit(1)

AWS_REGIONS = ['ap-northeast-1',
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to define AWS_REGIONS in modules that include module_utils/ec2.py

@willthames
Copy link
Contributor

Would be good to handle existing launchconfigurations in the create_autoscaling_groups (same as how you check that it exists before you delete it) rather than catch it in the same exception that will handle connection/authentication failures.

The more information a module returns, the better, in general.

@timperrett
Copy link
Contributor

Yes!! Love to see this merged!

mpdehaan added a commit that referenced this pull request Mar 11, 2014
Added modules to manage ec2 autoscaling groups
@mpdehaan mpdehaan merged commit 247d688 into ansible:devel Mar 11, 2014
@mpdehaan
Copy link
Contributor

Merged, thank you! We would be also welcome to entertain future upgrades if folks have suggestions/additions.

@willthames
Copy link
Contributor

Further improvements in #6419.

@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cloud feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants