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

[Backport release-0.44] fix(database): Ignore AvailabilityZone if MultiAZ is set #1913

Merged
merged 1 commit into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions apis/database/v1beta1/rdsinstance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,9 @@ type RDSInstanceParameters struct {
// Default: A random, system-chosen Availability Zone in the endpoint's AWS
// Region.
// Example: us-east-1d
// Constraint: The AvailabilityZone parameter can't be specified if the MultiAZ
// parameter is set to true. The specified Availability Zone must be in the
// Constraint: The AvailabilityZone parameter is ignored if the MultiAZ
// is set to true.
// The specified Availability Zone must be in the
// same AWS Region as the current endpoint.
// +immutable
// +optional
Expand Down
6 changes: 3 additions & 3 deletions package/crds/database.aws.crossplane.io_rdsinstances.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ spec:
and Availability Zones, see Regions and Availability Zones (http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Concepts.RegionsAndAvailabilityZones.html).
Default: A random, system-chosen Availability Zone in the endpoint''s
AWS Region. Example: us-east-1d Constraint: The AvailabilityZone
parameter can''t be specified if the MultiAZ parameter is set
to true. The specified Availability Zone must be in the same
AWS Region as the current endpoint.'
parameter is ignored if the MultiAZ is set to true. The specified
Availability Zone must be in the same AWS Region as the current
endpoint.'
type: string
backupRetentionPeriod:
description: 'BackupRetentionPeriod is the number of days for
Expand Down
12 changes: 11 additions & 1 deletion pkg/clients/database/rds.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ func GenerateRestoreRDSInstanceToPointInTimeInput(name string, p *v1beta1.RDSIns
// CreatePatch creates a *v1beta1.RDSInstanceParameters that has only the changed
// values between the target *v1beta1.RDSInstanceParameters and the current
// *rds.DBInstance
func CreatePatch(in *rdstypes.DBInstance, target *v1beta1.RDSInstanceParameters) (*v1beta1.RDSInstanceParameters, error) {
func CreatePatch(in *rdstypes.DBInstance, spec *v1beta1.RDSInstanceParameters) (*v1beta1.RDSInstanceParameters, error) { //nolint:gocyclo
target := spec.DeepCopy()
currentParams := &v1beta1.RDSInstanceParameters{}
LateInitialize(currentParams, in)

Expand All @@ -329,6 +330,15 @@ func CreatePatch(in *rdstypes.DBInstance, target *v1beta1.RDSInstanceParameters)
})
}

// AvailabilityZone parameters is not allowed for MultiAZ deployments.
// So set this to nil if that is the case to avoid unnecessary diffs.
if ptr.Deref(target.MultiAZ, false) {
target.AvailabilityZone = nil
}
if ptr.Deref(currentParams.MultiAZ, false) {
currentParams.AvailabilityZone = nil
}

// Don't attempt to scale down storage if autoscaling is enabled,
// and the current storage is larger than what was once
// requested. We still want to allow the user to manually scale
Expand Down
35 changes: 35 additions & 0 deletions pkg/clients/database/rds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,19 @@ func TestCreatePatch(t *testing.T) {
AllocatedStorage: allocatedStorage,
CharacterSetName: &characterSetName,
DBName: &dbName,
AvailabilityZone: ptr.To("az1"),
},
p: &v1beta1.RDSInstanceParameters{
AllocatedStorage: awsclient.IntAddress(awsclient.Int64(30)),
CharacterSetName: &characterSetName,
DBName: &dbName,
AvailabilityZone: ptr.To("az2"),
},
},
want: want{
patch: &v1beta1.RDSInstanceParameters{
AllocatedStorage: awsclient.IntAddress(awsclient.Int64(30)),
AvailabilityZone: ptr.To("az2"),
},
},
},
Expand Down Expand Up @@ -210,6 +213,21 @@ func TestCreatePatch(t *testing.T) {
patch: &v1beta1.RDSInstanceParameters{},
},
},
"IgnoreDifferentAvailabilityZoneForMultiAZ": {
args: args{
db: &rdstypes.DBInstance{
AvailabilityZone: ptr.To("az1"),
MultiAZ: true,
},
p: &v1beta1.RDSInstanceParameters{
AvailabilityZone: ptr.To("az2"),
MultiAZ: ptr.To(true),
},
},
want: want{
patch: &v1beta1.RDSInstanceParameters{},
},
},
}

for name, tc := range cases {
Expand Down Expand Up @@ -473,6 +491,23 @@ func TestIsUpToDate(t *testing.T) {
},
want: true,
},
"NoUpdateForDifferentAvailabilityZoneWhenMultiAZ": {
args: args{
db: rdstypes.DBInstance{
AvailabilityZone: ptr.To("az1"),
MultiAZ: true,
},
r: v1beta1.RDSInstance{
Spec: v1beta1.RDSInstanceSpec{
ForProvider: v1beta1.RDSInstanceParameters{
AvailabilityZone: ptr.To("az2"),
MultiAZ: ptr.To(true),
},
},
},
},
want: true,
},
}

for name, tc := range cases {
Expand Down