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

Add abstract classes in container.py #93

Merged
merged 22 commits into from
Jun 26, 2023
Merged

Conversation

carlcsaposs-canonical
Copy link
Contributor

@carlcsaposs-canonical carlcsaposs-canonical commented Jun 9, 2023

Concrete classes in rock.py

As a result, workload.py and mysql_shell.py are now 100% shared between VM & K8s (VM charm subclasses workload.AuthenticatedWorkload to add support for unix sockets)

Future plans: add abstraction so that charm.py can be shared between VM & k8s

@carlcsaposs-canonical
Copy link
Contributor Author

here's a first draft of the socket subclass of workload.AuthenticatedWorkload: https://github.com/canonical/mysql-router-operator/blob/75bfa5148740b3e199518f76eaa117e35bfcfedc/src/socket_workload.py

Copy link
Collaborator

@paulomach paulomach left a comment

Choose a reason for hiding this comment

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

as said before, container.py ABC's looking mint

Copy link
Contributor

@taurus-forever taurus-forever left a comment

Choose a reason for hiding this comment

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

Nice! Tnx!

@carlcsaposs-canonical carlcsaposs-canonical merged commit fa570fb into main Jun 26, 2023
@carlcsaposs-canonical carlcsaposs-canonical deleted the abstraction-for-vm branch June 26, 2023 12:07
carlcsaposs-canonical added a commit that referenced this pull request Jun 26, 2023
Share most of charm.py between kubernetes & machine charm

Depends on
#93

---------

Co-authored-by: Paulo Machado <paulo.machado@canonical.com>
carlcsaposs-canonical added a commit to canonical/mysql-router-operator that referenced this pull request Jun 28, 2023
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 added a commit that referenced this pull request Aug 11, 2023
Leftover from when `_bootstrap_router` was responsible for updating the pebble layer

#93 moved the layer update out of `_bootstrap_router` to `Container.update_mysql_router_service`. In that PR, `_restart` should have removed its call to `_bootstrap_router` and only used `update_mysql_router_service`

MySQL Router should not be bootstrapped again when TLS is enabled or disabled—it should only be restarted
carlcsaposs-canonical added a commit that referenced this pull request Aug 11, 2023
Leftover from when `_bootstrap_router` was responsible for updating the
pebble layer

#93 moved the layer update out of `_bootstrap_router` to
`Container.update_mysql_router_service`. In that PR, `_restart` should
have removed its call to `_bootstrap_router` and only used
`update_mysql_router_service`

MySQL Router should not be bootstrapped again when TLS is enabled or
disabled—it should only be restarted
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.

4 participants