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

[stable/redis-ha]: add redis source to restore options #296

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

hawwwdi
Copy link
Contributor

@hawwwdi hawwwdi commented Sep 11, 2024

What this PR does / why we need it:

This PR adds support for using a remote Redis instance as a data source for restoration. It utilizes the redis-cli --rdb command to perform the restore operation.

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [stable/mychartname])

@hawwwdi
Copy link
Contributor Author

hawwwdi commented Sep 16, 2024

@DandyDeveloper

@DandyDeveloper
Copy link
Owner

@hawwwdi Can we work through those inline changes your IDE has updated and undo them for now?

Also, can you provide some logging showing a successful restore operation? Backups testing locally for me is a bit of a nightmare :)

@hawwwdi hawwwdi closed this Nov 7, 2024
@hawwwdi hawwwdi force-pushed the add/restore-from-redis branch from 67bf680 to 4d03a7e Compare November 7, 2024 13:44
@hawwwdi hawwwdi reopened this Nov 7, 2024
@hawwwdi hawwwdi force-pushed the add/restore-from-redis branch 4 times, most recently from 1e4a877 to 0644a73 Compare November 7, 2024 14:32
@hawwwdi hawwwdi force-pushed the add/restore-from-redis branch from 0644a73 to d95da19 Compare November 7, 2024 14:34
@hawwwdi
Copy link
Contributor Author

hawwwdi commented Nov 7, 2024

@DandyDeveloper, I have fixed those inline changes and rolled them back. Additionally, I’ve made some changes to the restoration script: it is now only executed for the initial master instance and checks the connectivity of the provided Redis endpoint to prevent getting stuck in a crashLoopBackOff state in case of a pod restart.

@hawwwdi
Copy link
Contributor Author

hawwwdi commented Nov 7, 2024

example logs of a successful restore operation:
restore-redis init container:

k logs -n redis test-redis-ha-server-0 -c restore-redis -f   
test-redis.test-restore:6379 (10.105.192.181:6379) open
sending REPLCONF capa eof
sending REPLCONF rdb-only 1
SYNC sent to master, writing 7477632846 bytes to '/data/dump.rdb_'
Transfer finished with success.
'/data/dump.rdb_' -> '/data/dump.rdb'

redis instance:

k exec -it -n redis test-redis-ha-server-0 -- sh -c "echo info | redis-cli | tail -2"  
Defaulted container "redis" out of: redis, sentinel, split-brain-fix, redis-exporter, config-init (init), restore-redis (init)
# Keyspace
db1:keys=107713981,expires=0,avg_ttl=0

@hawwwdi
Copy link
Contributor Author

hawwwdi commented Nov 26, 2024

@DandyDeveloper Could you please review this?

@hawwwdi
Copy link
Contributor Author

hawwwdi commented Dec 21, 2024

@DandyDeveloper reminder

@DandyDeveloper
Copy link
Owner

I'll get it checked and if all good, I'll merge tomorrow!

@DandyDeveloper
Copy link
Owner

@hawwwdi I'll approved to get the test run but can you make this a minor rather than a patch? It's a big addition. The versioning should probably reflect this.

DandyDeveloper
DandyDeveloper previously approved these changes Dec 22, 2024
@hawwwdi
Copy link
Contributor Author

hawwwdi commented Dec 22, 2024

@hawwwdi I'll approved to get the test run but can you make this a minor rather than a patch? It's a big addition. The versioning should probably reflect this.

I fixed it.

@DandyDeveloper DandyDeveloper merged commit e879a62 into DandyDeveloper:master Dec 23, 2024
2 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.

2 participants