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

(rds): Aurora Serverless V2 instances are re-created when switching from a common workaround to an official solution #25942

Closed
moltar opened this issue Jun 12, 2023 · 9 comments · Fixed by #26472
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@moltar
Copy link
Contributor

moltar commented Jun 12, 2023

Describe the bug

A feature release via #25437 (Serverless V2 support) breaks a common workaround for V2.

Workaround explained here: #20197 (comment)

Note that it has 21 likes, so likely there are many instances of this workaround in the wild.

However, when switching to the official solution from #25437, the instances change the ID and need to be recreated.

A question I raised in the PR is whether or not it is possible to retain the original IDs somehow. I don't see the need for them to change.

Example: #25437 (comment)

Expected Behavior

The instances not to be re-created.

Current Behavior

ID changes and the instance gets re-created.

Reproduction Steps

#25437 (comment)

Possible Solution

N/A

Additional Information/Context

#25437 (comment)

CDK CLI Version

latest

Framework Version

2

Node.js Version

16

OS

macOS

Language

Typescript

Language Version

TS 5

Other information

No response

@moltar moltar added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 12, 2023
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Jun 12, 2023
@peterwoodworth peterwoodworth added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 12, 2023
@peterwoodworth
Copy link
Contributor

Have you tried setting instanceIdentifier?

@peterwoodworth peterwoodworth added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 12, 2023
@IllarionovDimitri
Copy link

instance_identifier worked for me; the switch from a workaround to the official solution indeed re-created the instances but no troubles during that, works fine

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 14, 2023
@moltar
Copy link
Contributor Author

moltar commented Jun 14, 2023

Have you tried setting instanceIdentifier?

If I set that prop, I simply get an additional prop, but it does not affect the ID:

      writer: ClusterInstance.serverlessV2('Instance', {
        instanceIdentifier: 'Instance', // ⬅️ Added
        publiclyAccessible: databaseEngine?.public,
        parameterGroup: this.parameterGroup,
      }),
-     "ClusterInstance1448F06E4": Object {
+     "ClusterInstanceAAE2C8BD": Object {
        "DeletionPolicy": "Delete",
        "Properties": Object {
          "DBClusterIdentifier": Object {
            "Ref": "ClusterEB0386A7",
          },
          "DBInstanceClass": "db.serverless",
+         "DBInstanceIdentifier": "Instance",
          "DBParameterGroupName": Object {

@moltar
Copy link
Contributor Author

moltar commented Jun 14, 2023

instance_identifier worked for me; the switch from a workaround to the official solution indeed re-created the instances but no troubles during that, works fine

re-created the instances

So you were able to re-create the instance, in a B/G deployment-way, without any downtime or data loss?

Which engine did you use?

@IllarionovDimitri
Copy link

here how I define the writer and the reader instances

writer_instance = rds.ClusterInstance.serverless_v2(
            f"{config.ID}-writer-instance",
            instance_identifier=f"{config.ID}-writer-instance",
        )

reader_instance = rds.ClusterInstance.serverless_v2(
            f"{config.ID}-reader-instance",
            instance_identifier=f"{config.ID}-reader-instance",
            scale_with_writer=True,
        )

and here I define the cluster, it is an Aurora Postgre Engine, but B/G deployment is not supported by this kind of engine, so I simply deployed the new/official solution on top of the workaround, w/o data loss.

db_cluster = rds.DatabaseCluster(
             ...
            engine=rds.DatabaseClusterEngine.aurora_postgres(
                version=rds.AuroraPostgresEngineVersion.VER_13_9
            ),
            ...
            security_groups=[sg_db_cluster_import],
            serverless_v2_max_capacity=1,
            serverless_v2_min_capacity=0.5,
            readers=[reader_instance],
            storage_type=rds.DBClusterStorageType.AURORA,
            subnet_group=subnet_group,
            ...        
            writer=writer_instance,
        )

@IllarionovDimitri
Copy link

after testing on dev stage, have just migrated our prod stage from workaround to official solution. no problems so far

@hastarin
Copy link

I just tried following this with the .NET version of the CDK and it wanted to create a new reader/writer and complained that the InstanceId already exists.

I tried to follow the documentation for migrating from instance props but the property it suggests setting of isFromLegacyInstanceProps does not exist for a ServerlessV2 reader/writer.

@dabrowne
Copy link
Contributor

I was able to successfully migrate from the custom resource workaround to the new Serverless V2 constructs without downtime using a combination of isFromLegacyInstanceProps: true and manually setting instanceIdentifier.

There was a bit of a stumbling block using CDK with Typescript, which is that isFromLegacyInstanceProps doesn't exist on the ServerlessV2ClusterInstanceProps type. This seems to be an oversight, as the author has assumed that nobody would be migrating serverless instances in this way as they have previously been unsupported. I found that including the prop anyway did however have the desired effect.

Here's what I ended up doing:

const instanceProps: rds.ServerlessV2ClusterInstanceProps & {
  isFromLegacyInstanceProps: boolean;
} = {
  publiclyAccessible: false,
  // ...
  isFromLegacyInstanceProps: true,
};

const dbCluster = new rds.DatabaseCluster(this, "cluster", {
  vpc,
  vpcSubnets,
  securityGroups,
  // ...
  writer: rds.ClusterInstance.serverlessV2("Instance1", {
    ...instanceProps,
    instanceIdentifier: "<current instance 1 identifier>",
  }),
  readers: [
    rds.ClusterInstance.serverlessV2("Instance2", {
      ...instanceProps,
      instanceIdentifier: "<current instance 2 identifier>",
      scaleWithWriter: true,
    }),
  ],
  serverlessV2MinCapacity: 1,
  serverlessV2MaxCapacity: 12,
  // ...
}

@mergify mergify bot closed this as completed in #26472 Jul 24, 2023
mergify bot pushed a commit that referenced this issue Jul 24, 2023
…stance.serverlessV2` (#26472)

**Context**
A recent feature release #25437 has added support for Aurora Serverless V2 cluster instances. This change also introduced a new approach for defining cluster instances, deprecating `instanceProps` in the process.

The new approach uses `ClusterInstance.provisioned()` and `ClusterInstance.serverlessV2()` to define instances and their parameters on a per-instance basis. A migration flag `isFromLegacyInstanceProps` has also been added to the `ClusterInstance.provisioned()` constructor props to allow for migration to this new approach without destructive changes to the generated CFN template.

**Bug**
Because the `DatabaseCluster` construct has not previously had official support for Serverless V2 instances, the same migration flag has not been made available for `ClusterInstance.serverlessV2()`. This ignores the fact that many people have already provisioned serverless v2 instances using a common workaround described here #20197 (comment). People who have used this method previously have no clean migration path. This has been previously raised in #25942.

**Fix**
This fix simply exposes the `isFromLegacyInstanceProps` flag on **both** `ProvisionedClusterInstanceProps` and `ServerlessV2ClusterInstanceProps`. The behaviour for this flag is already implemented and applied across both instance types, so this is a type-only change. I have however added a test to capture this upgrade path for Serverless V2 instances from the common workaround.


Closes #25942.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this issue Jul 29, 2023
…stance.serverlessV2` (aws#26472)

**Context**
A recent feature release aws#25437 has added support for Aurora Serverless V2 cluster instances. This change also introduced a new approach for defining cluster instances, deprecating `instanceProps` in the process.

The new approach uses `ClusterInstance.provisioned()` and `ClusterInstance.serverlessV2()` to define instances and their parameters on a per-instance basis. A migration flag `isFromLegacyInstanceProps` has also been added to the `ClusterInstance.provisioned()` constructor props to allow for migration to this new approach without destructive changes to the generated CFN template.

**Bug**
Because the `DatabaseCluster` construct has not previously had official support for Serverless V2 instances, the same migration flag has not been made available for `ClusterInstance.serverlessV2()`. This ignores the fact that many people have already provisioned serverless v2 instances using a common workaround described here aws#20197 (comment). People who have used this method previously have no clean migration path. This has been previously raised in aws#25942.

**Fix**
This fix simply exposes the `isFromLegacyInstanceProps` flag on **both** `ProvisionedClusterInstanceProps` and `ServerlessV2ClusterInstanceProps`. The behaviour for this flag is already implemented and applied across both instance types, so this is a type-only change. I have however added a test to capture this upgrade path for Serverless V2 instances from the common workaround.


Closes aws#25942.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
5 participants