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

ec2_instance - Fix issue with launch template #2323

Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
minor_changes:
- ec2_instance - Fix the issue when trying to run instances using launch template in an AWS environment where no default subnet is defined(https://github.com/ansible-collections/amazon.aws/issues/2321).
46 changes: 29 additions & 17 deletions plugins/modules/ec2_instance.py
Original file line number Diff line number Diff line change
@@ -1567,9 +1567,9 @@ def build_network_spec(client, module: AnsibleAWSModule) -> List[Dict[str, Any]]
for inty in network_interfaces
]
)
elif not network and not network_interfaces_ids:
# They did not specify any network interface configuration
# build network interface using subnet_id and security group(s) defined on the module
elif not network and not network_interfaces_ids and not module.params.get("launch_template"):
Copy link
Member

Choose a reason for hiding this comment

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

I won't block the PR on this, but as far as I can tell, this is the only line that actually needs to be changed to fix the bug. I'd really prefer that any refactoring should happen in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Handling the refactoring in a separate PR makes sense. I'll leave the decision to @abikouo , as he is the original author of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gravesm the PR does not include any code refactoring, all changes are aimed at fixing the reported issue

# No network interface configuration specified and no launch template
# Build network interface using subnet_id and security group(s) defined in the module
interfaces.append(ansible_to_boto3_eni_specification(client, module, {}, vpc_subnet_id, groups))
elif network:
# handle list of `network.interfaces` options
@@ -1796,9 +1796,14 @@ def build_run_instance_spec(client, module: AnsibleAWSModule, current_count: int

# Validate network parameters
validate_network_params(module, nb_instances)

spec["NetworkInterfaces"] = build_network_spec(client, module)
spec["BlockDeviceMappings"] = build_volume_spec(params)
# Build network specs
network_specs = build_network_spec(client, module)
if network_specs:
spec["NetworkInterfaces"] = network_specs
# Build volume specs
volume_specs = build_volume_spec(params)
if volume_specs:
spec["BlockDeviceMappings"] = volume_specs

tag_spec = build_instance_tags(params)
if tag_spec is not None:
@@ -2635,17 +2640,18 @@ def build_network_filters(client, module: AnsibleAWSModule) -> Dict[str, List[st


def build_filters(client, module: AnsibleAWSModule) -> Dict[str, Any]:
filters = {
# all states except shutting-down and terminated
"instance-state-name": ["pending", "running", "stopping", "stopped"],
}
if isinstance(module.params.get("instance_ids"), string_types):
filters["instance-id"] = [module.params.get("instance_ids")]
elif isinstance(module.params.get("instance_ids"), list) and len(module.params.get("instance_ids")):
filters["instance-id"] = module.params.get("instance_ids")
# all states except shutting-down and terminated
instance_state_names = ["pending", "running", "stopping", "stopped"]
filters = {}
instance_ids = module.params.get("instance_ids")
if isinstance(instance_ids, string_types):
filters = {"instance-id": [instance_ids], "instance-state-name": instance_state_names}
elif isinstance(instance_ids, list) and len(instance_ids):
filters = {"instance-id": instance_ids, "instance-state-name": instance_state_names}
else:
# Network filters
filters.update(build_network_filters(client, module))
if not module.params.get("launch_template"):
# Network filters
filters.update(build_network_filters(client, module))
if module.params.get("name"):
filters["tag:Name"] = [module.params.get("name")]
elif module.params.get("tags"):
@@ -2657,6 +2663,10 @@ def build_filters(client, module: AnsibleAWSModule) -> Dict[str, Any]:
filters["image-id"] = [module.params.get("image_id")]
elif (module.params.get("image") or {}).get("id"):
filters["image-id"] = [module.params.get("image", {}).get("id")]

if filters:
# add the instance state filter if any filter key was provided
filters["instance-state-name"] = instance_state_names
return filters


@@ -2864,7 +2874,9 @@ def main():
filters = build_filters(client, module)

try:
existing_matches = find_instances(client, module, filters=filters)
existing_matches = []
if filters:
existing_matches = find_instances(client, module, filters=filters)

if state in ("terminated", "absent"):
if existing_matches:
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
time=5m
cloud/aws
ec2_instance
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
dependencies:
- role: setup_ec2_facts
- role: setup_ec2_instance_env
vars:
ec2_instance_test_name: launch-template
112 changes: 112 additions & 0 deletions tests/integration/targets/ec2_instance_launch_template/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
---
- module_defaults:
group/aws:
access_key: "{{ aws_access_key }}"
secret_key: "{{ aws_secret_key }}"
session_token: "{{ security_token | default(omit) }}"
region: "{{ aws_region }}"
block:
- name: Create a Launch template
community.aws.ec2_launch_template:
template_name: "{{ resource_prefix }}-template"
instance_type: t2.micro
image_id: "{{ ec2_ami_id }}"
block_device_mappings:
- device_name: /dev/sdb
ebs:
volume_size: 20
delete_on_termination: true
volume_type: standard
network_interfaces:
- device_index: 0
associate_public_ip_address: false
delete_on_termination: true
subnet_id: "{{ testing_subnet_a.subnet.id }}"
description: "A network interface from the testing subnet a"
register: _launch_template

- name: Create EC2 instance using launch template (check mode)
amazon.aws.ec2_instance:
state: present
wait: true
launch_template:
id: "{{ _launch_template.template.launch_template_id }}"
register: _create_check
check_mode: true

- name: Ensure module reported change while running in check mode
ansible.builtin.assert:
that:
- _create_check is changed
- '"instance_ids" not in _create_check'

- name: Create EC2 instance using launch template
amazon.aws.ec2_instance:
state: present
launch_template:
id: "{{ _launch_template.template.launch_template_id }}"
wait: true
register: _instance_a

- name: Set instances to delete
ansible.builtin.set_fact:
test_instance_ids: "{{ _instance_a.instance_ids | default([]) }}"

- name: Validate instance created as expected
ansible.builtin.assert:
that:
- _instance_a is changed
- '"instance_ids" in _instance_a'
- '"instances" in _instance_a'
- _instance_a.instances | length == 1
- _instance_a.instances[0].instance_type == 't2.micro'
- _instance_a.instances[0].image_id == ec2_ami_id
- _instance_a.instances[0].network_interfaces | length == 1
- _instance_a.instances[0].network_interfaces[0].subnet_id == testing_subnet_a.subnet.id
- _instance_a.instances[0].network_interfaces[0].description == "A network interface from the testing subnet a"
# AWS adds automatically a tag with the launch template id
- '"aws:ec2launchtemplate:id" in _instance_a.instances[0].tags'
- _instance_a.instances[0].tags["aws:ec2launchtemplate:id"] == _launch_template.template.launch_template_id

- name: Create antoher EC2 instance using launch template and some override parameters
amazon.aws.ec2_instance:
state: present
launch_template:
id: "{{ _launch_template.template.launch_template_id }}"
instance_type: t3.nano
network_interfaces:
- device_index: 0
assign_public_ip: false
delete_on_termination: true
subnet_id: "{{ testing_subnet_b.subnet.id }}"
description: "A network interface from the testing subnet b"
wait: true
register: _instance_b

- name: Set instances to delete
ansible.builtin.set_fact:
test_instance_ids: "{{ test_instance_ids + _instance_b.instance_ids | default([])}}"

- name: Validate instance created as expected
ansible.builtin.assert:
that:
- _instance_b is changed
- '"instance_ids" in _instance_b'
- _instance_a.instance_ids != _instance_b.instance_ids
- _instance_b.instances | length == 1
- _instance_b.instances[0].instance_type == 't3.nano'
- _instance_b.instances[0].image_id == ec2_ami_id
- _instance_b.instances[0].network_interfaces | length == 1
- _instance_b.instances[0].network_interfaces[0].subnet_id == testing_subnet_b.subnet.id
- _instance_b.instances[0].network_interfaces[0].description == "A network interface from the testing subnet b"
# AWS adds automatically a tag with the launch template id
- '"aws:ec2launchtemplate:id" in _instance_b.instances[0].tags'
- _instance_b.instances[0].tags["aws:ec2launchtemplate:id"] == _launch_template.template.launch_template_id

always:
- name: Delete instances
amazon.aws.ec2_instance:
state: absent
instance_ids: "{{ test_instance_ids }}"
wait: true
when: test_instance_ids is defined