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 cronjob to purge backups #1167

Merged
merged 8 commits into from
Jan 19, 2024
Merged

Add cronjob to purge backups #1167

merged 8 commits into from
Jan 19, 2024

Conversation

emerkle826
Copy link
Contributor

@emerkle826 emerkle826 commented Jan 10, 2024

What this PR does:
Creates a CronJob that purges Medusa backups
Which issue(s) this PR fixes:
Fixes #1154

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

Copy link

codecov bot commented Jan 10, 2024

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (f96ea1c) 57.04% compared to head (d67a32c) 57.23%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1167      +/-   ##
==========================================
+ Coverage   57.04%   57.23%   +0.18%     
==========================================
  Files         101      101              
  Lines       10437    10529      +92     
==========================================
+ Hits         5954     6026      +72     
- Misses       3962     3977      +15     
- Partials      521      526       +5     
Files Coverage Δ
...ntrollers/k8ssandra/k8ssandracluster_controller.go 75.00% <ø> (ø)
pkg/medusa/reconcile.go 62.07% <100.00%> (+4.96%) ⬆️
controllers/k8ssandra/medusa_reconciler.go 67.91% <50.00%> (-1.77%) ⬇️
controllers/k8ssandra/cleanup.go 54.06% <45.45%> (-0.64%) ⬇️

... and 3 files with indirect coverage changes

@emerkle826 emerkle826 force-pushed the k8ssand-1154 branch 2 times, most recently from 7d1dffa to 64e3c4b Compare January 10, 2024 21:35
@emerkle826 emerkle826 marked this pull request as ready for review January 17, 2024 18:31
@emerkle826 emerkle826 requested a review from a team as a code owner January 17, 2024 18:31
pkg/medusa/reconcile.go Outdated Show resolved Hide resolved
@@ -447,6 +449,10 @@ func MedusaStandaloneDeploymentName(clusterName string, dcName string) string {
return fmt.Sprintf("%s-%s-medusa-standalone", clusterName, dcName)
}

func MedusaPurgeCronJobName(clusterName string, dcName string) string {
return fmt.Sprintf("%s-%s-medusa-purge", clusterName, dcName)
Copy link
Contributor

Choose a reason for hiding this comment

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

[issue] we should also ensure that the resulting string contains no more than 253 characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't address the length here, rather in the PurgeCronJob function here

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should find a way to truncate rather than failing outright.

controllers/k8ssandra/k8ssandracluster_controller.go Outdated Show resolved Hide resolved
pkg/medusa/reconcile.go Outdated Show resolved Hide resolved
controllers/k8ssandra/medusa_reconciler.go Show resolved Hide resolved
pkg/medusa/reconcile.go Outdated Show resolved Hide resolved
pkg/medusa/reconcile.go Outdated Show resolved Hide resolved
pkg/medusa/reconcile.go Outdated Show resolved Hide resolved
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

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

See analysis details on SonarCloud

@emerkle826 emerkle826 merged commit 8c32ae3 into main Jan 19, 2024
61 checks passed
@emerkle826 emerkle826 deleted the k8ssand-1154 branch January 19, 2024 15:14
@c3-clement
Copy link
Contributor

Hello @emerkle826 @olim7t , I have some concerns with this implementation.
Just raised an issue #1278

ericsmalling added a commit to ericsmalling/k8ssandra-operator that referenced this pull request Apr 11, 2024
ericsmalling added a commit to ericsmalling/k8ssandra-operator that referenced this pull request May 22, 2024
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.

Schedule purges on clusters that have Medusa configured
3 participants