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 launch_template condition #111

Closed
wants to merge 1 commit into from

Conversation

Akasurde
Copy link
Member

SUMMARY

Fixes: ansible/ansible#51941

Signed-off-by: Abhijeet Kasurde [email protected]

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

changelogs/fragments/51941_ec2_instance.yml
plugins/modules/ec2_instance.py

@jillr jillr changed the base branch from master to main July 2, 2020 19:48
Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

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

Thanks for this patch @Akasurde, would you mind adding some integration tests to cover the launch_template option for the name and id suboptions?

@ansibullbot
Copy link

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug community_review has_issue module module needs_triage small_patch Hopefully easy to review stale_ci CI is older than 7 days, rerun before merging labels Aug 19, 2020
@ansibullbot ansibullbot added the plugins plugin (any type) label Aug 27, 2020
@goneri
Copy link
Member

goneri commented Feb 3, 2021

Hi @Akasurde, is this something you are still working on?

Copy link
Member

@markuman markuman left a comment

Choose a reason for hiding this comment

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

LGTM

@tremble
Copy link
Contributor

tremble commented Feb 8, 2021

Needs a changelog and some integration tests

@markuman
Copy link
Member

markuman commented Feb 8, 2021

@Akasurde @jillr @goneri

I hit the same error which is fixed in this PR s/or/and not/.

However, that's not sufficient to get it work.

First, it looks like you cannot overwrite launch template parameters using boto3. And while ec2_instance is setting several default values (instance type, network interface, tag specification and block device mapping), you will always get the error message
"Failed to create new EC2 instance: An error occurred (Unsupported) when calling the RunInstances operation: The requested configuration is currently not supported. Please check the documentation for supported configurations.".

For example:
origin module is passing

{'BlockDeviceMappings': [], 'ClientToken': '37d405af374948e2bd427ff4a7a7fc04', 'InstanceType': 't2.micro', 'LaunchTemplate': {'LaunchTemplateName': 'ubuntu-20.04'}, 'MaxCount': 1, 'MinCount': 1, 'NetworkInterfaces': [{'DeviceIndex': 0, 'SubnetId': 'subnet-3e5fe743'}], 'TagSpecifications': [{'ResourceType': 'volume', 'Tags': [{'Key': 'Name', 'Value': 'lttest01'}]}, {'ResourceType': 'instance', 'Tags': [{'Key': 'Name', 'Value': 'lttest01'}]}]}

to run instance on playbook

    - name: test launchtemplate usage
      ec2_instance:
        wait: yes
        name: "{{ name }}"
        launch_template:
          name: ubuntu-20.04
        vpc_subnet_id: subnet-3e5fe743

With that patch, is runs smoothly

@@ -1273,16 +1262,17 @@ def build_run_instance_spec(params, ec2=None):
         MinCount=1,
     )
     # network parameters
-    spec['NetworkInterfaces'] = build_network_spec(params, ec2)
-    spec['BlockDeviceMappings'] = build_volume_spec(params)
-    spec.update(**build_top_level_options(params))
-    spec['TagSpecifications'] = build_instance_tags(params)
+    if not params.get('launch_template'):
+        spec['NetworkInterfaces'] = build_network_spec(params, ec2)
+        spec['BlockDeviceMappings'] = build_volume_spec(params)
+        spec['TagSpecifications'] = build_instance_tags(params)
+        # IAM profile
+        if params.get('instance_role'):
+            spec['IamInstanceProfile'] = dict(Arn=determine_iam_role(params.get('instance_role')))
+        spec['InstanceType'] = params['instance_type']
 
-    # IAM profile
-    if params.get('instance_role'):
-        spec['IamInstanceProfile'] = dict(Arn=determine_iam_role(params.get('instance_role')))
+    spec.update(**build_top_level_options(params))
 
-    spec['InstanceType'] = params['instance_type']
     return spec

But changes single parameter from the launch template is a necessary feature. The way to achieve it, is to describe the requested launch template and pass all parameters directly to run_instance and not request the launch template itself in run_instance().
That's a great task :)

@tremble
Copy link
Contributor

tremble commented Mar 29, 2021

@markuman Since you've already done some work on this, do you think you could create a fresh PR that we can get merged (changelog / integration tests / + bugfix) ?

@markuman
Copy link
Member

I'm closing this PR because it's not active anymore and the module has moved to amazon.aws collection.
But I've created an Issue there, imo with all related informations and links ansible-collections/amazon.aws#451

@markuman markuman closed this Aug 10, 2021
ansible-zuul bot pushed a commit to ansible-collections/amazon.aws that referenced this pull request Jan 25, 2022
… default value for instance_type (#587)

ec2_instance: Fix launch template condition, handle launch template - default value for instance_type

SUMMARY

The launch_template option in ec2_instance has a broken condition.
Also the launch_template option defaults the instance_type to t2.micro if not specified and ignores the instance_type specified in the launch_template as said in the issue #451.

Fixes

#451
#462

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

ec2_instance
ADDITIONAL INFORMATION

The change does not break existing functionality as tested by the integration test run locally.
Related to the condition fix in community.aws: ansible-collections/community.aws#111


Ran the following playbook to verify the change.
# create a launch template called "test-launch-template" 
    - name: create test launch template
      community.aws.ec2_launch_template:
        template_name: test-launch-template
        image_id: ami-002068ed284xxxxxx
        instance_type: t3a.small
        network_interfaces:
          - associate_public_ip_address: no
            delete_on_termination: yes
            device_index: 0
            groups:
              - sg-xxxxxxxxxxxxxxxxxx
            subnet_id: subnet-xxxxxxxxxxxxxxxxxx
        region: us-east-2
        block_device_mappings:
          - device_name: /dev/sdb
            ebs:
              volume_size: 5
              volume_type: gp3
              delete_on_termination: true
              encrypted: yes
          - device_name: /dev/sdc
            ebs:
              volume_size: 2
              volume_type: gp2
              delete_on_termination: true
              encrypted: no
        tags:
          ssome: tags

# launch a ec2 instance using launch template created earlier - launches t3a.small instance as expected
    - name: test launch template usage
      ec2_instance:
        wait: yes
        name: "test-instance-mk-t3a.small"
        launch_template:
          name: test-launch-template
        vpc_subnet_id: subnet-xxxxxxxxxxxxxxxxxx

# launch ec2 instance using launch template created earlier - override instance type to be launch to t3.xlarge
    - name: test launch template usage - override instance type
      ec2_instance:
        wait: yes
        name: "test-instance-mk-t3.xlarge"
        instance_type: t3.xlarge
        launch_template:
          name: test-launch-template
        vpc_subnet_id: subnet-xxxxxxxxxxxxxxxxxx

Reviewed-by: Jill R <None>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Emanuele Leopardi <None>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 bug This issue/PR relates to a bug community_review has_issue module module needs_changelog needs_tests plugins plugin (any type) small_patch Hopefully easy to review stale_ci CI is older than 7 days, rerun before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use launch_template name in ec2_instance module
6 participants