mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 05:25:14 +09:00
IvanHunters
9e3ed1f2cb
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The redis backup round-trip suite went red on PRs that do not touch redis, each time on an empty read (#4395). The report snapshots of those runs point to two separate causes, and I fix both here. The empty marker reads came from the demo helper that ran redis-cli in a throwaway `kubectl run -i --rm` Pod. kubectl gets that Pod's stdout over an attach, and containerd's attach only carries output written after the attach is registered. kubectl falls back to the Pod log only when the attach fails. A redis-cli that finishes in that gap comes back as an empty string with exit 0. In the failed runs, the master's log shows the key present, and sentinel logged no failover, yet the GET read ''. `redis_cmd` now runs redis-cli by `kubectl exec` in a sentinel Pod, which already has it. An exec'd process gets its own pipes, so its output can't be lost that way, and its exit code comes back. The password now goes over stdin rather than a Pod env var. The missing-strategy failure was real: `cozy-default-redis` did not exist yet. It gets rendered behind a lookup of the platform bucket, so it appears only after a forced Helm upgrade. helm-controller holds that upgrade while `cozystack-basics` is not Ready, and the e2e install's late platform patches keep it not Ready for several minutes. The demo read the object once. It now waits for the object with `kubectl wait --for=create` and a 15 minute bound. That wait fails right away on anything other than NotFound. The demo's reads also stop sending stderr to `/dev/null`, so the next failure names its cause. `hack/redis-backup-demo-helpers.bats` covers `redis_cmd` with stubbed kubectl and redis-cli: it reads through exec, keeps the password out of argv, sends no AUTH when there is no Secret, and fails loudly when the exec fails or sentinel has no master. Closes #4395 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> No UI changes. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `examples/backups/redis/` and a new unit test under `hack/`. No entry in the trigger map covers either. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(backups): the Redis backup demo reads its keys through `kubectl exec`, so a short-lived Pod can no longer drop its output, and it waits for the platform's `cozy-default-redis` strategy to appear instead of failing on its first read. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Redis backup and restore commands now report Kubernetes and Secret read errors instead of hiding them, while still treating a missing password Secret as empty. * Redis commands pass authentication securely and fail clearly when Sentinel cannot identify a master. * Backup demos wait for the Redis backup strategy to appear and provide clearer error messages. * **Tests** * Added coverage for Redis authentication handling, command failures, and missing Sentinel masters. <!-- end of auto-generated comment: release notes by coderabbit.ai -->