Refactor #51

carlcsaposs-canonical · 2023-04-13T18:48:27Z

Issue

Solution

Support multiple database provides relations
Remove all event deferrals
Avoid duplicating information from relation databags to peer databag; only use peer databag when necessary (i.e. single source of truth for state)
Handle database requires/provides events in a single handler
Bootstrap mysqlrouter independently of database provides relations
Encapsulate workload and mysql_shell to make future porting to VMs easier
Modify TLS implementation to fix a couple design flaws
Improve status messages & debug logging
Includes changes from Use charmed-mysql rock instead of the docker image #55 to use rock and remove run.sh script

carlcsaposs-canonical · 2023-04-13T18:49:42Z

~~With the exception of relations/tls.py~~, it may be easier to understand the changes by looking at the new files instead of the diff: https://github.com/canonical/mysql-router-k8s-operator/tree/refactor/src

paulomach

Looking nice. I like the thin relations code and usage of lib interface, definitely something to use more broadly.
Left some notes as a first pass

src/charm.py

src/relations/database_provides.py

src/charm.py

src/workload.py

src/charm.py

shayancanonical

I like the direction! - decoupling the relations and making the handlers idempotent like this PR does leads to cleaner code in my opinion

src/charm.py

src/relations/database_provides.py

src/workload.py

Needed for mysqlrouter refactoring: canonical/mysql-router-k8s-operator#51

carlcsaposs-canonical · 2023-04-20T20:20:42Z

Added support for multiple relations

TODO:

switch mysql router user to limited permissions user
switch from mysql connector to mysqlsh, move mysqlsh methods to new Python module (so that database_provides doesn't import database_requires)

Needed for mysqlrouter refactoring: canonical/mysql-router-k8s-operator#51

canonical/data-platform-workflows#37

This reverts commit 0f46562.

src/mysql_shell.py

.github/workflows/ci.yaml

src/workload.py

paulomach

IMO it's good to go once the test passes

Fixes #56 Depends on #51

DPE-1854, DPE-1910 Ported from canonical/mysql-router-k8s-operator#51 Changes from kubernetes PR: - No TLS - canonical/mysql-router-k8s-operator#93 - canonical/mysql-router-k8s-operator#97 Will re-implement legacy interface in separate PR

carlcsaposs-canonical requested review from paulomach, taurus-forever and shayancanonical April 13, 2023 18:50

paulomach reviewed Apr 17, 2023

View reviewed changes

shayancanonical reviewed Apr 17, 2023

View reviewed changes

src/charm.py Show resolved Hide resolved

src/relations/database_provides.py Outdated Show resolved Hide resolved

src/workload.py Outdated Show resolved Hide resolved

carlcsaposs-canonical added a commit to canonical/mysql-k8s-operator that referenced this pull request Apr 18, 2023

Grant permission to create databases to mysqlrouter user

7ff38c5

Needed for mysqlrouter refactoring: canonical/mysql-router-k8s-operator#51

carlcsaposs-canonical mentioned this pull request Apr 18, 2023

Grant ALL PRIVILEGES to mysqlrouter user canonical/mysql-k8s-operator#211

Merged

carlcsaposs-canonical force-pushed the refactor branch from ebd6c75 to c2043fc Compare April 18, 2023 19:30

carlcsaposs-canonical requested review from paulomach and shayancanonical April 20, 2023 20:25

carlcsaposs-canonical added a commit to canonical/mysql-k8s-operator that referenced this pull request Apr 21, 2023

Grant ALL PRIVILEGES to mysqlrouter user (#211)

a98b76c

Needed for mysqlrouter refactoring: canonical/mysql-router-k8s-operator#51

carlcsaposs-canonical added 17 commits April 25, 2023 17:35

Remove Foresight

db6a03e

canonical/data-platform-workflows#37

Refactor

4bf9636

update comment

360e2d2

fix

665d89f

Move TLS container methods to workload.py

3aac704

remove unused imports

7f96a60

Implement mysql connector methods

761d5b1

Use --version instead of -V

6bbd0cf

temporarily skip unit/lint for integration test

e6ef472

Use BlockedStatus for missing relation(s)

4830446

Add src/relations/__init__.py

82d28bd

Fix relations imports

279228d

Handle non-existent relations

81aa07f

wait until mysql router ready

bee49af

fixup! Handle non-existent relations

59bacc8

fixup! Use BlockedStatus for missing relation(s)

172e541

Handle non-existent service

853877e

carlcsaposs-canonical added 3 commits May 15, 2023 11:32

update comment

1b2d206

patch k8s service on install

2befc4c

Revert "Remove --conf-use-gr-notifications"

f9e1ea8

This reverts commit 0f46562.

carlcsaposs-canonical mentioned this pull request May 15, 2023

Set BlockedStatus if unsupported extra user role requested #61

Closed

carlcsaposs-canonical added 2 commits May 15, 2023 17:54

Update sans

ba1da60

Update comment

2270eae

carlcsaposs-canonical requested a review from shayancanonical May 15, 2023 18:17

revert workflow debug

3a0da55

carlcsaposs-canonical mentioned this pull request May 15, 2023

Use charmed-mysql rock instead of the docker image #55

Closed

carlcsaposs-canonical added 2 commits May 15, 2023 18:49

change database name

a443d54

update sans

78e9922

This was referenced May 15, 2023

Add set_secret and get_secret methods #62

Closed

Add Python 3.6 compatability #63

Closed

Add databag wrappers for data_interfaces relations #64

Closed

delgod approved these changes May 16, 2023

View reviewed changes

src/mysql_shell.py Show resolved Hide resolved

delgod requested a review from paulomach May 16, 2023 07:22

shayancanonical reviewed May 16, 2023

View reviewed changes

.github/workflows/ci.yaml Outdated Show resolved Hide resolved

src/workload.py Show resolved Hide resolved

remove router id from databag

177b580

paulomach approved these changes May 16, 2023

View reviewed changes

carlcsaposs-canonical mentioned this pull request May 16, 2023

Cleanup router metadata canonical/mysql-k8s-operator#224

Merged

shayancanonical approved these changes May 17, 2023

View reviewed changes

carlcsaposs-canonical added a commit that referenced this pull request May 17, 2023

Persist TLS private key across TLS relation created/broken

d15eacd

Fixes #56 Depends on #51

carlcsaposs-canonical mentioned this pull request May 17, 2023

Persist TLS private key across TLS relation created/broken #65

Merged

carlcsaposs-canonical merged commit 7113e60 into main May 18, 2023

carlcsaposs-canonical deleted the refactor branch May 18, 2023 11:18

carlcsaposs-canonical added a commit that referenced this pull request May 18, 2023

Persist TLS private key across TLS relation created/broken

010c57d

Fixes #56 Depends on #51

This was referenced May 18, 2023

Database configuration is not shared with relating application #54

Closed

MySQL-router 8.0.32 pymysql.err.OperationalError: caching sha2: Unknown packet for public key: b'-' #52

Closed

carlcsaposs-canonical mentioned this pull request Jun 21, 2023

Refactor canonical/mysql-router-operator#44

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor #51

Refactor #51

carlcsaposs-canonical commented Apr 13, 2023 •

edited

Loading

carlcsaposs-canonical commented Apr 13, 2023 •

edited

Loading

paulomach left a comment

shayancanonical left a comment

carlcsaposs-canonical commented Apr 20, 2023

paulomach left a comment

Refactor #51

Refactor #51

Conversation

carlcsaposs-canonical commented Apr 13, 2023 • edited Loading

Issue

Solution

carlcsaposs-canonical commented Apr 13, 2023 • edited Loading

paulomach left a comment

Choose a reason for hiding this comment

shayancanonical left a comment

Choose a reason for hiding this comment

carlcsaposs-canonical commented Apr 20, 2023

paulomach left a comment

Choose a reason for hiding this comment

carlcsaposs-canonical commented Apr 13, 2023 •

edited

Loading

carlcsaposs-canonical commented Apr 13, 2023 •

edited

Loading