fix(rds): add the dependency on proxy targets to ensure dbInstance #12237
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
github-actions
bot
added
the
@aws-cdk/aws-rds
skinny85
previously requested changes
Dec 28, 2020
There was a problem hiding this comment.
Looks nice @saudkhanzada ! A few comments before I merge this in.
saudkhanzada
force-pushed
the
fix/add-dependency-for-proxy-target
branch
from
73e48fe
to
a833302
Compare
mergify
bot
dismissed
skinny85’s stale review
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
skinny85
approved these changes
Jan 4, 2021
There was a problem hiding this comment.
Awesome, thanks so much for the contribution @saudkhanzada !
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
mergify bot
pushed a commit
that referenced
this pull request
Dec 28, 2023
#28471) ### Description The related issue reports that deployment fails due to circular dependencies when multiple RDSProxy are created. The `DatabaseProxy` uses the `node.addDependency` method to ensure that the `CfnDBProxyTargetGroup` is created after the `DBCluster` and `DBInstance` are created (#12237). This works well for a single `DatabaseProxy`, but does not work well when multiple `DatabaseProxy` are created with `DatabaseCluster.addProxy`. When creating a `DatabaseProxy` with the `DatabaseCluster.addProxy` method, it is created as a child of the `DatabaseCluster`. https://github.com/aws/aws-cdk/blob/cd54c4239ec29182e30fd91634505df560d6e5f8/packages/aws-cdk-lib/aws-rds/lib/cluster.ts#L446 The `node.addDependency` method recursively sets dependencies on child Constructs, so if multiple `DatabaseProxy` are created as a child of a `DatabaseCluster` in the construct tree, multiple `DatabaseProxy` dependencies on each other. If the `addProxy` method is not used, the user initializes the `DatabaseProxy` directly and it does not become a child of `DatabaseCluster`. For example, ```ts new DatabaseProxy(stack, 'DBProxy', { proxyTarget: rds.ProxyTarget.fromCluster(cluster), vpc, }); ``` I believe this is the cause of the `these resources have a dependency cycle` error reported in the related issue. To correct this error, this PR uses `CfnResource.addDependency` instead of `node.addDependency` to avoid recurrent dependencies. Closes #25633 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
paulhcsun
pushed a commit
to paulhcsun/aws-cdk
that referenced
this pull request
Jan 5, 2024
aws#28471) ### Description The related issue reports that deployment fails due to circular dependencies when multiple RDSProxy are created. The `DatabaseProxy` uses the `node.addDependency` method to ensure that the `CfnDBProxyTargetGroup` is created after the `DBCluster` and `DBInstance` are created (aws#12237). This works well for a single `DatabaseProxy`, but does not work well when multiple `DatabaseProxy` are created with `DatabaseCluster.addProxy`. When creating a `DatabaseProxy` with the `DatabaseCluster.addProxy` method, it is created as a child of the `DatabaseCluster`. https://github.com/aws/aws-cdk/blob/cd54c4239ec29182e30fd91634505df560d6e5f8/packages/aws-cdk-lib/aws-rds/lib/cluster.ts#L446 The `node.addDependency` method recursively sets dependencies on child Constructs, so if multiple `DatabaseProxy` are created as a child of a `DatabaseCluster` in the construct tree, multiple `DatabaseProxy` dependencies on each other. If the `addProxy` method is not used, the user initializes the `DatabaseProxy` directly and it does not become a child of `DatabaseCluster`. For example, ```ts new DatabaseProxy(stack, 'DBProxy', { proxyTarget: rds.ProxyTarget.fromCluster(cluster), vpc, }); ``` I believe this is the cause of the `these resources have a dependency cycle` error reported in the related issue. To correct this error, this PR uses `CfnResource.addDependency` instead of `node.addDependency` to avoid recurrent dependencies. Closes aws#25633 ---- *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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
availability while creating the proxy
Fixes: #11311
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license