jmsperu commented on PR #12900:
URL: https://github.com/apache/cloudstack/pull/12900#issuecomment-5452728893
Thanks for running it, @DaanHoogland.
**On the Trillian result (tid-16825): 148 OK, 1 error.** The failure is
`test_01_redundant_vpc_site2site_vpn` in `test_vpc_vpn.py` — redundant-VPC
site-to-site VPN. It does not touch the backup framework or this plugin, and
this PR changes only `InfrastructureBackupTask`, `NASBackupProvider` and their
tests. Unrelated failure.
**The open review threads are all addressed at `3fa88ea`** — they just were
not resolved:
- *Negative `retentionCount` causing `ArrayIndexOutOfBoundsException`* —
clamped at the top of `cleanupOldBackups`, so `toDelete` can never exceed
`backups.length`.
- *Task scheduled on every management server* — serialised by
`GlobalLock("infra-backup")`, and each server now writes to its own
`<location>/infra-backup/<ms-hostname>/<timestamp>` directory with retention
applied per server, so concurrent runs neither collide nor delete each other's
backups.
- *Extract the repeated backup blocks into helpers, drop the prelude
comments* — done as `backupDatabases(...)`, `backupDirectoryIfPresent(...)` and
`backupDirectory(...)`; the comments came out with them.
Could you resolve them if you are happy, or say what still reads wrong.
`@abh1sar`, this is the one Daan asked you to look at.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
