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

Feature/add operational status to migrated rds nstances #192

Merged

Conversation

abodhekar
Copy link
Contributor

@abodhekar abodhekar commented Oct 9, 2023

This is to -

  1. Add "OPERATIONAL_STATUS" as "INACTIVE" to those RDS Instances which were migrated/upgraded and now aren't operational.
  2. Also updated similar Tags for associated DBClsuer, DBParamGroups and DMClusterParamGroup
  3. Once this tag is added and verified (only for DBInstance), We are deleting associated crossplane DBInstance.

@abodhekar abodhekar requested review from bjeevan-ib, ssuman2-infoblox and abalaven and removed request for ssuman2-infoblox October 9, 2023 08:26
@abodhekar abodhekar marked this pull request as ready for review October 9, 2023 08:26
@abodhekar abodhekar requested a review from abalaven October 9, 2023 18:41
abalaven
abalaven previously approved these changes Oct 9, 2023
@abodhekar abodhekar force-pushed the feature/add-OperationalStatus-to-migrated-RDS-nstances branch from 02d44ef to 94ed0a4 Compare October 13, 2023 12:39
…er related resources

allowing post actions after migration only for once

binding tagging component with DBClaim reconcile cycle

changes to handle tagging and deletion for DBCluster, DBCluserParamGroup etc

updating comments. Also, deleted resources if tags updation could not be verified within defined time

refactoring

code refactored
@abodhekar abodhekar force-pushed the feature/add-OperationalStatus-to-migrated-RDS-nstances branch from 94ed0a4 to 28df458 Compare October 13, 2023 12:42
Copy link
Contributor

@bjeevan-ib bjeevan-ib left a comment

Choose a reason for hiding this comment

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

We also need to revisit the deleteworkflow - maybe we can treat it as a separate unit of work

@@ -80,6 +80,9 @@ const (
// DebugLevel is used to set V level to 1 as suggested by official docs
// https://github.com/kubernetes-sigs/controller-runtime/blob/main/TMP-LOGGING.md
DebugLevel = 1

operationalStatusTagKey string = "OPERATIONAL_STATUS"
Copy link
Contributor

Choose a reason for hiding this comment

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

the key and value need to match the style of tags we are using right now. if I remember right, devops goes with initcap with kabab case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bjeevan-ib - done

@@ -220,6 +231,8 @@ func (r *DatabaseClaimReconciler) getMode(dbClaim *persistancev1.DatabaseClaim)
return M_UpgradeDBInProgress

}
} else if dbClaim.Status.OldDB.DbState == persistancev1.PostMigrationInProgress {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this not be made a simple check right in the top - by checking for oldDB being not null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bjeevan-ib - done

@abodhekar abodhekar requested a review from bjeevan-ib October 18, 2023 07:14
@bjeevan-ib bjeevan-ib merged commit 315c655 into main Nov 1, 2023
2 checks passed
@bjeevan-ib bjeevan-ib deleted the feature/add-OperationalStatus-to-migrated-RDS-nstances branch November 1, 2023 17:02
abodhekar added a commit that referenced this pull request Nov 1, 2023
bjeevan-ib pushed a commit that referenced this pull request Nov 1, 2023
@abodhekar abodhekar restored the feature/add-OperationalStatus-to-migrated-RDS-nstances branch November 2, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants