amazon.aws icon indicating copy to clipboard operation
amazon.aws copied to clipboard

ec2_instance fails if there is no default vpc and no vpc_subnet_id specified

Open nerijus opened this issue 3 years ago • 16 comments

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 avatar Mar 29 '21 14:03 nerijus

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.

nerijus avatar Mar 29 '21 14:03 nerijus

@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 avatar Mar 29 '21 14:03 ansibullbot

cc @Shaps @jillr @s-hertel @tremble @wimnat click here for bot help

ansibullbot avatar Mar 29 '21 14:03 ansibullbot

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

alinabuzachis avatar Apr 16 '21 19:04 alinabuzachis

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

nerijus avatar Apr 17 '21 12:04 nerijus

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 avatar May 03 '21 18:05 jillr

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.

jillr avatar May 03 '21 19:05 jillr

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 avatar May 03 '21 20:05 nerijus

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

nerijus avatar May 04 '21 09:05 nerijus

cc @ryansb click here for bot help

ansibullbot avatar Aug 18 '21 09:08 ansibullbot

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.

nerijus avatar Aug 18 '21 10:08 nerijus

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.

tremble avatar Feb 06 '23 08:02 tremble

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.

nerijus avatar Feb 06 '23 10:02 nerijus

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.

tremble avatar Feb 06 '23 11:02 tremble

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.

nerijus avatar Feb 06 '23 12:02 nerijus

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

azrdev avatar Sep 26 '23 14:09 azrdev