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

Make a tempfile while backup is running #731

Merged
merged 6 commits into from
Apr 15, 2024
Merged

Conversation

rzvoncek
Copy link
Contributor

@rzvoncek rzvoncek commented Mar 25, 2024

Fixes k8ssandra/k8ssandra-operator#1256
Fixes k8ssandra/k8ssandra-operator#1255

This PR adds the feature of reloading Medusa's configuration when running within the k8ssandra-operator.

The implementation has two parts:

  • Medusa will touch a file in /tmp to indicate a backup is running. This is easily detectable from a shell w/o invoking anything special (another Medusa instance included).
  • The entry point of the Docker image we use in k8ssandra-operator will start Medusa and then proceed to watch the medusa.ini file for changes by recalculating its digest every 5 seconds. If the digest changes, it'll check for running backups. If there are none, it'll kill the Medusa's gRPC process and exit, thus killing the entire container.

The k8ssandra-operator will then restart the container, which will bring in the updated config.

(The debian build is failing for a known reason that gets fixed in another PR).

Copy link

codecov bot commented Mar 25, 2024

Codecov Report

Attention: Patch coverage is 63.82979% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 79.73%. Comparing base (7be8cc1) to head (6a755bf).
Report is 3 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   80.02%   79.73%   -0.30%     
==========================================
  Files          52       52              
  Lines        4751     4796      +45     
  Branches      970      977       +7     
==========================================
+ Hits         3802     3824      +22     
- Misses        920      943      +23     
  Partials       29       29              
Files Coverage Δ
medusa/backup_manager.py 85.82% <100.00%> (+0.74%) ⬆️
medusa/backup_node.py 86.40% <66.66%> (-0.60%) ⬇️
medusa/service/grpc/server.py 78.52% <45.45%> (-2.38%) ⬇️
medusa/utils.py 74.00% <68.96%> (-6.96%) ⬇️

... and 2 files with indirect coverage changes

@rzvoncek rzvoncek marked this pull request as ready for review March 27, 2024 09:52
medusa/utils.py Outdated
class MedusaTempFile(object):

_tempfile = None
_tempfile_path = '/tmp/medusa_backup_in_progress'
Copy link
Contributor

Choose a reason for hiding this comment

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

Todo: for better cross platform compatibility, could you use tempfile.gettempdir() instead of hardcoding /tmp ?

medusa/utils.py Outdated

def exists(self):
if self._tempfile is not None:
return pathlib.Path(self._tempfile_path).exists()
Copy link
Contributor

Choose a reason for hiding this comment

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

Todo: let's assume an exception could be thrown here. We should display a warning and return False.

@adejanovski
Copy link
Contributor

I don't know if it's related to changes in this PR but I'm seeing the following errors in the logs:

[2024-03-29 09:42:13,612] INFO: Recording async backup information.
[2024-03-29 09:42:13,612] ERROR: Failed to record backup information executed in async manner. Error: too many values to unpack (expected 8)

and:

[2024-03-29 09:32:12,494] INFO: Recording async backup information.
[2024-03-29 09:32:12,495] ERROR: Exception in callback record_backup_info(<Future cancelled>) at /home/cassandra/medusa/service/grpc/server.py:346
handle: <Handle record_backup_info(<Future cancelled>) at /home/cassandra/medusa/service/grpc/server.py:346>
Traceback (most recent call last):
  File "/usr/lib/python3.10/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/home/cassandra/medusa/service/grpc/server.py", line 349, in record_backup_info
    if future.exception():
asyncio.exceptions.CancelledError: Removal of backup requested. Cancelling backup Name: medusa-backup8 with done state: False

@rzvoncek
Copy link
Contributor Author

rzvoncek commented Apr 1, 2024

Thanks for catching those, I've pushed a fix.

Copy link

sonarcloud bot commented Apr 1, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants