RDS DBInstance modifying unchanged fields causes errors

[armsnyder]

What happened?

We are using the dbinstances.rds.aws.crossplane.io CRD to create RDS Aurora cluster instances, and we found that we are unable to modify the spec after creation due to an error condition.

The provider has a bug where it is sending all spec.forProvider fields to the ModifyDBInstance AWS API call, and some of these parameters are not accepted by that API, depending on the combination of parameters used, even if the parameter values are unchanged.

How can we reproduce it?

Apply yaml:

apiVersion: rds.aws.crossplane.io/v1alpha1
kind: DBCluster
metadata:
  name: example-aurora-mysql-cluster
spec:
  forProvider:
    region: us-east-1
    engine: aurora-mysql
    engineVersion: 5.7.mysql_aurora.2.09.1
    masterUsername: admin
    masterUserPasswordSecretRef:
      name: example-aurora-mysql-cluster
      namespace: crossplane-system
      key: password
    databaseName: auroradb
    skipFinalSnapshot: true
  writeConnectionSecretToRef:
    name: example-aurora-mysql-cluster
    namespace: default
  providerConfigRef:
    name: example
---
apiVersion: v1
kind: Secret
metadata:
  name: example-aurora-mysql-cluster
  namespace: crossplane-system
type: Opaque
data:
  password: dGVzdFBhc3N3b3JkITEyMw== # testPassword!123
---
apiVersion: rds.aws.crossplane.io/v1alpha1
kind: DBInstance
metadata:
  name: example-aurora-mysql-instance
spec:
  forProvider:
    region: us-east-1
    dbInstanceClass: db.t3.small
    engine: aurora-mysql
    dbClusterIdentifier: example-aurora-mysql-cluster
  writeConnectionSecretToRef:
    name: example-aurora-mysql-instance
    namespace: default
  providerConfigRef:
    name: example

Update a spec field, for example change spec.forProvider.dbInstanceClass to db.t3.medium.

The object sync fails with the following status.conditions:

conditions:
- lastTransitionTime: "2021-10-27T23:26:46Z"
  reason: Creating
  status: "False"
  type: Ready
- lastTransitionTime: "2021-10-27T23:30:33Z"
  message: "update failed: cannot update DBInstance in AWS: InvalidParameterCombination:
    The specified DB instance is a member of a DB cluster. Storage grows based on
    usage. You don't need to set it explicitly.\n\tstatus code: 400, request id:
    291d5811-eda2-4fdc-abd3-bf4c676ee959"
  reason: ReconcileError
  status: "False"
  type: Synced

Additional context

We noticed that the ack-generate tool, used by this provider, can generate inputs that only set modified fields in API requests. For example, here is where it is used in the ACK RDS controller. Can this provider make use of this functionality to fix this issue across all generated controllers?

What environment did it happen in?

Crossplane version: v1.4.2
AWS provider version: v0.20.0

[mothershipper]

@haarchri by chance, do you know if 07ce4f2 resolves this issue?

We're hitting this now too - I'm not 100% sure about the root cause described above, but I've noticed that our DbInstance.ForProvider.Spec keeps getting AllocatedStorage set to 1, which seems like it would then trigger the update call to send it in the modify call:

https://github.com/crossplane/provider-aws/blob/master/pkg/controller/rds/dbinstance/zz_conversions.go#L889-L891

We can try to upgrade later, but we're a few versions behind.

[haarchri]

For Storage this is fixed by #1197 this will be part of 0.26 release

[eloo]

@haarchri unfortunately this issue seems not to be resolved
we still see the following error in our managed resources (rds instances attached to a rds cluster)

update failed: cannot update DBInstance in AWS: InvalidParameterCombination: The specified DB instance is a member of a DB cluster. Storage grows based on usage. You don't need to set it explicitly.

[haarchri]

Is this a new Instance or an old instance?

[eloo]

its an old instance. we have seen that we need to remove the old AllocatedStorage in the for provider section.

but afterwards we get a similar error backup retention

managed/dbinstance.rds.aws.crossplane.io  cannot update DBInsta
nce in AWS: InvalidParameterCombination: The specified DB Instance is a member of a cluster. Modify backup retention for the
DB Cluster using the ModifyDbCluster API

but if i remove this it is going to be reconciled directly
so i guess there are more fields which needs to be excluded when the instance is attached to a cluster

[eloo]

@haarchri should we reopen that ticket or create a new one as this bug seems not be fully fixed.

[haarchri]

for storage we fixed it - now we have the backup retention - think its trivial to add this like storage - i would prefer to open a new issue

[eloo]

sounds good. a will create one

[iAnomaly]

Just to be clear, it's going to be more than just BackupRetention. You can visit the AWS CLI docs for modify-db-instance and search for the string "managed by the DB cluster" to find all fields that cannot be set when associating a DBInstance with a DBCluster.

I've opened a PR for all the remaining fields: https://github.com/crossplane/provider-aws/pull/1285/files

[eloo]

@iAnomaly awesome
so i guess there is no need for a new ticket if there is already a PR open now

i hope we can get this merged soon :) currently all of our dbinstance are out of sync

[iAnomaly]

I think a new/separate Issue would still be good @eloo since this one is technically fixed already.

[eloo]

#1286 created

[RamazanKara]

Not fixed for me, occured again when i switched a db instance from single to multi az