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

K8OP-295 Expose Medusa's gRPC server port configuration #1456

Merged
merged 5 commits into from
Jan 21, 2025
Merged

Conversation

rzvoncek
Copy link
Contributor

@rzvoncek rzvoncek commented Nov 25, 2024

What this PR does:
This PR allows users of k8ssandra-operator to specify newly-added config field in Medusa that controls which port Medusa will use to start its gRPC server on.

Update: Manual testing discolosed some formatting issues (bad space type) in the to-be medusa config map. That's fixed now and the PR is ready to go.

Which issue(s) this PR fixes:
Fixes #1455

Checklist

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

@rzvoncek rzvoncek marked this pull request as ready for review November 26, 2024 12:05
@rzvoncek rzvoncek requested a review from a team as a code owner November 26, 2024 12:05
@rzvoncek
Copy link
Contributor Author

rzvoncek commented Jan 3, 2025

@adejanovski , I've used the Github UI to resolve the merge conflict (it was just a changelog entry). But it now made a merge commit. I'm not sure that's what we want. Please let me know if it's a no-no and I'll redo this, most likely with an entirely new branch into which I'll cherry-pick the relevant commits.

Copy link

codecov bot commented Jan 3, 2025

Codecov Report

Attention: Patch coverage is 43.28358% with 38 lines in your changes missing coverage. Please review.

Project coverage is 58.99%. Comparing base (42de723) to head (176d33c).
Report is 26 commits behind head on main.

Current head 176d33c differs from pull request most recent head c571de9

Please upload reports for the commit c571de9 to get more accurate results.

Files with missing lines Patch % Lines
pkg/cassandra/datacenter.go 0.00% 17 Missing ⚠️
controllers/medusa/medusatask_controller.go 33.33% 10 Missing and 2 partials ⚠️
controllers/medusa/medusabackupjob_controller.go 50.00% 4 Missing and 2 partials ⚠️
controllers/medusa/medusarestorejob_controller.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1456      +/-   ##
==========================================
- Coverage   59.02%   58.99%   -0.03%     
==========================================
  Files         125      125              
  Lines       12695    12763      +68     
==========================================
+ Hits         7493     7530      +37     
- Misses       4524     4553      +29     
- Partials      678      680       +2     
Files with missing lines Coverage Δ
pkg/medusa/reconcile.go 70.67% <100.00%> (+0.42%) ⬆️
controllers/medusa/medusarestorejob_controller.go 56.67% <50.00%> (-0.28%) ⬇️
controllers/medusa/medusabackupjob_controller.go 60.80% <50.00%> (+2.10%) ⬆️
controllers/medusa/medusatask_controller.go 48.22% <33.33%> (-0.53%) ⬇️
pkg/cassandra/datacenter.go 64.44% <0.00%> (-2.74%) ⬇️

... and 3 files with indirect coverage changes

@rzvoncek
Copy link
Contributor Author

rzvoncek commented Jan 6, 2025

I had to rebase this to resolve another conflict. The merge commit went away, and the CI is green.

Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

Just one blocking comment on using a multi arch image by default, and one quick refactoring suggestion.

pkg/medusa/reconcile.go Outdated Show resolved Hide resolved
controllers/medusa/medusatask_controller.go Outdated Show resolved Hide resolved
Copy link

@rzvoncek rzvoncek merged commit 4772eb1 into main Jan 21, 2025
61 checks passed
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.

Expose Medusa's GRPC server port config field
2 participants