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 fails if there is no default vpc and no vpc_subnet_id specified #467

Open
nerijus opened this issue Mar 29, 2021 · 16 comments
Open
Assignees
Labels
bug This issue/PR relates to a bug module module plugins plugin (any type) python3 traceback

Comments

@nerijus
Copy link
Contributor

nerijus commented Mar 29, 2021

SUMMARY

There is only one VPC in the region, but it is not default (Default VPC: No). I.e. there is no default VPC at all in this region.
I try to create ec2 instance by using:

- name: Create a new ec2 instance
  ec2_instance:
    name: "{{ route53_hostname }}"
    key_name: "{{ key_name }}"
    security_group: unhindered
    instance_type: "{{ size }}"
    image_id: 'ami-0d2f2...'
    region: "{{ region }}"
    availability_zone : "{{ availability_zone }}"
    volumes: "{{ volumes }}"

It fails with:

 File "/tmp/ansible_ec2_instance_payload_c9xfa938/ansible_ec2_instance_payload.zip/ansible_collections/community/aws/plugins/modules/ec2_instance.py", line 1409, in get_default_subnet
TypeError: 'NoneType' object is not subscriptable

The docs at https://docs.ansible.com/ansible/latest/collections/community/aws/ec2_instance_module.html#parameter-availability_zone say that availability zone is useful if not specifying the vpc_subnet_id parameter.
But it does not work in my case. When I specify vpc_subnet_id, it works.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

ec2_instance

ANSIBLE VERSION
ansible 2.10.6
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/nerijus/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.9.2 (default, Feb 20 2021, 00:00:00) [GCC 10.2.1 20201125 (Red Hat 10.2.1-9)]
@nerijus
Copy link
Contributor Author

nerijus commented Mar 29, 2021

It starts to work if I use the following patch:

--- ec2_instance.py.orig        2021-03-29 15:09:08.022478916 +0300
+++ ec2_instance.py     2021-03-29 17:11:49.212577095 +0300
@@ -1396,22 +1396,27 @@
 def get_default_vpc(ec2):
     vpcs = ec2.describe_vpcs(
         aws_retry=True,
         Filters=ansible_dict_to_boto3_filter_list({'isDefault': 'true'}))
     if len(vpcs.get('Vpcs', [])):
         return vpcs.get('Vpcs')[0]
+    vpcs = ec2.describe_vpcs(
+        aws_retry=True,
+        Filters=ansible_dict_to_boto3_filter_list({'isDefault': 'false'}))
+    if len(vpcs.get('Vpcs', [])):
+        return vpcs.get('Vpcs')[0]
     return None


 def get_default_subnet(ec2, vpc, availability_zone=None):
     subnets = ec2.describe_subnets(
         aws_retry=True,
         Filters=ansible_dict_to_boto3_filter_list({
             'vpc-id': vpc['VpcId'],
             'state': 'available',
-            'default-for-az': 'true',
+#            'default-for-az': 'true',
         })
     )
     if len(subnets.get('Subnets', [])):
         if availability_zone is not None:
             subs_by_az = dict((subnet['AvailabilityZone'], subnet) for subnet in subnets.get('Subnets'))
             if availability_zone in subs_by_az:

although not fully - it creates ec2 instance, but in default availability zone, not the one I specified.

@ansibullbot
Copy link

@nerijus: Greetings! Thanks for taking the time to open this issue. In order for the community to handle your issue effectively, we need a bit more information.

Here are the items we could not find in your description:

  • ansible version

Please set the description of this issue with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/ISSUE_TEMPLATE/bug_report.md

click here for bot help

@ansibullbot
Copy link

@alinabuzachis
Copy link
Collaborator

Hi @nerijus, thank you for raising this issue. I assume you are using a pre 2013 account, could you confirm? Thanks.

@nerijus
Copy link
Contributor Author

nerijus commented Apr 17, 2021

The oldest invoice is from December 3, 2012, so yes, it is pre 2013 account.

@jillr
Copy link
Collaborator

jillr commented May 3, 2021

Hi @nerijus. It sounds like your account might have been EC2 Classic originally but your suggestion does not appear to work on my EC2 Classic account. Can you confirm please if you're using VPCs in your account or not?

@jillr
Copy link
Collaborator

jillr commented May 3, 2021

I'm trying to reproduce an account setup that would have a default subnet but no default VPC to test with and I'm not seeing how that would work - create_default_subnet can only be called when a default VPC is present. When both default resources exist the module is able to filter on that default status for both VPC and subnet but if those are not present the sanest option will be to provide the subnet as a vpc_subnet_id (so that we can resolve the VPC with certainty).

If I've misunderstood your environment please let me know.

@nerijus
Copy link
Contributor Author

nerijus commented May 3, 2021

I would like to use availability_zone and not specify vpc_subnet_id. It is not possible now, contrary to what the docs say.
Does it work for you with no default VPC? I don't think it has something to do with classic account or not.

@nerijus
Copy link
Contributor Author

nerijus commented May 4, 2021

And yes, I am using VPCs, as I reported - "when I specify vpc_subnet_id, it works".

@tremble tremble transferred this issue from ansible-collections/community.aws Aug 18, 2021
@ansibullbot
Copy link

cc @ryansb
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module plugins plugin (any type) python3 traceback labels Aug 18, 2021
@nerijus
Copy link
Contributor Author

nerijus commented Aug 18, 2021

We have got emails from aws that EC2 Classic will be deprecated, and it said that we don't have classic resources in our regions.

goneri pushed a commit that referenced this issue Sep 20, 2022
* Move ec2_launch_template to standard role-type test

* Run elb_application_lb in our standard environment

* Move elb_target_info to the standard role structure

* add remote_tmp_dir dependency to ec2_launch_template

* move test_multiple_actions_fail.yml inside full_test so we don't need to run the prep work twice

* Rename the ALBs: "my-alb" isn't helpful and resource prefix is likely to change on us

* Ensure ALBs are deleted if "failed" tests created something

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@887d624
GomathiselviS pushed a commit to GomathiselviS/amazon.aws that referenced this issue Sep 20, 2022
…ns#467)

* Move ec2_launch_template to standard role-type test

* Run elb_application_lb in our standard environment

* Move elb_target_info to the standard role structure

* add remote_tmp_dir dependency to ec2_launch_template

* move test_multiple_actions_fail.yml inside full_test so we don't need to run the prep work twice

* Rename the ALBs: "my-alb" isn't helpful and resource prefix is likely to change on us

* Ensure ALBs are deleted if "failed" tests created something

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@887d624
@tremble
Copy link
Contributor

tremble commented Feb 6, 2023

The order of VPCs when calling describe_vpcs is not guaranteed. Picking what happens to be returned as the first is going to be non-deterministic in accounts with multiple VPCs.

If there's no default VPC and no subnet specified I think it's perfectly reasonable for the module to fail.

@nerijus
Copy link
Contributor Author

nerijus commented Feb 6, 2023

But I am talking about situation when "There is only one VPC in the region, but it is not default". So I assume it is safe to return first when there is only one VPC? And yes, if there are multiple VPCs, then it should fail as before.

@tremble
Copy link
Contributor

tremble commented Feb 6, 2023

Take the simple example that you add a new VPC to your account, suddenly all playbooks relying on that VPC being the only VPC fail. While what you're suggesting would work for your current, specific, use-case (a single VPC, with no VPC marked as default), in my opinion it's not a "good" solution because it adds fragile extra logic while only solving for a very specific edge-case.

Managing Security Groups and Instances is already very complex code (due to the massive array of options), adding special cases to try to guess what people want simply makes things unpredictable.

The specific case of falling back to the "default subnet" for a specified AZ in the "default VPC" is different because Amazon explicitly ensures it is the only VPC marked as "default" in the region and the only subnet marked as "default" in that availability zone. This in turn means that adding additional VPCs or additional subnets has no unexpected side effects. Additionally, while the AWS Web UI might automatically pick a VPC/subnet for you, it's showing you what it's picked and there's then an explicit confirmation step before you launch the resource.

There are already modules which support fetching the information you would need (ec2_vpc_subnet_info), I'd strongly recommend using those so you can be more explicit about the rules you want to follow for picking the subnets.

@nerijus
Copy link
Contributor Author

nerijus commented Feb 6, 2023

I am not sure it is so specific "edge" case, I think there are lots of accounts with one VPC which is not default (because AWS does not allow to mark an existing nondefault VPC as a default VPC). Yes, it will fail if I add another VPC, which I would expect. But I understand that it would add compexity, and if you do not plan to change it, please feel free to close the issue.

@azrdev
Copy link

azrdev commented Sep 26, 2023

If there's no default VPC and no subnet specified I think it's perfectly reasonable for the module to fail.

but it should emit a better error message than a backtrace and TypeError: 'NoneType' object is not subscriptable

abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Oct 24, 2023
…ns#467)

* Move ec2_launch_template to standard role-type test

* Run elb_application_lb in our standard environment

* Move elb_target_info to the standard role structure

* add remote_tmp_dir dependency to ec2_launch_template

* move test_multiple_actions_fail.yml inside full_test so we don't need to run the prep work twice

* Rename the ALBs: "my-alb" isn't helpful and resource prefix is likely to change on us

* Ensure ALBs are deleted if "failed" tests created something
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Oct 18, 2024
…ns#467)

* Move ec2_launch_template to standard role-type test

* Run elb_application_lb in our standard environment

* Move elb_target_info to the standard role structure

* add remote_tmp_dir dependency to ec2_launch_template

* move test_multiple_actions_fail.yml inside full_test so we don't need to run the prep work twice

* Rename the ALBs: "my-alb" isn't helpful and resource prefix is likely to change on us

* Ensure ALBs are deleted if "failed" tests created something

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@887d624
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug module module plugins plugin (any type) python3 traceback
Projects
None yet
Development

No branches or pull requests

6 participants