Skip to content

Conversation

@naimadswdn
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented Oct 23, 2025

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

@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 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

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

@naimadswdn
Copy link
Contributor Author

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

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
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants