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
New module: Add AWS Batch automation support (cloud/amazon/batch_compute_environment) #24402
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code is so tidy. :-) I'll try to run it later this week and review it.
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with Ansible. If not, see <http://www.gnu.org/licenses/>. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a docs review so you pass shippable till I can check out and your changes and look at it in-depth.... Applies to the other two files too.
Up here you'll want the metadata:
ANSIBLE_METADATA = {'metadata_version': '1.0',
'status': ['preview'],
'supported_by': 'community'}
from botocore.exceptions import ClientError, ParamValidationError, MissingParametersError | ||
HAS_BOTO3 = True | ||
except ImportError: | ||
HAS_BOTO3 = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything from line 17 (import json) to this line will need to be below the RETURN section.
It is idempotent and supports "Check" mode. Use module M(batch_compute_environment) to manage the compute | ||
environment, M(batch_job_queue) to manage job queues, M(batch_job_definition) to manage job definitions. | ||
|
||
version_added: "2.4?" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the ?
- 2.4 is likely, and if not that can be updated :)
description: | ||
- The desired number of EC2 vCPUS in the compute environment. | ||
|
||
instance_types: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to specify 'type: str/list/bool/dict' for some of these.
description: | ||
- The Amazon Machine Image (AMI) ID used for instances launched in the compute environment. | ||
|
||
subnets: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type: list ?
- The VPC subnets into which the compute resources are launched. | ||
required: true | ||
|
||
security_group_ids: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type: list ?
- The Amazon ECS instance role applied to Amazon EC2 instances in a compute environment. | ||
required: true | ||
|
||
tags: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type: dict ?
The test
The test
The test
The test
The test
The test
|
@s-hertel I can take care of the PEP8 errors and try to reproduce these errors locally but I'm not sure why I have the documentation errors with the command |
The test
The test
The test
The test
The test
|
@jonmer85 Sorry about the documentation issues. I'm not sure what's wrong at a cursory look. I'll check out your changes as soon as I can and try to figure out what's wrong. |
The test
The test
The test
The test
The test
|
Thanks @s-hertel. I got rid of many of the PEP errors. If I put my ansible import statements at the top of the file, my automation fails. I can look into it more though to determine why. Please let me know if you found out why the documentation sanity checks fail. Thank you! |
volumes: | ||
description: | ||
- A list of data volumes used in a job. List of dictionaries with the following | ||
form: { host: { sourcePath: <string> }, name: <string> } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line isn't valid yaml because of the ':'s. That's why the docs are failing. You can use suboptions:
to document options inside an option. Make sure to document type/description/required just like for options
.
|
||
# ansible import module(s) kept at ~eof as recommended | ||
from ansible.module_utils.basic import * | ||
from ansible.module_utils.ec2 import * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... I'm not sure why it fails for you if these are moved up to the top. By 'top' do you mean around line 186? That is around where they should go. And
in addition, importing * is avoided now so things don't 'magically' work. Also you'll want to import HAS_BOTO3 rather than set it on line 182 or line 184.
So you'll want something like from ansible.module_utils.ec2 import ec2_argument_spec, get_aws_connection_info, boto3_conn, HAS_BOTO3
.
The test
The test
The test
The test
The test
|
The test
The test
The test
The test
The test
|
@s-hertel It took a bit but it looks like all tests pass now :) |
@Etherdaemon @akazakov @alachaum @amir343 @bekelchik @bpennypacker @brandond @fiunchinho @j-carl @jarv @jmenga @joelthompson @jsdalton @linuxdynasty @loia @MichaelBaydoun @michaeljs1990 @minichate @mjschultz @mmochan @naslanidis @pjodouin @pwnall @RickMendes @ryansydnor @scottanderson42 @shepdelacreme @silviud @simplesteph @steynovich @tastychutney @tedder @timmahoney @TomBamford @whiter @wimnat @Zeekin As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
@s-hertel How do I vote with a "shipit"? |
@jonmer85 Sorry for not getting back to this sooner - it's still on my radar to review. If you type that without word without quotes the bot should find it (but you'll need to be added to maintainers first, I think). Two votes leads to a bot-automated merge (though there are caveats). Here's bot command info: https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md |
@jonmer85 this PR contains more than one new module. Please submit only one new module per pullrequest. For further explanation, please read grouped module documentation |
@s-hertel I created three new PR's for this work as requested: |
@jonmer85 Thanks! I'm sorry for the hassle. |
Closing this as it's now broken up into:
|
SUMMARY
This adds automation support for the following AWS Batch features - Compute Environments, Job Definitions, and Job Queues
ISSUE TYPE
COMPONENT NAME
amazon module
ANSIBLE VERSION
ansible 2.2.2.0
config file =
configured module search path = Default w/o overrides