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

fix: ping instead of checking pod readiness #1038

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

com6056
Copy link
Contributor

@com6056 com6056 commented Aug 2, 2024

Description

Right now, since the operator relies on checking pod readiness, it makes using a more robust readiness probe not possible. This changes it so instead of checking the pod readiness, we just directly PING all of the redis nodes instead (which the default readiness probe does anyways).

Also exposes podManagementPolicy to be configurable since you need Parallel if you are using readiness probes that check cluster health.

Fixes #1016

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Tests have been added/modified and all tests pass.
  • Functionality/bugs have been confirmed to be unchanged or fixed.
  • I have performed a self-review of my own code.
  • Documentation has been updated or added where necessary.

Additional Context

@com6056 com6056 changed the title ping instead of checking pod readiness fix: ping instead of checking pod readiness Aug 2, 2024
@com6056 com6056 force-pushed the jrodgers-ping-for-ready branch from d1e81b1 to d14a505 Compare August 2, 2024 17:20
@com6056 com6056 force-pushed the jrodgers-ping-for-ready branch from d14a505 to 6de507e Compare August 2, 2024 17:20
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 37.50000% with 25 lines in your changes missing coverage. Please review.

Project coverage is 45.97%. Comparing base (d121d86) to head (ae15bc7).
Report is 91 commits behind head on master.

Files Patch % Lines
k8sutils/cluster-scaling.go 0.00% 16 Missing ⚠️
k8sutils/statefulset.go 42.85% 8 Missing ⚠️
controllers/rediscluster_controller.go 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1038       +/-   ##
===========================================
+ Coverage   35.20%   45.97%   +10.76%     
===========================================
  Files          19       20        +1     
  Lines        3213     2730      -483     
===========================================
+ Hits         1131     1255      +124     
+ Misses       2015     1400      -615     
- Partials       67       75        +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Jordan Rodgers <[email protected]>
@com6056 com6056 force-pushed the jrodgers-ping-for-ready branch from c2f6680 to a9f97cb Compare August 2, 2024 17:49
Signed-off-by: Jordan Rodgers <[email protected]>
@com6056 com6056 force-pushed the jrodgers-ping-for-ready branch from f98328d to 62b750c Compare August 2, 2024 18:06
com6056 added 2 commits August 2, 2024 11:13
Signed-off-by: Jordan Rodgers <[email protected]>
@com6056 com6056 force-pushed the jrodgers-ping-for-ready branch from 3f6cb3d to ae15bc7 Compare August 2, 2024 18:59
@drivebyer
Copy link
Collaborator

@com6056 Could you please resolve the conflict

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.

Using a readiness probe halts operator progress
2 participants