feat: parameterize logs location and make etcd relation optional#445
Merged
patriciareinoso merged 17 commits intocanonical:DPE-9349-rolling-ops-maintenancefrom Apr 24, 2026
Conversation
* patch: etcd rolling ops version * first working version * fix format * fix linting * add tenacity to integration test * remove unnecessary logs * add dataplatform as reviewes * rename and add integration tests * linting and rebase * first part of comments * more comments answered * more comments answered * fix linting job * fix UT * mark tests as only k8s * fix integration tests * use charmlibs apt * remove sans dns * add dependencies to .toml * add uv lock * add wait in itnegration tests * increate timeout * increase log count * unlimited debug-log * comments review * fix paths * migrate v1 * fix integration tests * fix integration tests * add lock and integration tests * unify operations * add tenacity * draft * fallback implementation * add sync lock and state * feat: advanced rolling ops using etcd (canonical#364) ## Context All the code on this PR is new This implementation is based on [DA241 Spec](https://docs.google.com/document/d/1ez4h6vOOyHy5mu6xDblcBt8PPAtMe7MUp75MtgG1sns/edit?tab=t.0) - The content of `charmlibs/advanced-rollingops/src/charmlibs/advanced_rollingops/_dp_interfaces_v1.py` belongs to another library that is currently being migrated to charmlibs so you can ignore it for now. ## Summary This PR is the first part of the implementation of advanced rolling ops at cluster level. This PR includes: - Management of the client certificate used to connect to etcd - The leader unit creates a self-signed certificate with live of 50 years - Share the certificate with the other units using a peer relation - Implementation of the integration with etcd - Share the mtls certificate - Observe the `resource_created` event - Observe the `endpoints_changed` event - Management of a env file needed to connecto etcd via `etcdctl` This PR does not implement the locking mechanism. In here we only test that we can connect to etcd from each unit. ## Current workflow: 1. The unit make a request 2. A new background process is spawn 3. The background process dispatches a Juju hook 4. The unit observes that hook 5. The unit writes and read a value in etcd 6. If the unit was able to connect to etcd, it executes the "restart" function. This is a very simplified workflow to be able to test that the units from different apps can reach etcd. ## To do - Implement the actual locking mechanism - Figure out how to properly install etcdctl * feat: migrate rollingops v1 from charm-rolling-ops repo (canonical#415) * define syn lock backend * fix merge * clean up * fix peer integration tests * fix integration tests * fix integration tests * docstrings * add update status handled and improve integration tests * general cleanup
Signed-off-by: Patricia Reinoso <patricia.reinoso@canonical.com>
patriciareinoso
requested review from
Gu1nness,
Mehdi-Bendriss and
sinclert-canonical
patriciareinoso
changed the title
patriciareinoso
changed the title
sinclert-canonical
left a comment
sinclert-canonical
left a comment
There was a problem hiding this comment.
🧡 Thanks for the quick iteration on MySQL's feedback. This is looking great.
Please consider that, even if making the ETCD relation optional is the greatest requirement lift you could make for library consumers (thanks again!), it is only a piece of a greater pattern:
This library is, rightfully, offering a way to deal with both:
- Intra-application rolling-ops (thought the peer relation).
- Inter-application rolling-ops (thought the ETCD relation).
The library is also, rightfully, offering a way to emit / react to both:
- Synchronous events (less common within charms).
- Asynchronous events (most common within charms).
In both scenarios, library consumers may want to deal with one, the other, or both. Therefore, the big pattern here would be to make all RollingOpsManager arguments that deal with these functionalities, optional, so each consumer can tweak the instantiation to their particular needs.
Gu1nness
reviewed
Apr 24, 2026
Contributor
Gu1nness
left a comment
Gu1nness
left a comment
There was a problem hiding this comment.
Mostly moving from str to pathops.LocalPath IMHO.
Otherwise, all good.
About the is_ready is it is_waiting that you implemented ? Do that solve our problem ?
sinclert-canonical
approved these changes
Apr 24, 2026
sinclert-canonical
left a comment
sinclert-canonical
left a comment
There was a problem hiding this comment.
Thanks for your tireless efforts. This is looking great!
Gu1nness
approved these changes
Apr 24, 2026
Mehdi-Bendriss
approved these changes
Apr 24, 2026
patriciareinoso
merged commit d154e98
into
canonical:DPE-9349-rolling-ops-maintenance
42 of 44 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR
base_dirwhere all the files related to rolling ops will be written.We write:
- Log file for the background proces
- TLS certificates
- ETCDCTL configuration file
The new parameter is optional, if not provided we default to
/var/lib/rollingopsMakes the
etcd_relation_nameandcluter_idoptional parameters. If not provided they default toNone. In this case we would only be using the peer backend.Add a
is_waitingfunction.There are no functional changes in:
rollingops/src/charmlibs/rollingops/etcd/_certificates.pyrollingops/src/charmlibs/rollingops/etcd/_etcdctl.pyBut the diff is not nicely shown in github. I changed those modules to replace static functions with objects