Only include DBSubnetGroupName inside ModifyDBInstanceRequest when there is delta (#61)

Issue #, if available: aws-controllers-k8s/community#1147

Description of changes:
* By default, ACK code-generator creates `UpdateRequest` payload completely from desired state of the resource and does not consider `delta` between `desired` & `latest` while creating the UpdateRequest payload.
* Due to above the UpdateRequest also contains the fields which are same between desired and latest state.
* This works fine in most cases where AWS Update APIs ignore the no-op updates in the backend but RDS's `ModifyDBInstance` API does not. If ModifyDBInstance API tries to update `DBSubnetGroupName` with the same name, it returns an error "InvalidVPCNetworkStateFault: The specified DB instance is already in the target DB subnet group.\n\tstatus code: 400" 
* I have added a hook that checks for the delta of `DBSubnetGroupName` before making the ModifyDBInstance API call

-----

* Tested manually and validated that the "InvalidVPCNetworkStateFault" error goes away
* added e2e test as well covering above case

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

Author: vijtrip2

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,13 +1,13 @@
 ack_generate_info:
-  build_date: "2022-02-08T20:23:58Z"
+  build_date: "2022-02-15T17:20:43Z"
   build_hash: edec6dad2fbd530d615d01e96f5251a806e1f36d
   go_version: go1.17.5
   version: v0.16.5
 api_directory_checksum: c5762d0b5707ca20866f2f0e85bc23863733ca11
 api_version: v1alpha1
 aws_sdk_go_version: v1.42.0
 generator_config_info:
-  file_checksum: 4b6518ecbce521a28d8a406d7b07f2662601136b
+  file_checksum: a0cccebb7e683a3b9b6082b5cde6630fd5ad4237
   original_file_name: generator.yaml
 last_modification:
   reason: API generation

apis/v1alpha1/generator.yaml

@@ -120,6 +120,8 @@ resources:
         template_path: hooks/db_instance/sdk_read_many_post_set_output.go.tpl
       sdk_update_pre_build_request:
         template_path: hooks/db_instance/sdk_update_pre_build_request.go.tpl
+      sdk_update_post_build_request:
+        template_path: hooks/db_instance/sdk_update_post_build_request.go.tpl
       sdk_delete_pre_build_request:
         template_path: hooks/db_instance/sdk_delete_pre_build_request.go.tpl
     exceptions:

generator.yaml

@@ -120,6 +120,8 @@ resources:
         template_path: hooks/db_instance/sdk_read_many_post_set_output.go.tpl
       sdk_update_pre_build_request:
         template_path: hooks/db_instance/sdk_update_pre_build_request.go.tpl
+      sdk_update_post_build_request:
+        template_path: hooks/db_instance/sdk_update_post_build_request.go.tpl
       sdk_delete_pre_build_request:
         template_path: hooks/db_instance/sdk_delete_pre_build_request.go.tpl
     exceptions:

pkg/resource/db_instance/sdk.go

@@ -0,0 +1,8 @@
+	// ModifyDBInstance call will return ValidationError when the
+	// ModifyDBInstanceRequest contains the same DBSubnetGroupName
+	// as the DBInstance. So, if there is no delta between
+	// desired and latest for Spec.DBSubnetGroupName, exclude it
+	// from ModifyDBInstanceRequest
+	if !delta.DifferentAt("Spec.DBSubnetGroupName") {
+		input.DBSubnetGroupName = nil
+	}

templates/hooks/db_instance/sdk_update_post_build_request.go.tpl

@@ -28,6 +28,7 @@ class TestBootstrapResources:
     VPCID: str
     SubnetAZ1: str
     SubnetAZ2: str
+    DBSubnetGroupName: str
 
 _bootstrap_resources = None
 

test/e2e/bootstrap_resources.py

@@ -7,6 +7,7 @@ spec:
   copyTagsToSnapshot: $COPY_TAGS_TO_SNAPSHOT
   dbInstanceClass: db.t3.micro
   dbInstanceIdentifier: $DB_INSTANCE_ID
+  dbSubnetGroupName: $DB_SUBNET_GROUP_NAME
   engine: postgres
   engineVersion: "13.2"
   masterUsername: root

test/e2e/resources/db_instance_postgres13_t3_micro.yaml

@@ -18,6 +18,7 @@
 import time
 
 from acktest import resources
+from acktest.resources import random_suffix_name
 from acktest.aws.identity import get_region
 from e2e import bootstrap_directory
 from e2e.bootstrap_resources import (
@@ -94,6 +95,21 @@ def create_subnet(vpc_id: str, az_id: str, cidr: str) -> str:
     return subnet_id
 
 
+def create_db_subnet_group(db_subnet_group_name: str, subnet_az1_id: str, subnet_az2_id:str):
+    region = get_region()
+    rds = boto3.client("rds", region_name=region)
+
+    logging.debug(f"Creating DBSubnetGroup with name {db_subnet_group_name}")
+
+    rds.create_db_subnet_group(
+        DBSubnetGroupName=db_subnet_group_name,
+        DBSubnetGroupDescription='DBSubnetGroup for e2e testing of ACK rds-controller',
+        SubnetIds=[subnet_az1_id, subnet_az2_id],
+    )
+
+    logging.info(f"Created DBSubnetGroup {db_subnet_group_name}")
+
+
 def service_bootstrap() -> dict:
     logging.getLogger().setLevel(logging.INFO)
     region = get_region()
@@ -103,14 +119,17 @@ def service_bootstrap() -> dict:
     subnet_az1_id = create_subnet(vpc_id, az1, SUBNET_AZ1_CIDR)
     az2 = f"{region}b"
     subnet_az2_id = create_subnet(vpc_id, az2, SUBNET_AZ2_CIDR)
-
+    db_subnet_group_name = random_suffix_name("ack-test-subnet-group", 30)
+    create_db_subnet_group(db_subnet_group_name, subnet_az1_id, subnet_az2_id)
 
     return TestBootstrapResources(
         vpc_id,
         subnet_az1_id,
         subnet_az2_id,
+        db_subnet_group_name
     ).__dict__
 
+
 if __name__ == "__main__":
     config = service_bootstrap()
     # Write config to current directory by default

test/e2e/service_bootstrap.py

@@ -42,13 +42,29 @@ def delete_vpc(vpc_id: str):
     logging.info(f"Deleted VPC {vpc_id}")
 
 
+def delete_db_subnet_group(db_subnet_group_name: str):
+    region = get_region()
+    rds = boto3.client("rds", region_name=region)
+
+    rds.delete_db_subnet_group(
+        DBSubnetGroupName=db_subnet_group_name,
+    )
+
+    logging.info(f"Deleted DBSubnetGroup {db_subnet_group_name}")
+
+
 def service_cleanup(config: dict):
     logging.getLogger().setLevel(logging.INFO)
 
     resources = TestBootstrapResources(
         **config
     )
 
+    try:
+        delete_db_subnet_group(resources.DBSubnetGroupName)
+    except:
+        logging.exception(f"Unable to delete DBSubnetGroup {resources.DBSubnetGroupName}")
+
     try:
         delete_subnet(resources.SubnetAZ1)
     except:
@@ -64,6 +80,7 @@ def service_cleanup(config: dict):
     except:
         logging.exception(f"Unable to delete VPC {resources.VPCID}")
 
+
 if __name__ == "__main__":   
     bootstrap_config = resources.read_bootstrap_config(bootstrap_directory)
     service_cleanup(bootstrap_config) 

test/e2e/service_cleanup.py

@@ -19,6 +19,7 @@
 import pytest
 
 from acktest.k8s import resource as k8s
+from acktest.resources import random_suffix_name
 from e2e import service_marker, CRD_GROUP, CRD_VERSION, load_rds_resource
 from e2e.replacement_values import REPLACEMENT_VALUES
 from e2e.bootstrap_resources import get_bootstrap_resources
@@ -67,7 +68,7 @@ def test_crud_postgres13_t3_micro(
             self,
             k8s_secret,
     ):
-        db_instance_id = "pg13-t3-micro"
+        db_instance_id = random_suffix_name("pg13-t3-micro", 20)
         secret = k8s_secret(
             self.MUP_NS,
             self.MUP_SEC_NAME,
@@ -81,6 +82,7 @@ def test_crud_postgres13_t3_micro(
         replacements["MASTER_USER_PASS_SECRET_NAMESPACE"] = secret.ns
         replacements["MASTER_USER_PASS_SECRET_NAME"] = secret.name
         replacements["MASTER_USER_PASS_SECRET_KEY"] = secret.key
+        replacements["DB_SUBNET_GROUP_NAME"] = get_bootstrap_resources().DBSubnetGroupName
 
         resource_data = load_rds_resource(
             "db_instance_postgres13_t3_micro",
@@ -128,7 +130,8 @@ def test_crud_postgres13_t3_micro(
         # shows the new value of the field.
         latest = db_instance.get(db_instance_id)
         assert latest is not None
-        assert latest['CopyTagsToSnapshot'] == False
+        assert latest['CopyTagsToSnapshot'] is False
+        assert latest['DBSubnetGroup']['DBSubnetGroupName'] == get_bootstrap_resources().DBSubnetGroupName
         updates = {
             "spec": {"copyTagsToSnapshot": True},
         }