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

Add Global Cluster support #673

Merged

Conversation

ActuallyTrent
Copy link
Contributor

@ActuallyTrent ActuallyTrent commented May 21, 2021

Description of your changes

Adds support for Global Aurora Clusters

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

  1. Created a new GlobalCluster without an existing DBCluster resource. Came up properly
  2. Created a new GlobalCluster referencing an existing DBCluster resource via sourceDBClusterIDRef and verified it created a GlobalCluster with the sourceDBCluster attached properly.

Signed-off-by: Trenton Lawrence <pitime2@outlook.com>
@ActuallyTrent ActuallyTrent force-pushed the task/mothership/add-global-cluster branch from 172f62c to 9553ddd Compare May 21, 2021 13:44
Copy link
Member

@muvaf muvaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after a couple small comments! Let me know when it's ready to be merged.

// SourceDBClusterIdentifier.
// +immutable
// +optional
SourceDBClusterIDRef *xpv1.Reference `json:"sourceDBClusterIDRef,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SourceDBClusterIDRef *xpv1.Reference `json:"sourceDBClusterIDRef,omitempty"`
SourceDBClusterIdentifierRef *xpv1.Reference `json:"sourceDBClusterIdentifierRef,omitempty"`

We use the exact name of the field as prefix for consistencty.

// set SourceDBClusterIdentifier.
// +immutable
// +optional
SourceDBClusterIDSelector *xpv1.Selector `json:"sourceDBClusterIDSelector,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SourceDBClusterIDSelector *xpv1.Selector `json:"sourceDBClusterIDSelector,omitempty"`
SourceDBClusterIdentifierSelector *xpv1.Reference `json:"sourceDBClusterIdentifierSelector,omitempty"`

We use the exact name of the field as prefix for consistencty.

clusterIdentifier := aws.String(meta.GetExternalName(cr))
resp := &svcsdk.DescribeGlobalClustersOutput{}
for _, dbCluster := range obj.GlobalClusters {
if aws.StringValue(dbCluster.GlobalClusterIdentifier) == aws.StringValue(clusterIdentifier) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if aws.StringValue(dbCluster.GlobalClusterIdentifier) == aws.StringValue(clusterIdentifier) {
if aws.StringValue(dbCluster.GlobalClusterIdentifier) == meta.GetExternalName(cr) {

…ency

Signed-off-by: Trenton Lawrence <pitime2@outlook.com>
@ActuallyTrent ActuallyTrent force-pushed the task/mothership/add-global-cluster branch from 714204d to fec6ea2 Compare May 25, 2021 10:25
@muvaf muvaf merged commit a2d78f4 into crossplane-contrib:master May 25, 2021
@muvaf
Copy link
Member

muvaf commented May 25, 2021

Thanks @PocketMobsters !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants