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.rdsinstance: Add storage autoscaling (MaxAllocatedStorage) #794

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

chlunde
Copy link
Collaborator

@chlunde chlunde commented Aug 9, 2021

Description of your changes

Add storage autoscaling parameter MaxAllocatedStorage.
This parameter defines the upper limit to which Amazon RDS can automatically
scale the storage of the DB instance.

For more information about this setting, including limitations that apply
to it, see Managing capacity automatically with Amazon RDS storage autoscaling
(https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_PIOPS.StorageTypes.html#USER_PIOPS.Autoscaling)
in the Amazon RDS User Guide.

Make rds.CreatePatch exclude AllocatedStorage if autoscaling is enabled,
and the requested AllocatedStorage is lower than was is currently
allocated.

This allows a user to manually scale up storage even if autoscaling is
enabled, and prevents crossplane from attempting to scale down if
AWS has performed autoscaling.

One problem is that this change silently ignores any attempts to manually
downscale when autoscaling is enabled. Since downscaling is not
supported and it is not possible to distinguish this from autoscaling, I
don't see any other solutions.

I have added tests to verify the logic for omitting StorageSize and the
up to date-test:

--- FAIL: TestObserve (0.00s)
    --- FAIL: TestObserve/AutoscaledStorageIsUpToDate (0.00s)
        rdsinstance_test.go:354: r: -want, +got:
              managed.ExternalObservation{
              	ResourceExists:          true,
            - 	ResourceUpToDate:        true,
            + 	ResourceUpToDate:        false,
              	ResourceLateInitialized: false,
              	ConnectionDetails:       nil,
              	Diff:                    "",
              }
--- FAIL: TestUpdate (0.00s)
    --- FAIL: TestUpdate/AutoscaleExcludeStorage (0.00s)
        rdsinstance_test.go:674: r: -want, +got:
              interface{}(
            + 	e"cannot modify RDS instance: AllocatedStorage must not be set when on a modify request when AWS has autoscaled the storage",
              )

Fixes #661

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

  • Add unit test for IsUpToDate and Modify
  • Test scaling up by filling disk, verified that we don't hang on forever trying to scale down (thanks @petteja)
  • Test scaling up manually in the console, verified that we don't hang on forever trying to scale down
  • Tested manually scaling up by bumping the allocated storage in the CR - still works

@chlunde
Copy link
Collaborator Author

chlunde commented Sep 12, 2021

Rebased and retested.

-- Fill disk (too fast for "graceful" autoscaling)
CREATE TABLE large_test (num1 bigint, num2 double precision, num3 double precision);
INSERT INTO large_test (num1, num2, num3)
  SELECT round(random()*10), random(), random()*142
  FROM generate_series(1, 200000000) s(i);
apiVersion: database.aws.crossplane.io/v1beta1
kind: RDSInstance
metadata:
  name: example-rds
spec:
  forProvider:
    allocatedStorage: 20
    maxAllocatedStorage: 60
    autoMinorVersionUpgrade: true
    backupRetentionPeriod: 0
    copyTagsToSnapshot: false
    dbInstanceClass: db.t2.micro
    deletionProtection: false
    enableIAMDatabaseAuthentication: false
    enablePerformanceInsights: false
    engine: postgres
    engineVersion: "12.4"
    masterUsername: adminuser
    multiAZ: false
    publiclyAccessible: true
    region: us-east-1
    storageEncrypted: false
    storageType: gp2
  writeConnectionSecretToRef:
    name: example-rdsinstance-out
    namespace: crossplane-system

@chlunde chlunde removed their assignment Sep 12, 2021
@AaronME AaronME self-assigned this Sep 24, 2021
@haarchri
Copy link
Member

haarchri commented Nov 4, 2021

@chlunde can you rebase again ? testet the future it is working

This parameter defines the upper limit to which Amazon RDS can automatically
scale the storage of the DB instance.

For more information about this setting, including limitations that apply
to it, see Managing capacity automatically with Amazon RDS storage autoscaling
(https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_PIOPS.StorageTypes.html#USER_PIOPS.Autoscaling)
in the Amazon RDS User Guide.

Make rds.CreatePatch exclude AllocatedStorage if autoscaling is enabled,
and the requested AllocatedStorage is lower than was is currently
allocated.

This allows a user to manually scale *up* storage even if autoscaling is
enabled, and prevents crossplane from attempting to scale *down* if
AWS has performed autoscaling.

One problem is that this change silently ignores any attempts to manually
downscale when autoscaling is enabled. Since downscaling is not
supported and it is not possible to distinguish this from autoscaling, I
don't see any other solutions.

I have added tests to verify the logic for omitting StorageSize and the
up to date-test:

    --- FAIL: TestObserve (0.00s)
        --- FAIL: TestObserve/AutoscaledStorageIsUpToDate (0.00s)
            rdsinstance_test.go:354: r: -want, +got:
                  managed.ExternalObservation{
                        ResourceExists:          true,
                -       ResourceUpToDate:        true,
                +       ResourceUpToDate:        false,
                        ResourceLateInitialized: false,
                        ConnectionDetails:       nil,
                        Diff:                    "",
                  }
    --- FAIL: TestUpdate (0.00s)
        --- FAIL: TestUpdate/AutoscaleExcludeStorage (0.00s)
            rdsinstance_test.go:674: r: -want, +got:
                  interface{}(
                +       e"cannot modify RDS instance: AllocatedStorage must not be set when on a modify request when AWS has autoscaled the storage",
                  )

Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
@chlunde
Copy link
Collaborator Author

chlunde commented Feb 7, 2022

@haarchri rebased

@chlunde chlunde requested a review from haarchri February 7, 2022 20:11
Copy link
Member

@haarchri haarchri left a comment

Choose a reason for hiding this comment

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

LGTM - tested again it is working thanks for your efford @chlunde

@chlunde chlunde merged commit 7961797 into crossplane-contrib:master Feb 8, 2022
@chlunde chlunde deleted the rds-autoscaling branch February 8, 2022 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add RDS storage autoscaling config to RDSInstance
3 participants