Skip to content

Commit

Permalink
[DBInstance] Adding support to convert from non-cdb to cdb for Oracle…
Browse files Browse the repository at this point in the history 
… engines (#489)

* [DBInstance] Adding support to convert from non-cdb to cdb for Oracle engines

* [DBInstance] Add pending engine check for oracle non-cdb to cdb conversion

* [DBInstance] Ensure Engine is set for oracle non cdb to cdb conversion check

* [DBInstance] Handle exceptions from fetchDBInstance in UpdateHandler

* [DBInstance] Fix build failure

---------

Co-authored-by: Mackenzie Marshall <[email protected]>
  • Loading branch information
@elnaggar-amr @mackmars
elnaggar-amr and mackmars authored Dec 18, 2023
1 parent 8094ccb commit 6ad7b0d
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 14 deletions.
1 change: 1 addition & 0 deletions aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,7 @@ boolean isNoPendingChanges(final DBInstance dbInstance) {
pending.backupRetentionPeriod() == null &&
pending.multiAZ() == null &&
pending.engineVersion() == null &&
pending.engine() == null &&
pending.iops() == null &&
pending.dbInstanceIdentifier() == null &&
pending.licenseModel() == null &&
Expand Down
2 changes: 2 additions & 0 deletions aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/Translator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ public static ModifyDbInstanceRequest modifyDbInstanceRequestV12(
.dbInstanceIdentifier(desiredModel.getDBInstanceIdentifier())
.dbParameterGroupName(diff(previousModel.getDBParameterGroupName(), desiredModel.getDBParameterGroupName()))
.dbSecurityGroups(diff(previousModel.getDBSecurityGroups(), desiredModel.getDBSecurityGroups()))
.engine(diff(previousModel.getEngine(), desiredModel.getEngine()))
.engineVersion(diff(previousModel.getEngineVersion(), desiredModel.getEngineVersion()))
.masterUserPassword(diff(previousModel.getMasterUserPassword(), desiredModel.getMasterUserPassword()))
.multiAZ(diff(previousModel.getMultiAZ(), desiredModel.getMultiAZ()))
Expand Down Expand Up @@ -453,6 +454,7 @@ public static ModifyDbInstanceRequest modifyDbInstanceRequest(
.domainIAMRoleName(diff(previousModel.getDomainIAMRoleName(), desiredModel.getDomainIAMRoleName()))
.domainOu(desiredModel.getDomainOu())
.enableIAMDatabaseAuthentication(diff(previousModel.getEnableIAMDatabaseAuthentication(), desiredModel.getEnableIAMDatabaseAuthentication()))
.engine(diff(previousModel.getEngine(), desiredModel.getEngine()))
.licenseModel(diff(previousModel.getLicenseModel(), desiredModel.getLicenseModel()))
.masterUserPassword(diff(previousModel.getMasterUserPassword(), desiredModel.getMasterUserPassword()))
.maxAllocatedStorage(diff(previousModel.getMaxAllocatedStorage(), desiredModel.getMaxAllocatedStorage()))
Expand Down
18 changes: 15 additions & 3 deletions aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/UpdateHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,21 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
final VersionedProxyClient<RdsClient> rdsProxyClient,
final VersionedProxyClient<Ec2Client> ec2ProxyClient
) {
if (!ImmutabilityHelper.isChangeMutable(request.getPreviousResourceState(), request.getDesiredResourceState())) {
final ProxyClient<RdsClient> rdsClient = rdsProxyClient.defaultClient();
DBInstance instance;

try {
instance = StringUtils.isNullOrEmpty(request.getPreviousResourceState().getEngine()) ?
fetchDBInstance(rdsClient, request.getPreviousResourceState()) : null;
} catch (Exception ex) {
return Commons.handleException(
ProgressEvent.progress(request.getPreviousResourceState(), callbackContext),
ex,
DEFAULT_DB_INSTANCE_ERROR_RULE_SET
);
}

if (!ImmutabilityHelper.isChangeMutable(request.getPreviousResourceState(), request.getDesiredResourceState(), instance)) {
return ProgressEvent.failed(
request.getDesiredResourceState(),
callbackContext,
Expand All @@ -71,8 +85,6 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
return handleResourceDrift(proxy, request, callbackContext, rdsProxyClient, ec2ProxyClient, logger);
}

final ProxyClient<RdsClient> rdsClient = rdsProxyClient.defaultClient();

final Tagging.TagSet previousTags = Tagging.TagSet.builder()
.systemTags(Tagging.translateTagsToSdk(request.getPreviousSystemTags()))
.stackTags(Tagging.translateTagsToSdk(request.getPreviousResourceTags()))
Expand Down
25 changes: 20 additions & 5 deletions aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/util/ImmutabilityHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@
import com.amazonaws.util.StringUtils;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;

import software.amazon.awssdk.services.rds.model.DBInstance;
import software.amazon.rds.dbinstance.ResourceModel;

public final class ImmutabilityHelper {

private static final String ORACLE_SE = "oracle-se";
private static final String ORACLE_SE1 = "oracle-se1";
private static final String ORACLE_SE2 = "oracle-se2";
private static final String ORACLE_SE2_CDB = "oracle-se2-cdb";
private static final String ORACLE_EE = "oracle-ee";
private static final String ORACLE_EE_CDB = "oracle-ee-cdb";

private static final String AURORA = "aurora";
private static final String AURORA_MYSQL = "aurora-mysql";
Expand All @@ -29,17 +34,26 @@ static boolean isUpgradeToOracleSE2(final ResourceModel previous, final Resource
ORACLE_SE2.equalsIgnoreCase(desired.getEngine());
}

static boolean isOracleConvertToCDB(final String previous, final ResourceModel desired) {
return previous != null &&
(
(ORACLE_EE.equalsIgnoreCase(previous) && ORACLE_EE_CDB.equalsIgnoreCase(desired.getEngine())) ||
(ORACLE_SE2.equalsIgnoreCase(previous) && ORACLE_SE2_CDB.equalsIgnoreCase(desired.getEngine()))
);
}

static boolean isUpgradeToAuroraMySQL(final ResourceModel previous, final ResourceModel desired) {
return previous.getEngine() != null &&
desired.getEngine() != null &&
previous.getEngine().equals(AURORA) &&
desired.getEngine().equals(AURORA_MYSQL);
}

static boolean isEngineMutable(final ResourceModel previous, final ResourceModel desired) {
return Objects.equal(previous.getEngine(), desired.getEngine()) ||
static boolean isEngineMutable(final ResourceModel previous, final ResourceModel desired, final String currentEngine) {
return Objects.equal(currentEngine, desired.getEngine()) ||
isUpgradeToAuroraMySQL(previous, desired) ||
isUpgradeToOracleSE2(previous, desired);
isUpgradeToOracleSE2(previous, desired) ||
isOracleConvertToCDB(currentEngine, desired);
}

static boolean isPerformanceInsightsKMSKeyIdMutable(final ResourceModel previous, final ResourceModel desired) {
Expand Down Expand Up @@ -93,8 +107,9 @@ public static boolean isSourceDbiResourceIdMutable(final ResourceModel previous,
StringUtils.isNullOrEmpty(desired.getSourceDbiResourceId());
}

public static boolean isChangeMutable(final ResourceModel previous, final ResourceModel desired) {
return isEngineMutable(previous, desired) &&
public static boolean isChangeMutable(final ResourceModel previous, final ResourceModel desired, final DBInstance instance) {
final String currentEngine = (!StringUtils.isNullOrEmpty(previous.getEngine()) || instance == null) ? previous.getEngine() : instance.engine();
return isEngineMutable(previous, desired, currentEngine) &&
isPerformanceInsightsKMSKeyIdMutable(previous, desired) &&
isAvailabilityZoneChangeMutable(previous, desired) &&
isSourceDBInstanceIdentifierMutable(previous, desired) &&
Expand Down
26 changes: 20 additions & 6 deletions ...-dbinstance/src/test/java/software/amazon/rds/dbinstance/util/ImmutabilityHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ public void test_isUpgradeToOracleSE2() {
}
}

@Test
public void test_isOracleConvertToCDB() {
final List<ResourceModelTestCase> tests = Arrays.asList(
ResourceModelTestCase.builder().previous(ResourceModel.builder().engine("oracle-ee").build()).desired(ResourceModel.builder().engine("oracle-ee-cdb").build()).expect(true).build(),
ResourceModelTestCase.builder().previous(ResourceModel.builder().engine("oracle-se2").build()).desired(ResourceModel.builder().engine("oracle-se2-cdb").build()).expect(true).build(),
ResourceModelTestCase.builder().previous(ResourceModel.builder().engine("oracle-ee").build()).desired(ResourceModel.builder().engine("oracle-se2-cdb").build()).expect(false).build(),
ResourceModelTestCase.builder().previous(ResourceModel.builder().engine("oracle-se2").build()).desired(ResourceModel.builder().engine("oracle-ee-cdb").build()).expect(false).build(),
ResourceModelTestCase.builder().previous(ResourceModel.builder().engine("oracle-se2").build()).desired(ResourceModel.builder().engine("oracle-se2").build()).expect(false).build()
);
for (final ResourceModelTestCase test : tests) {
assertThat(ImmutabilityHelper.isOracleConvertToCDB(test.previous.getEngine(), test.desired)).isEqualTo(test.expect);
}
}

@Test
public void test_isUpgradeToAuroraMySQL() {
final List<EngineTestCase> tests = Arrays.asList(
Expand Down Expand Up @@ -122,7 +136,7 @@ public void test_isEngineMutable() {
.build()
);
for (final ResourceModelTestCase test : tests) {
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired)).isEqualTo(test.expect);
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired, null)).isEqualTo(test.expect);
}
}

Expand Down Expand Up @@ -150,7 +164,7 @@ public void test_isAvailabilityZoneChangeMutable() {
.expect(true)
.build());
for (final ResourceModelTestCase test : tests) {
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired)).isEqualTo(test.expect);
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired, null)).isEqualTo(test.expect);
}
}

Expand Down Expand Up @@ -178,7 +192,7 @@ public void test_isSourceDBIdentifierMutable() {
.expect(false)
.build());
for (final ResourceModelTestCase test : tests) {
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired)).isEqualTo(test.expect);
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired, null)).isEqualTo(test.expect);
}
}

Expand Down Expand Up @@ -206,7 +220,7 @@ public void test_isDBSnapshotIdentifierMutable() {
.expect(false)
.build());
for (final ResourceModelTestCase test : tests) {
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired)).isEqualTo(test.expect);
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired, null)).isEqualTo(test.expect);
}
}

Expand Down Expand Up @@ -234,7 +248,7 @@ public void test_isDBClusterSnapshotIdentifierMutable() {
.expect(false)
.build());
for (final ResourceModelTestCase test : tests) {
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired)).isEqualTo(test.expect);
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired, null)).isEqualTo(test.expect);
}
}

Expand Down Expand Up @@ -288,7 +302,7 @@ public void test_isSourceDBClusterIdentifierMutable() {
.expect(false)
.build());
for (final ResourceModelTestCase test : tests) {
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired)).isEqualTo(test.expect);
assertThat(ImmutabilityHelper.isChangeMutable(test.previous, test.desired, null)).isEqualTo(test.expect);
}
}
}

0 comments on commit 6ad7b0d

Please sign in to comment.