mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-13 08:05:19 +09:00
<!-- 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 When the Job behind a mariadb-operator `Backup` or `Restore` fails, the operator sets `Complete=True` with reason `JobFailed` (`SetCompleteWithJob` in mariadb-operator 25.10.2). The MariaDB driver read `Complete=True` alone as success, so a backup whose dump or upload failed ended `Succeeded` and produced a `Ready` Cozystack `Backup` pointing at an archive that does not exist, and a failed restore ended `Succeeded` too. A scheduled `Plan` kept producing such backups. The driver now treats `Complete=True` as success only with reason `JobComplete`, and fails the BackupJob or RestoreJob on `JobFailed` whatever the status, without creating a `Backup`. The failure message names the operator object and carries the Job's own `Failed` reason and message, since the operator's condition message is only `Failed` and the Job's pod, with its logs, is already deleted when the Job fails. A Job that is missing or cannot be read is named in the message instead. Any other `Complete` state keeps waiting under the existing deadline, as before; that includes `Complete=False` with reason `Failed`, which the operator sets with its own cause (`Error creating Job`, `Backup not complete`) for a failure outside the Job and then retries, and the deadline failure now carries that message. Regression tests drive the backup and restore reconcilers through every `Complete` status/reason combination, an absent `Complete` condition, and expired deadlines for an absent condition, an unknown reason and reason `Failed`. They assert the job phase and message as persisted in the API rather than on the in-memory object, a requeue while the run is in progress, and, for backups, that no `Backup` is created on failure. Separate tests cover the failure message for a Job that cannot be read and for a Job without a `Failed` condition. Backups that already exist are not touched. A `Ready` MariaDB `Backup` created by a failed run before this fix may have no archive of its own. Restoring from it can fail or select another dump from the shared prefix. ### 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. --> ### 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: ### 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(mariadb): a MariaDB BackupJob or RestoreJob whose operator Job failed now ends Failed instead of Succeeded, and a failed backup no longer creates a Ready Backup without an archive. Backups created by failed runs before this fix are not removed; restoring from one can fail or select another dump from the shared prefix. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Backup and restore operations now report failed jobs as failures, including details from the underlying job when available. * Operations are marked successful only when the job completes successfully. Other outcomes continue to be monitored until the existing deadline, after which failures include available condition details. * Failed backups no longer create backup artifacts. <!-- end of auto-generated comment: release notes by coderabbit.ai -->