-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
feat: guarantee to avoid bad master ip on Sentinel #1289
feat: guarantee to avoid bad master ip on Sentinel #1289
Conversation
Please review and advise @drivebyer @shubham-cmyk |
76080a7
to
9788ad4
Compare
the CI/Go Test is failing due to the new way of not updating the Sts when the value is nil (which is expected),
|
This block is returning empty IP |
Okay you have to just create a pod.
Write one yaml that could match the sentinel pod metadata that we check. Take Help from here you can also put this this converter code into testutils/redis-sentinel/ |
Signed-off-by: Husni Alhamdani <dhanielluis@gmail.com>
Signed-off-by: Husni Alhamdani <dhanielluis@gmail.com>
Signed-off-by: Husni Alhamdani <dhanielluis@gmail.com>
Signed-off-by: Husni Alhamdani <dhanielluis@gmail.com>
Signed-off-by: Husni Alhamdani <dhanielluis@gmail.com>
d50451c
to
5cb5b4f
Compare
pkg/k8sutils/redis-sentinel_test.go
Outdated
}, | ||
}, | ||
) | ||
if got, err := getSentinelEnvVariable(ctx, tt.args.client, tt.args.cr, dynamicClient); !reflect.DeepEqual(got, tt.want) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the getSentinelEnvVariable
is calling getRedisReplicationMasterPod
function that using redis client (it's looking for response of "master"), but it never happens,
I was trying to add some client fake, but not sure how to go further, please check @drivebyer @shubham-cmyk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I introduce gomonkey to fix it
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1289 +/- ##
=======================================
Coverage ? 33.62%
=======================================
Files ? 55
Lines ? 6087
Branches ? 0
=======================================
Hits ? 2047
Misses ? 3854
Partials ? 186 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Guarantee to avoid bad master ip on Sentinel, in edge/worse cases, usually we are facing replication instance pointing to
master 0.0.0.0
Fixes #ISSUE
Type of change
Checklist
Additional Context