RDS: Add DBSubnet resource, VPC security groups #13

RaitoBezarius · 2019-10-30T14:00:32Z

This commit enables EC2 RDS resource to use:

DBSubnets
VPC security groups

As a result, it adds a DBSubnet resource which consists of at least 2
VPC subnets containing at least each AZ of your region.

It enables support of RDS in a VPC.

This makes it possible to use RDS in a VPC setup, i.e. in EC2-VPC.

Will fix #6.

This commit enables EC2 RDS resource to use: - DBSubnets - VPC security groups As a result, it adds a DBSubnet resource which consists of at least 2 VPC subnets containing at least each AZ of your region. It enables support of RDS in a VPC. This makes it possible to use RDS in a VPC setup, i.e. in EC2-VPC.

AmineChikhaoui · 2019-10-30T14:33:39Z

Hi @RaitoBezarius , would you mind adding a working example under examples/ ?

RaitoBezarius · 2019-10-30T14:34:23Z

@AmineChikhaoui Sure, will do in some minutes.

RaitoBezarius · 2019-10-30T19:15:18Z

It has been 300 minutes but it's done :) @AmineChikhaoui

AmineChikhaoui · 2019-10-31T13:42:33Z

No worries @RaitoBezarius , thanks !
I'll try to play with it later this evening.

PsyanticY · 2019-10-31T17:25:14Z

nix/ec2-rds-subnet-group.nix

+    region = mkOption {
+      type = types.str;
+      description = "Amazon RDS DB subnet group region.";
+    };
+
+    accessKeyId = mkOption {
+      default = "";
+      type = types.str;
+      description = "The AWS Access Key ID.";
+    };


u can use imports = [ ./common-ec2-auth-options.nix ]; for these options like we do in the other modules for example

PsyanticY · 2019-10-31T17:25:21Z

nix/ec2-rds-subnet-group.nix

+      default = [];
+      type = types.listOf (types.either types.str (resource "vpc-subnet"));
+      apply = map (x: if builtins.isString x then x else "res-" + x._name + "." + x._type + ".subnetId");
+      description = "List of VPC subnets to use for this DB subnet group (must contain at least a subnet for each AZ), it can be a VPC Subnet resource or a string representing the ID of the VPC Subnet.";


break into multiple lines.

PsyanticY · 2019-11-01T16:08:30Z

Update this line to if dbinstance.status not in {'creating', 'backing-up', 'available', 'modifying', 'configuring-enhanced-monitoring'}:

PsyanticY · 2019-11-01T16:27:13Z

examples/ec2-rds-with-vpc.nix

+        engine = "postgres";
+        dbName = "testNixOps";
+        multiAZ = true;
+        vpcSecurityGroups = [ resources.ec2SecurityGroups.database ];


should be vpcSecurityGroupIds

I also needed to add securityGroups = []; for it to work.
@AmineChikhaoui what about removing support for ec2 classic altogether ?

RaitoBezarius · 2019-11-01T16:28:55Z

Update this line to if dbinstance.status not in {'creating', 'backing-up', 'available', 'modifying', 'configuring-enhanced-monitoring'}:

Why this change?

PsyanticY · 2019-11-01T16:30:06Z

nix/ec2-rds-dbinstance.nix

+      type = types.nullOr (types.either types.str (resource "ec2-rds-db-subnet-group"));
+      apply = group: if (builtins.isString group || group == null) then group else "res-" + group._name;
+      description = ''
+        A name for a DBSubnetGroup, they must contain at least one subnet in each availability zone in the AWS region.


multi lines

PsyanticY · 2019-11-01T16:53:33Z

Thanks @RaitoBezarius

Tested deploying the example after some minor changes. and it worked fine. We have an error in destroying the example since nixops do not wait for the rds to finalize termination and fail when destroying the security group cause it is still attached to the delete in progress rds.

[nix-shell:~/rds/nixops]$ nixops destroy -d rds-test
warning: are you sure you want to destroy RDS instance ‘test-multi-az’? (y/N) y
test-rds-instance> deleting RDS instance `test-multi-az'...
test-rds-instance> saving final snapshot as test-multi-az-final-snapshot-535c8be39f404c32aa949bb5de2ea1e1
db-subnet........> destroying rds db subnet group nixops-c6b58b14-fcba-11e9-b33e-0ae168952ac0-db-subnet
database.........> deleting EC2 security group `charon-c6b58b14-fcba-11e9-b33e-0ae168952ac0-database' ID `sg-05e1714ce3a1603e7'...
error: Multiple exceptions (2): 
  * database: EC2ResponseError: 400 Bad Request
<?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>DependencyViolation</Code><Message>resource sg-05e1714ce3a1603e7 has a dependent object</Message></Error></Errors><RequestID>1f56d3bf-0b36-4064-bf5a-eb5215bcc515</RequestID></Response>
  * db-subnet: An error occurred (InvalidDBSubnetGroupStateFault) when calling the DeleteDBSubnetGroup operation: Cannot delete the subnet group 'nixops-c6b58b14-fcba-11e9-b33e-0ae168952ac0-db-subnet' because at least one database instance: test-multi-az is still using it.

We should adress this. also I think we need to add the backup functionality to RDS module as well as remove automatically taking final snapshot (make a prompt or an argument).
@AmineChikhaoui

RaitoBezarius · 2019-11-01T16:54:55Z

Tested deploying the example after some minor changes. and it worked fine. We have an error in destroying the example since nixops do not wait for the rds to finalize termination and fail when destroying the security group cause it is still attached to the delete in progress rds.

[nix-shell:~/rds/nixops]$ nixops destroy -d rds-test
warning: are you sure you want to destroy RDS instance ‘test-multi-az’? (y/N) y
test-rds-instance> deleting RDS instance `test-multi-az'...
test-rds-instance> saving final snapshot as test-multi-az-final-snapshot-535c8be39f404c32aa949bb5de2ea1e1
db-subnet........> destroying rds db subnet group nixops-c6b58b14-fcba-11e9-b33e-0ae168952ac0-db-subnet
database.........> deleting EC2 security group `charon-c6b58b14-fcba-11e9-b33e-0ae168952ac0-database' ID `sg-05e1714ce3a1603e7'...
error: Multiple exceptions (2): 
  * database: EC2ResponseError: 400 Bad Request
<?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>DependencyViolation</Code><Message>resource sg-05e1714ce3a1603e7 has a dependent object</Message></Error></Errors><RequestID>1f56d3bf-0b36-4064-bf5a-eb5215bcc515</RequestID></Response>
  * db-subnet: An error occurred (InvalidDBSubnetGroupStateFault) when calling the DeleteDBSubnetGroup operation: Cannot delete the subnet group 'nixops-c6b58b14-fcba-11e9-b33e-0ae168952ac0-db-subnet' because at least one database instance: test-multi-az is still using it.

We should adress this. also I think we need to add the backup functionality to RDS module as well as remove automatically taking final snapshot (make a prompt or an argument).
@AmineChikhaoui

FYI, I noticed this issue and opened #11 for it, +1 for the final snapshot, I think it's a sane default to have though.

AmineChikhaoui · 2019-11-01T19:05:22Z

nix/ec2-rds-subnet-group.nix

+      ''; 
+    };
+
+    imports = [ ./common-ec2-options.nix ];


This addition wasn't tested, can you please make sure that it works ?
It doesn't evaluate as ./common-ec2-options.nix is a function so please make it's merged to options

options = { /* options declarations */ } // (import ./common-ec2-options.nix { inherit lib; });

Also if I fix that locally the diff engine will throw an error as it doesn't find a handler that realizes the tags diff, so it needs to be fixed in the python side to create a handler for the tags option.

AmineChikhaoui · 2019-11-01T19:06:59Z

examples/ec2-rds-with-vpc.nix

+  {
+    network.description = "NixOps RDS in a VPC Testing";
+  # A VPC.
+  resources.vpc.private = {


Can you also fix the indentation in this file please and remove any comments such as # A VPC which don't seem to be useful.

AmineChikhaoui · 2019-11-01T19:08:45Z

nixopsaws/resources/ec2_rds_dbinstance.py

+                or isinstance(r, nixopsaws.resources.ec2_rds_subnet_group.EC2RDSSubnetGroupState)
+                or isinstance(r, nixopsaws.resources.ec2_security_group.EC2SecurityGroupState)}
+
+    def destroy_before(self, resources):


Do we really need destroy_before ? afaik it's automatically handled by doing the opposite of create_after

I didn't know this, as I was exploring the solution for #11

RaitoBezarius · 2019-11-01T19:16:39Z

@AmineChikhaoui Changes were made.

AmineChikhaoui · 2019-11-01T19:22:36Z

Thank you for the quick changes but please test before asking for review again :)

RaitoBezarius · 2019-11-01T20:03:51Z

@AmineChikhaoui Now it is tested.

AmineChikhaoui · 2019-11-04T14:20:07Z

@RaitoBezarius I think there might be some minor changes to be done to make sure that when not using VPC security groups/DB subnet the config works as well.
I get this with the old example of RDS:

[nix-shell:~/src/nixops]$ nixops deploy -d rds-legacy
Traceback (most recent call last):
  File "/home/amine/src/nixops/scripts/nixops", line 250, in <module>
    args.op(args)
  File "/home/amine/src/nixops/nixops/script_defs.py", line 427, in op_deploy
    max_concurrent_activate=args.max_concurrent_activate)
  File "/home/amine/src/nixops/nixops/deployment.py", line 1062, in deploy
    self.run_with_notify('deploy', lambda: self._deploy(**kwargs))
  File "/home/amine/src/nixops/nixops/deployment.py", line 1051, in run_with_notify
    f()
  File "/home/amine/src/nixops/nixops/deployment.py", line 1062, in <lambda>
    self.run_with_notify('deploy', lambda: self._deploy(**kwargs))
  File "/home/amine/src/nixops/nixops/deployment.py", line 915, in _deploy
    self.evaluate_active(include, exclude, kill_obsolete)
  File "/home/amine/src/nixops/nixops/deployment.py", line 877, in evaluate_active
    self.evaluate()
  File "/home/amine/src/nixops/nixops/deployment.py", line 383, in evaluate
    defn = _create_definition(y, config["resources"][res_type][name], res_type)
  File "/home/amine/src/nixops/nixops/deployment.py", line 1265, in _create_definition
    return cls(xml)
  File "/nix/store/nihlv8ac909rw3lca1jdhalx3j72sw1x-nixops-aws-1.7pre0_abcdef/lib/python2.7/site-packages/nixopsaws/resources/ec2_rds_dbinstance.py", line 49, in __init__
    self.rds_dbinstance_db_subnet_group = xml.find("attrs/attr[@name='dbSubnetGroup']/string").get("value")
AttributeError: 'NoneType' object has no attribute 'get'

…t group is available in XML description

RaitoBezarius · 2019-11-10T15:44:40Z

@AmineChikhaoui I made the change but I'm unable to test it, I would require an ol'EC2 Classic account, those are not available anymore as AWS defaults to EC2-VPC nowadays AFAIK.

I tried to run it but AWS throws me an error saying: botocore.exceptions.ClientError: An error occurred (InvalidParameterValue) when calling the AuthorizeDBSecurityGroupIngress operation: VPC DB Security Groups cannot be modified with this API version. Please use an API version between 2012-01-15 and 2012-10-31 to modify this group.

But according to this: hashicorp/terraform#486 it's expected for people having an EC2 VPC account (like myself).

Let me know your thoughts on this.

NickSeagull · 2019-11-14T17:30:45Z

Any progress on this?

RaitoBezarius · 2019-11-14T17:53:05Z

@NickSeagull AFAIK, I'm waiting for @AmineChikhaoui to give it a try again and see what should we do regarding the EC2-Classic stuff that I cannot test.

AmineChikhaoui · 2019-11-14T18:07:43Z

Last time I tested on ec2-classic there was still some problems, I'll try to post the stacktrace next time but I'm not sure how helpful that would be unless I'm going to do the fixes as you can't test it.

RaitoBezarius · 2019-11-14T18:19:18Z

@AmineChikhaoui It might help us to move forward, I wonder whether I can get my hand on an EC2 Classic account, I'll try to ask AWS support staff.

RaitoBezarius · 2020-09-10T16:44:05Z

Closing because of #113

RaitoBezarius force-pushed the raito/rds-for-vpc branch from 8362273 to 4102176 Compare October 30, 2019 19:16

example: Add RDS with VPC

4102176

PsyanticY reviewed Oct 31, 2019

View reviewed changes

PsyanticY reviewed Nov 1, 2019

View reviewed changes

RaitoBezarius added 2 commits November 1, 2019 17:31

PR: Address review

bb99cb9

PR: Add a RDS non-error state as per PsyanticY's remark

0d34b02

AmineChikhaoui requested changes Nov 1, 2019

View reviewed changes

AmineChikhaoui reviewed Nov 1, 2019

View reviewed changes

RaitoBezarius added 3 commits November 1, 2019 20:14

PR: Fix import of common EC2 auth options & reindent the example

ce79966

PR: Rewrite example's comments

648249f

PR: Remove destroy_before hook

9b2de6f

PR: Post-test fixes

2063833

Fix: default to [] for vpcSecurityGroupIds and do not assume db subne…

7f112b7

…t group is available in XML description

grahamc mentioned this pull request Sep 9, 2020

RDS: Support VPC #113

Merged

RaitoBezarius closed this Sep 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RDS: Add DBSubnet resource, VPC security groups #13

RDS: Add DBSubnet resource, VPC security groups #13

RaitoBezarius commented Oct 30, 2019 •

edited

AmineChikhaoui commented Oct 30, 2019

RaitoBezarius commented Oct 30, 2019

RaitoBezarius commented Oct 30, 2019

AmineChikhaoui commented Oct 31, 2019

PsyanticY Oct 31, 2019

RaitoBezarius Nov 1, 2019

PsyanticY Oct 31, 2019

RaitoBezarius Nov 1, 2019

PsyanticY commented Nov 1, 2019

PsyanticY Nov 1, 2019 •

edited

RaitoBezarius Nov 1, 2019

RaitoBezarius commented Nov 1, 2019

PsyanticY Nov 1, 2019

PsyanticY commented Nov 1, 2019 •

edited

RaitoBezarius commented Nov 1, 2019

AmineChikhaoui Nov 1, 2019

AmineChikhaoui Nov 1, 2019

AmineChikhaoui Nov 1, 2019

RaitoBezarius Nov 1, 2019

RaitoBezarius commented Nov 1, 2019

AmineChikhaoui commented Nov 1, 2019

RaitoBezarius commented Nov 1, 2019

AmineChikhaoui commented Nov 4, 2019

RaitoBezarius commented Nov 10, 2019

NickSeagull commented Nov 14, 2019

RaitoBezarius commented Nov 14, 2019

AmineChikhaoui commented Nov 14, 2019

RaitoBezarius commented Nov 14, 2019

RaitoBezarius commented Sep 10, 2020

RDS: Add DBSubnet resource, VPC security groups #13

RDS: Add DBSubnet resource, VPC security groups #13

Conversation

RaitoBezarius commented Oct 30, 2019 • edited

AmineChikhaoui commented Oct 30, 2019

RaitoBezarius commented Oct 30, 2019

RaitoBezarius commented Oct 30, 2019

AmineChikhaoui commented Oct 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PsyanticY commented Nov 1, 2019

PsyanticY Nov 1, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaitoBezarius commented Nov 1, 2019

Choose a reason for hiding this comment

PsyanticY commented Nov 1, 2019 • edited

RaitoBezarius commented Nov 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaitoBezarius commented Nov 1, 2019

AmineChikhaoui commented Nov 1, 2019

RaitoBezarius commented Nov 1, 2019

AmineChikhaoui commented Nov 4, 2019

RaitoBezarius commented Nov 10, 2019

NickSeagull commented Nov 14, 2019

RaitoBezarius commented Nov 14, 2019

AmineChikhaoui commented Nov 14, 2019

RaitoBezarius commented Nov 14, 2019

RaitoBezarius commented Sep 10, 2020

RaitoBezarius commented Oct 30, 2019 •

edited

PsyanticY Nov 1, 2019 •

edited

PsyanticY commented Nov 1, 2019 •

edited