Skip to content

fix: Add endpoint validation in cluster creation commands #1569

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Add endpoint validation in cluster creation commands #1569

wants to merge 1 commit into from

Conversation

@naimadswdn
Copy link
Contributor

@naimadswdn naimadswdn commented Oct 23, 2025

Redis cluster creation was failing silently due to empty endpoint values being added to Redis CLI commands after PR 1568 merge. Scope:

  • Validate getEndpoint() returns non-empty values before using them
  • Return errors from createRedisReplicationCommand when endpoints fail
  • Prevents silent failures in cluster bootstrap process

Description

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

@naimadswdn naimadswdn force-pushed the fix/fix-redis-cluster-creation branch from 04121f6 to 341c351 Compare October 23, 2025 13:15
This was referenced Oct 23, 2025
Merged
Open
@codecov
Copy link

codecov bot commented Oct 23, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 40.00000% with 12 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@566db6f). Learn more about missing BASE report.

Files with missing lines Patch % Lines
internal/k8sutils/redis.go 40.00% 9 Missing and 3 partials ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1569   +/-   ##
=======================================
  Coverage        ?   32.01%           
=======================================
  Files           ?       79           
  Lines           ?     5872           
  Branches        ?        0           
=======================================
  Hits            ?     1880           
  Misses          ?     3803           
  Partials        ?      189

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@naimadswdn
Copy link
Contributor Author

naimadswdn commented Oct 24, 2025

@drivebyer I run the E2E test locally (on simpler kind cluster) and it works. Any ideas how we can get more info about the failed E2E here and in my second PR?

@drivebyer
Copy link
Collaborator

drivebyer commented Oct 24, 2025

@drivebyer I run the E2E test locally (on simpler kind cluster) and it works. Any ideas how we can get more info about the failed E2E here and in my second PR?

Probably a flaky test. You could try debugging by uncommenting the lines at ci.yaml#L269-L279

@drivebyer
Copy link
Collaborator

drivebyer commented Oct 24, 2025

The E2E test failed, likely not because of PR #1568
. It’s a long-time flaky test, I think.

@naimadswdn
Copy link
Contributor Author

naimadswdn commented Oct 24, 2025

Ok, I will take a look into the debug steps. Thanks.

In the meantime, can we merge the second PR? #1557

@naimadswdn naimadswdn force-pushed the fix/fix-redis-cluster-creation branch 2 times, most recently from 1c89395 to fafe97c Compare October 24, 2025 08:33
@naimadswdn
Copy link
Contributor Author

naimadswdn commented Oct 24, 2025

It seems like the flaky test indeed..
I observed the redis cluster setup and all was working as expected.
image

Let me comment the debug steps again and see.

@naimadswdn naimadswdn force-pushed the fix/fix-redis-cluster-creation branch 2 times, most recently from 341c351 to 7fb04f1 Compare October 28, 2025 06:07
@drivebyer drivebyer force-pushed the fix/fix-redis-cluster-creation branch 2 times, most recently from e5c21fb to e5f1ede Compare November 4, 2025 09:10
fix: Add endpoint validation in cluster creation commands
e29a1fd 
Redis cluster creation was failing silently due to empty endpoint values being added to Redis CLI commands after [PR 1568](OT-CONTAINER-KIT#1568) merge.
Scope:
- Validate getEndpoint() returns non-empty values before using them
- Return errors from createRedisReplicationCommand when endpoints fail
- Prevents silent failures in cluster bootstrap process

Signed-off-by: Damian Seredyn <[email protected]>
@naimadswdn naimadswdn force-pushed the fix/fix-redis-cluster-creation branch from e5f1ede to e29a1fd Compare November 4, 2025 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shubham-cmyk shubham-cmyk Awaiting requested review from shubham-cmyk shubham-cmyk is a code owner

@drivebyer drivebyer Awaiting requested review from drivebyer drivebyer is a code owner

@iamabhishek-dubey iamabhishek-dubey Awaiting requested review from iamabhishek-dubey iamabhishek-dubey is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@naimadswdn @drivebyer