-
Notifications
You must be signed in to change notification settings - Fork 14
Feat/migrate pxc to mgr #123
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a comprehensive MySQL 5.7 → 8.0 migration guide and three automation scripts implementing a mysqldump-streaming workflow: prereqs, environment checks, schema/charset/GTID/auth validation, user/privilege migration, streaming cutover, verification, rollback, troubleshooting, and an appendix with scripts and sample outputs. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Engineer
participant Secrets as Secret Store
participant Scripts as Migration Scripts (bash)
participant SourceDB as MySQL 5.7
participant TargetDB as MySQL 8.0
Engineer->>Secrets: retrieve credentials
Engineer->>Scripts: run 00-pre-migration-check.sh
Scripts->>SourceDB: inspect schema, users, GTID, charsets
Scripts-->>Engineer: report compatibility issues
Engineer->>Scripts: run 01-migrate-all.sh (streaming mysqldump)
Scripts->>SourceDB: stream data (mysqldump)
Scripts->>TargetDB: apply stream (mysql import)
Scripts->>TargetDB: adjust auth plugins and GTID handling
Engineer->>Scripts: run 02-verify-migration.sh
Scripts-->>Engineer: verification results (objects, users, row counts)
Engineer->>TargetDB: perform cutover / monitoring
Note over SourceDB,TargetDB: Optional rollback if verification fails
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@docs/en/solutions/How_to_Migrate_MySQL_57_to_80.md`:
- Around line 48-56: The markdown tables in this document use the compact pipe
style and violate MD060; update each table (e.g., the test case table that
contains rows like "TC-01...") to the spaced pipe style by adding a single space
before and after each '|' delimiter in header, separator, and data rows so pipes
are surrounded by spaces (apply the same change for all other tables flagged
such as those around the ranges referenced in the review comment).
- Around line 223-248: The HA guidance and the example YAML disagree: the
bullets recommend 3 replicas but the Mysql spec under spec.mgr sets members: 1
(and router replicas are 1 elsewhere). Either update the YAML to demonstrate HA
by setting spec.mgr.members: 3 and ensure any router replica settings are also
increased to 3, or explicitly state in the bullet list that the provided YAML is
a single-node test/staging example and not suitable for production; update
references to spec.mgr, members, mgr, and any router replica settings to keep
text and YAML consistent.
- Line 17: The sentence "MySQL 5.7 End of Life (EOL) is approaching in October
2023" should be changed to past tense and reflect that the EOL date has already
passed; locate that exact sentence in the doc and replace it with wording such
as "MySQL 5.7 reached End of Life (EOL) in October 2023" and adjust surrounding
verbs (e.g., "must upgrade" → "should upgrade" or "need to have upgraded") to
maintain tense consistency in the paragraph and keep the rest of the migration
considerations unchanged.
- Around line 585-611: The bolded lines "Method 1: Using mysqldump
(Recommended)" and "Method 2: Manual Export with CREATE USER Statements" are
styled as emphasis but should be actual headings for TOC/navigation; replace the
bold markup with appropriate Markdown heading syntax (e.g., "## Method 1: Using
mysqldump (Recommended)" and "## Method 2: Manual Export with CREATE USER
Statements") so the sections are recognized as headings and included in the
document structure, and ensure any adjacent inline bold text remains unchanged.
- Around line 1216-1274: The fenced code blocks showing MySQL error outputs
(e.g., the blocks containing "ERROR 3546 (HY000) ...", "ERROR 1366 (HY000):
Incorrect string value", and "ERROR 1449 (HY000): The user specified as a
definer ...") are missing a language tag and trigger MD040; update each
triple-backtick fence to include a language (use "text") so the blocks become
fenced as ```text, ensuring all error output blocks in this document (including
the authentication plugin error block) are annotated consistently.
- Around line 871-899: The checks that query mysql.proc will fail on MySQL 8.0;
update the SELECTs used to populate SOURCE_COUNT and TARGET_COUNT (the kubectl
exec mysql -e "...") to query information_schema.ROUTINES instead of mysql.proc
and use ROUTINE_SCHEMA = '${db}' and ROUTINE_TYPE = 'PROCEDURE' (for the Stored
Procedures block) or 'FUNCTION' (for the Stored Functions block); keep the
surrounding shell variables/logic (TOTAL_CHECKS, SOURCE_COUNT, TARGET_COUNT, db,
SOURCE_NAME/TARGET_NAME invocations) and only change the SQL string in both the
source and target kubectl exec calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/en/solutions/How_to_Migrate_MySQL_57_to_80.md`:
- Line 634: Replace the bolded authentication option titles with proper markdown
headings so they appear in the TOC: find the bold strings "**Option 1: Change
Users to mysql_native_password (Migration Compatibility)**" and the other two
bolded option lines (the entries at the same pattern) and convert them to
heading syntax (e.g., "### Option 1: Change Users to mysql_native_password
(Migration Compatibility)") ensuring consistent heading level for all three
occurrences so they are included in the document's table of contents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/en/solutions/How_to_Migrate_MySQL_57_to_80.md`:
- Around line 1127-1138: Replace the bold subsection labels with proper Markdown
headings so they appear in the TOC: change "**Script fails with \"Cannot connect
to Kubernetes cluster\"**", "**Script fails with \"Source cluster not
found\"**", and "**Migration fails for specific database**" to heading syntax
(e.g., "#### Script fails with \"...\"") and ensure the following fenced code
blocks include language markers like ```bash so the commands render correctly;
update the three subsections in the document to use the heading tokens and
proper fenced code blocks accordingly.
- Around line 904-1111: Add the missing code-fence language tag "text" to the
expected-output blocks shown under the Pre-Migration Check, the "Expected
Output" section in "Script 2: Complete Migration" (the large migration run
sample) and the "Expected Output" section in "Script 3: Comprehensive
Verification" so each triple-backtick fence becomes ```text; locate these blocks
by the headings "MySQL 5.7 to 8.0 Pre-Migration Check", "MySQL 5.7 to 8.0
Migration" (Script 2) and "MySQL 5.7 to 8.0 Migration Verification" (Script 3)
and update their opening fences only.
- Around line 76-80: The table and prerequisites contain broken internal link
fragments; update the links in the table rows and prerequisites so they match
the actual heading IDs: replace `#schema-compatibility-analysis` with
`#step-2-schema-compatibility-analysis` (heading "Step 2: Schema Compatibility
Analysis") and replace `#character-set-and-collation-analysis` with
`#step-3-character-set-and-collation-analysis` (heading "Step 3: Character Set
and Collation Analysis"); for the `#migration-procedure` link either create a
matching heading (e.g., add "### Migration Procedure" so the fragment
`#migration-procedure` resolves) or change that link to point to an appropriate
existing section (e.g., the actual migration step heading) and update all
instances accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/en/solutions/How_to_Migrate_MySQL_57_to_80.md`:
- Line 609: The fenced code blocks showing raw error messages are missing
language identifiers; update each error-message fence in
How_to_Migrate_MySQL_57_to_80.md (the three/four error example blocks around the
provided diff locations) to include the `text` language tag (i.e., change ``` to
```text) so rendering/MD040 is satisfied; locate the specific fences containing
lines like "ERROR 3546 (HY000) at line XX: Cannot update GTID_PURGED..." and the
other two/three error examples and prepend `text` to their opening
triple-backtick markers.
- Line 1075: Replace the troubleshooting subsection titles that are currently
plain bold text (e.g., '**Script fails with "Cannot connect to Kubernetes
cluster"**', '**Script fails with "Source cluster not found"**', '**Migration
fails for specific database**') with proper Markdown heading syntax (for example
prepend '#### ' to each title) so they become real headings and appear in the
table of contents; ensure the surrounding code blocks remain fenced and intact
and apply the same change to the other occurrences noted (around the lines
corresponding to the other two titles).
- Line 852: Update the three markdown fenced output blocks that currently start
with ``` (the ones whose content begins with the header
"======================================== MySQL 5.7 to 8.0 Pre-Migration Check
========================================") to include the language tag "text" on
the opening fence (i.e. change ``` to ```text) for each occurrence so the
examples are consistently marked as plain-text output.
- Around line 76-80: Update the broken internal link fragments in the table:
replace the fragment "#schema-compatibility-analysis" with
"#step-1-schema-compatibility-analysis" (matches heading "### Step 1: Schema
Compatibility Analysis") and replace "#character-set-and-collation-analysis"
with "#step-2-character-set-and-collation-analysis" (matches heading "### Step
2: Character Set and Collation Analysis"); for "#migration-procedure" locate the
actual "Migration Procedure" heading in the document and use its real generated
fragment slug (replace the table link with that exact slug); apply these same
three corrections for the other occurrences noted in the comment.
🧹 Nitpick comments (1)
docs/en/solutions/How_to_Migrate_MySQL_57_to_80.md (1)
119-127: Consider documenting secure password handling alternatives.Throughout the guide and scripts, MySQL passwords are passed via the
-p${MYSQL_PASSWORD}command-line flag. While functional in containerized environments, this approach has security implications:
- Passwords visible in process lists (
kubectl execcommand history)- Shell history may retain plaintext passwords
- Pod security policies may log command arguments
While the current approach is acceptable for internal scripts, consider adding a security note or documenting the
MYSQL_PWDenvironment variable alternative for production use:# More secure alternative using environment variable kubectl exec ${TARGET_NAME}-0 -n ${TARGET_NAMESPACE} -c mysql -- \ bash -c "MYSQL_PWD='${MYSQL_PASSWORD}' mysql -uroot -e 'SELECT 1;'"This is a minor improvement given the controlled ACP environment, but worth mentioning in the security-conscious sections.
Also applies to: 1215-1217, 1546-1552
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.