Skip to content

Commit

Permalink
Merge pull request #115 from ciaranRoche/intly-5467
Browse files Browse the repository at this point in the history 
fix handle modify aws instance
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Feb 20, 2020
2 parents 575ae29 + e338784 commit 95edb68
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 23 deletions.
2 changes: 1 addition & 1 deletion pkg/providers/aws/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const (

DefaultFinalizer = "finalizers.cloud-resources-operator.integreatly.org"

defaultReconcileTime = time.Second * 300
defaultReconcileTime = time.Second * 30

regionUSEast1 = "us-east-1"
regionUSWest2 = "us-west-2"
Expand Down
1 change: 1 addition & 0 deletions pkg/providers/aws/credentials.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var (
"elasticache:DescribeCacheClusters",
"elasticache:DescribeCacheSubnetGroups",
"elasticache:CreateCacheSubnetGroup",
"elasticache:ModifyReplicationGroup",
"rds:DescribeDBInstances",
"rds:CreateDBInstance",
"rds:DeleteDBInstance",
Expand Down
62 changes: 54 additions & 8 deletions pkg/providers/aws/provider_postgres.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,24 @@ func (p *PostgresProvider) createRDSInstance(ctx context.Context, cr *v1alpha1.P

// check rds instance phase
if *foundInstance.DBInstanceStatus != "available" {
logrus.Infof("found instance %s current status %s", *foundInstance.DBInstanceIdentifier, *foundInstance.DBInstanceStatus)
return nil, croType.StatusMessage(fmt.Sprintf("createRDSInstance() in progress, current aws rds resource status is %s", *foundInstance.DBInstanceStatus)), nil
}

// check if found instance and user strategy differs, and modify instance
logrus.Info("found existing rds instance")
logrus.Infof("found existing rds instance: %s", *foundInstance.DBInstanceIdentifier)
mi := buildRDSUpdateStrategy(rdsCfg, foundInstance)
if mi == nil {
logrus.Infof("rds instance %s is as expected", *foundInstance.DBInstanceIdentifier)
}
if mi != nil {
if _, err = rdsSvc.ModifyDBInstance(mi); err != nil {
return nil, "failed to modify instance", err
errMsg := fmt.Sprintf("error experienced trying to modify db instance: %s", *foundInstance.DBInstanceIdentifier)
return nil, croType.StatusMessage(errMsg), errorUtil.Wrap(err, errMsg)
}
return nil, croType.StatusMessage(fmt.Sprintf("changes detected, modifyDBInstance() in progress, current aws rds resource status is %s", *foundInstance.DBInstanceStatus)), nil
logrus.Infof("set pending modifications for rds instance: %s", *foundInstance.DBInstanceIdentifier)
}

// Add Tags to Aws Postgres resources
msg, err := p.TagRDSPostgres(ctx, cr, rdsSvc, foundInstance)
if err != nil {
Expand Down Expand Up @@ -456,8 +462,9 @@ func (p *PostgresProvider) getRDSConfig(ctx context.Context, r *v1alpha1.Postgre
return rdsCreateConfig, rdsDeleteConfig, stratCfg, nil
}

// verifies if there is a change between a found instance and the configuration from the instance strat
// verifies if there is a change between a found instance and the configuration from the instance strat and verified the changes are not pending
func buildRDSUpdateStrategy(rdsConfig *rds.CreateDBInstanceInput, foundConfig *rds.DBInstance) *rds.ModifyDBInstanceInput {
logrus.Infof("verifying that %s configuration is as expected", *foundConfig.DBInstanceIdentifier)
updateFound := false

mi := &rds.ModifyDBInstanceInput{}
Expand Down Expand Up @@ -495,12 +502,51 @@ func buildRDSUpdateStrategy(rdsConfig *rds.CreateDBInstanceInput, foundConfig *r
mi.MultiAZ = rdsConfig.MultiAZ
updateFound = true
}
if !updateFound {
if !updateFound || !verifyPendingModification(mi, foundConfig.PendingModifiedValues) {
return nil
}
return mi
}

// returns true if modify input is not pending
func verifyPendingModification(mi *rds.ModifyDBInstanceInput, pm *rds.PendingModifiedValues) bool {
pendingModifications := true
if pm == nil {
return pendingModifications
}
if mi.DBPortNumber != nil && pm.Port != nil {
if *mi.DBPortNumber == *pm.Port {
pendingModifications = false
}
}
if mi.BackupRetentionPeriod != nil && pm.BackupRetentionPeriod != nil {
if *mi.BackupRetentionPeriod == *pm.BackupRetentionPeriod {
pendingModifications = false
}
}
if mi.DBInstanceClass != nil && pm.DBInstanceClass != nil {
if *mi.DBInstanceClass == *pm.DBInstanceClass {
pendingModifications = false
}
}
if mi.AllocatedStorage != nil && pm.AllocatedStorage != nil {
if *mi.AllocatedStorage == *pm.AllocatedStorage {
pendingModifications = false
}
}
if mi.EngineVersion != nil && pm.EngineVersion != nil {
if *mi.EngineVersion == *pm.EngineVersion {
pendingModifications = false
}
}
if mi.MultiAZ != nil && pm.MultiAZ != nil {
if *mi.MultiAZ == *pm.MultiAZ {
pendingModifications = false
}
}
return pendingModifications
}

// verify postgres create config
func (p *PostgresProvider) buildRDSCreateStrategy(ctx context.Context, pg *v1alpha1.Postgres, ec2Svc ec2iface.EC2API, rdsCreateConfig *rds.CreateDBInstanceInput, postgresPassword string) error {
if rdsCreateConfig.DeletionProtection == nil {
Expand Down Expand Up @@ -653,7 +699,7 @@ func (p *PostgresProvider) configureRDSVpc(ctx context.Context, rdsSvc rdsiface.
}
}
if foundSubnet != nil {
logrus.Info(fmt.Sprintf("subnet group %s found", *foundSubnet.DBSubnetGroupName))
logrus.Infof("subnet group %s found", *foundSubnet.DBSubnetGroupName)
return nil
}

Expand Down Expand Up @@ -748,7 +794,7 @@ func (p *PostgresProvider) setPostgresServiceMaintenanceMetric(ctx context.Conte
return
}

logrus.Info(fmt.Sprintf("rds serviceupdates: %d available", len(output.PendingMaintenanceActions)))
logrus.Infof("rds serviceupdates: %d available", len(output.PendingMaintenanceActions))
for _, su := range output.PendingMaintenanceActions {
metricLabels := map[string]string{}

Expand All @@ -775,7 +821,7 @@ func (p *PostgresProvider) createRDSConnectionMetric(ctx context.Context, cr *v1
logrus.Infof("testing and exposing postgres connection metric for: %s", *instance.DBInstanceIdentifier)
clusterID, err := resources.GetClusterID(ctx, p.Client)
if err != nil {
logrus.Error(fmt.Sprintf("failed to get cluster id while exposing connection metric for %s", *instance.DBInstanceIdentifier))
logrus.Errorf("failed to get cluster id while exposing connection metric for %s", *instance.DBInstanceIdentifier)

}

Expand Down
166 changes: 161 additions & 5 deletions pkg/providers/aws/provider_postgres_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func buildDbInstanceDeletionProtection() []*rds.DBInstance {
}
}

func buildDBInstance(testID string) []*rds.DBInstance {
func buildAvailableDBInstance(testID string) []*rds.DBInstance {
return []*rds.DBInstance{
{
DBInstanceIdentifier: aws.String(testID),
Expand All @@ -237,6 +237,86 @@ func buildDBInstance(testID string) []*rds.DBInstance {
}
}

func buildPendingDBInstance(testID string) []*rds.DBInstance {
return []*rds.DBInstance{
{
DBInstanceIdentifier: aws.String(testID),
DBInstanceStatus: aws.String("pending"),
},
}
}

func buildAvailableCreateInput(testID string) *rds.CreateDBInstanceInput {
return &rds.CreateDBInstanceInput{
DBInstanceIdentifier: aws.String(testID),
DeletionProtection: aws.Bool(defaultAwsPostgresDeletionProtection),
Port: aws.Int64(defaultAwsPostgresPort),
BackupRetentionPeriod: aws.Int64(defaultAwsBackupRetentionPeriod),
DBInstanceClass: aws.String(defaultAwsDBInstanceClass),
PubliclyAccessible: aws.Bool(defaultAwsPubliclyAccessible),
AllocatedStorage: aws.Int64(defaultAwsAllocatedStorage),
EngineVersion: aws.String(defaultAwsEngineVersion),
MultiAZ: aws.Bool(true),
}
}

func buildRequiresModificationsCreateInput(testID string) *rds.CreateDBInstanceInput {
return &rds.CreateDBInstanceInput{
DBInstanceIdentifier: aws.String(testID),
DeletionProtection: aws.Bool(defaultAwsPostgresDeletionProtection),
Port: aws.Int64(123),
BackupRetentionPeriod: aws.Int64(defaultAwsBackupRetentionPeriod),
DBInstanceClass: aws.String(defaultAwsDBInstanceClass),
PubliclyAccessible: aws.Bool(defaultAwsPubliclyAccessible),
AllocatedStorage: aws.Int64(defaultAwsAllocatedStorage),
EngineVersion: aws.String(defaultAwsEngineVersion),
MultiAZ: aws.Bool(true),
}
}

func buildNewRequiresModificationsCreateInput(testID string) *rds.CreateDBInstanceInput {
return &rds.CreateDBInstanceInput{
DBInstanceIdentifier: aws.String(testID),
DeletionProtection: aws.Bool(defaultAwsPostgresDeletionProtection),
Port: aws.Int64(123),
BackupRetentionPeriod: aws.Int64(123),
DBInstanceClass: aws.String(defaultAwsDBInstanceClass),
PubliclyAccessible: aws.Bool(defaultAwsPubliclyAccessible),
AllocatedStorage: aws.Int64(defaultAwsAllocatedStorage),
EngineVersion: aws.String(defaultAwsEngineVersion),
MultiAZ: aws.Bool(true),
}
}

func buildPendingModifiedDBInstance(testID string) []*rds.DBInstance {
return []*rds.DBInstance{
{
DBInstanceIdentifier: aws.String(testID),
DBInstanceStatus: aws.String("available"),
AvailabilityZone: aws.String("test-availabilityZone"),
DBInstanceArn: aws.String("arn-test"),
DeletionProtection: aws.Bool(defaultAwsPostgresDeletionProtection),
MasterUsername: aws.String(defaultAwsPostgresUser),
DBName: aws.String(defaultAwsPostgresDatabase),
BackupRetentionPeriod: aws.Int64(defaultAwsBackupRetentionPeriod),
DBInstanceClass: aws.String(defaultAwsDBInstanceClass),
PubliclyAccessible: aws.Bool(defaultAwsPubliclyAccessible),
AllocatedStorage: aws.Int64(defaultAwsAllocatedStorage),
EngineVersion: aws.String(defaultAwsEngineVersion),
Engine: aws.String(defaultAwsEngine),
MultiAZ: aws.Bool(true),
Endpoint: &rds.Endpoint{
Address: aws.String("blob"),
HostedZoneId: aws.String("blog"),
Port: aws.Int64(defaultAwsPostgresPort),
},
PendingModifiedValues: &rds.PendingModifiedValues{
Port: aws.Int64(123),
},
},
}
}

func buildVpcs() []*ec2.Vpc {
return []*ec2.Vpc{
{
Expand Down Expand Up @@ -317,7 +397,7 @@ func TestAWSPostgresProvider_createPostgresInstance(t *testing.T) {
wantErr bool
}{
{
name: "test rds is created",
name: "test rds CreateReplicationGroup is called",
args: args{
rdsSvc: &mockRdsClient{dbInstances: []*rds.DBInstance{}},
ec2Svc: &mockEc2Client{vpcs: buildVpcs(), subnets: buildSubnets(), secGroups: buildSecurityGroups(secName), azs: buildAZ()},
Expand All @@ -338,7 +418,7 @@ func TestAWSPostgresProvider_createPostgresInstance(t *testing.T) {
{
name: "test rds is exists and is available",
args: args{
rdsSvc: &mockRdsClient{dbInstances: buildDBInstance(testIdentifier)},
rdsSvc: &mockRdsClient{dbInstances: buildAvailableDBInstance(testIdentifier)},
ec2Svc: &mockEc2Client{vpcs: buildVpcs(), subnets: buildSubnets(), secGroups: buildSecurityGroups(secName), azs: buildAZ()},
ctx: context.TODO(),
cr: buildTestPostgresCR(),
Expand All @@ -363,9 +443,9 @@ func TestAWSPostgresProvider_createPostgresInstance(t *testing.T) {
wantErr: false,
},
{
name: "test rds needs to be modified",
name: "test rds is exists and is not available",
args: args{
rdsSvc: &mockRdsClient{dbInstances: buildDBInstance(testIdentifier)},
rdsSvc: &mockRdsClient{dbInstances: buildPendingDBInstance(testIdentifier)},
ec2Svc: &mockEc2Client{vpcs: buildVpcs(), subnets: buildSubnets(), secGroups: buildSecurityGroups(secName), azs: buildAZ()},
ctx: context.TODO(),
cr: buildTestPostgresCR(),
Expand All @@ -383,6 +463,82 @@ func TestAWSPostgresProvider_createPostgresInstance(t *testing.T) {
want: nil,
wantErr: false,
},
{
name: "test rds exists and status is available and needs to be modified",
args: args{
rdsSvc: &mockRdsClient{dbInstances: buildAvailableDBInstance(testIdentifier)},
ec2Svc: &mockEc2Client{vpcs: buildVpcs(), subnets: buildSubnets(), secGroups: buildSecurityGroups(secName), azs: buildAZ()},
ctx: context.TODO(),
cr: buildTestPostgresCR(),
postgresCfg: buildRequiresModificationsCreateInput(testIdentifier),
},
fields: fields{
Client: fake.NewFakeClientWithScheme(scheme, buildTestPostgresCR(), builtTestCredSecret(), buildTestInfra()),
Logger: testLogger,
CredentialManager: nil,
ConfigManager: nil,
TCPPinger: buildMockConnectionTester(),
},
want: nil,
wantErr: false,
},
{
name: "test rds exists and status is available and does not need to be modified",
args: args{
rdsSvc: &mockRdsClient{dbInstances: buildAvailableDBInstance(testIdentifier)},
ec2Svc: &mockEc2Client{vpcs: buildVpcs(), subnets: buildSubnets(), secGroups: buildSecurityGroups(secName), azs: buildAZ()},
ctx: context.TODO(),
cr: buildTestPostgresCR(),
postgresCfg: buildAvailableCreateInput(testIdentifier),
},
fields: fields{
Client: fake.NewFakeClientWithScheme(scheme, buildTestPostgresCR(), builtTestCredSecret(), buildTestInfra()),
Logger: testLogger,
CredentialManager: nil,
ConfigManager: nil,
TCPPinger: buildMockConnectionTester(),
},
want: nil,
wantErr: false,
},
{
name: "test rds exists and status is available and needs to be modified but maintenance is pending",
args: args{
rdsSvc: &mockRdsClient{dbInstances: buildPendingModifiedDBInstance(testIdentifier)},
ec2Svc: &mockEc2Client{vpcs: buildVpcs(), subnets: buildSubnets(), secGroups: buildSecurityGroups(secName), azs: buildAZ()},
ctx: context.TODO(),
cr: buildTestPostgresCR(),
postgresCfg: buildRequiresModificationsCreateInput(testIdentifier),
},
fields: fields{
Client: fake.NewFakeClientWithScheme(scheme, buildTestPostgresCR(), builtTestCredSecret(), buildTestInfra()),
Logger: testLogger,
CredentialManager: nil,
ConfigManager: nil,
TCPPinger: buildMockConnectionTester(),
},
want: nil,
wantErr: false,
},
{
name: "test rds exists and status is available and needs to update pending maintenance",
args: args{
rdsSvc: &mockRdsClient{dbInstances: buildPendingModifiedDBInstance(testIdentifier)},
ec2Svc: &mockEc2Client{vpcs: buildVpcs(), subnets: buildSubnets(), secGroups: buildSecurityGroups(secName), azs: buildAZ()},
ctx: context.TODO(),
cr: buildTestPostgresCR(),
postgresCfg: buildNewRequiresModificationsCreateInput(testIdentifier),
},
fields: fields{
Client: fake.NewFakeClientWithScheme(scheme, buildTestPostgresCR(), builtTestCredSecret(), buildTestInfra()),
Logger: testLogger,
CredentialManager: nil,
ConfigManager: nil,
TCPPinger: buildMockConnectionTester(),
},
want: nil,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Loading

0 comments on commit 95edb68

Please sign in to comment.