Copilot commented on code in PR #13074:
URL: https://github.com/apache/cloudstack/pull/13074#discussion_r3304723037
##########
test/integration/smoke/test_backup_recovery_nas.py:
##########
@@ -265,3 +265,222 @@ def
test_vm_backup_create_vm_from_backup_in_another_zone(self):
self.assertEqual(backup_repository.crosszoneinstancecreation, True,
"Cross-Zone Instance Creation could not be enabled on the backup repository")
self.vm_backup_create_vm_from_backup_int(template.id, [network.id])
+
+ # ------------------------------------------------------------------
+ # Incremental backup tests (RFC #12899 / PR #13074)
+ # ------------------------------------------------------------------
+ # These tests exercise the incremental NAS backup chain semantics:
+ # full -> incN cadence, restore-from-incremental, delete-middle chain
+ # repair, refuse-delete-full-with-children, and stopped-VM fallback.
+ #
+ # All tests set nas.backup.full.every to a small value (3) so a chain
+ # forms quickly without needing many backup iterations. They restore
+ # the original value at teardown.
+
+ def _set_full_every(self, value):
+ Configurations.update(self.apiclient, name='nas.backup.full.every',
+ value=str(value), zoneid=self.zone.id)
+
+ def _backup_type(self, backup):
+ # Backup objects expose `type`; for chained backups it's
"INCREMENTAL", else "FULL".
+ return getattr(backup, 'type', 'FULL') or 'FULL'
+
+ @attr(tags=["advanced", "backup"], required_hardware="true")
+ def test_incremental_chain_cadence(self):
+ """
+ With nas.backup.full.every=3, the sequence of backups should be
+ FULL, INCREMENTAL, INCREMENTAL, FULL, INCREMENTAL, ...
+ """
+ self.backup_offering.assignOffering(self.apiclient, self.vm.id)
+ self._set_full_every(3)
+ try:
+ ssh_client_vm = self.vm.get_ssh_client(reconnect=True)
+ ssh_client_vm.execute("touch /root/incremental_marker_1.txt")
+
+ created = []
+ for i in range(5):
+ Backup.create(self.apiclient, self.vm.id, "inc_chain_%d" % i)
+ # write a small change so each incremental has something to
capture
+ ssh_client_vm.execute("dd if=/dev/urandom of=/root/delta_%d
bs=64k count=4 2>/dev/null" % i)
+ time.sleep(2)
+ created = Backup.list(self.apiclient, self.vm.id)
+
+ self.assertEqual(len(created), 5, "Expected 5 backups after 5
Backup.create calls")
+ # Sort oldest-first by date
+ created.sort(key=lambda b: b.created)
+
+ expected = ['FULL', 'INCREMENTAL', 'INCREMENTAL', 'FULL',
'INCREMENTAL']
+ actual = [self._backup_type(b).upper() for b in created]
+ self.assertEqual(actual, expected,
+ "With nas.backup.full.every=3, chain pattern should be %s but
was %s" % (expected, actual))
+
+ # Cleanup all backups (newest first to satisfy chain rules without
forced=true)
+ for b in reversed(created):
+ Backup.delete(self.apiclient, b.id)
+ finally:
+ self._set_full_every(10)
+ self.backup_offering.removeOffering(self.apiclient, self.vm.id)
Review Comment:
These incremental tests claim they “restore the original value at teardown”,
but the finally block hard-codes nas.backup.full.every back to 10. If the
environment’s original setting differs, this will leak config changes across
tests. Capture the original value (e.g., via Configurations.list/show) and
restore that instead of using a constant.
--
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]
