bhouse-nexthop commented on PR #14646: URL: https://github.com/apache/cloudstack/pull/14646#issuecomment-6752446795
### Test report: snapshot operations when the snapshot's volume is gone **Summary:** for a snapshot whose volume was deleted, `revertSnapshot` is refused with a clear error, and `createVolume` and `createTemplate` from it work. When the volume row itself is missing, as in #14599, all three are refused with clear errors and nothing is left behind. No NPE was logged on any of these paths. **Environment** - A CloudStack 24.0 (main) build with this PR, alongside some of my other open PRs, including #14600. Three KVM hosts, Ceph RBD primary storage, one NFS secondary store, `snapshot.backup.to.secondary=true`. - The PR targets 4.22; it wasn't run on a 4.22 build. 4.22's `SnapshotManagerImpl.revertSnapshot` also calls `getState()` on the result of `_volsDao.findById()` without a null check. - A 1 GB data volume with one snapshot (BackedUp). **Case 1: the volume was deleted** (`deleteVolume`; the row stays, `Expunged`, with `removed` set) - `listSnapshots` shows the snapshot with `revertable=false`. | Call | Result | |---|---| | `revertSnapshot` | refused, 431: `Cannot revert snapshot <uuid>, because its volume no longer exists` | | `createVolume snapshotid=<snapshot>` | OK: a 1 GiB volume, Ready (then deleted) | | `createTemplate snapshotid=<snapshot>` | OK: template Ready (then deleted) | Before this PR, `revertSnapshot` here failed with `Cannot invoke "com.cloud.storage.VolumeVO.getState()" because "volume" is null`. I saw that while testing #14600. **Case 2: the volume row is missing** (as in #14599) - To get there, I pointed a second snapshot's `volume_id` at an id with no row, and put it back afterwards. | Call | Result | |---|---| | `revertSnapshot` | refused, 431: `Cannot revert snapshot <uuid>, because its volume no longer exists` | | `createVolume snapshotid=<snapshot>` | refused, 431: `Cannot create a volume from snapshot <uuid>, because the volume it was taken from no longer exists` | | `createTemplate snapshotid=<snapshot>` | refused, 431: `Cannot create a Template from Snapshot <uuid>, because the volume it was taken from no longer exists` | After the refusals there was no template record and no volume left with the requested names. The template refusal happens before a template record is written, as the PR says. - None of these calls logged a `NullPointerException`. - Both snapshots, the created volume and the created template were deleted afterwards. **Result:** pass. **Side note, not from this PR:** taking a snapshot of a *detached* volume on the zone-wide RBD pool logged a caught NPE at DEBUG: `post process snapshot failed java.lang.NullPointerException: Cannot invoke "java.lang.Long.longValue()" because "clusterId" is null` at `SnapshotManagerImpl.postCreateSnapshot`. That is the `kvmIncrementalSnapshot.valueIn(clusterId)` / `endLastChainIfNeeded(clusterId, ...)` branch. The snapshot itself was BackedUp. This code is unchanged by the PR. **Not covered** - `copySnapshot` without `sourcezoneid`: the lab has one zone and one secondary store. - Case 2 used a DB change to make the volume row missing. The production case came from a row that was really deleted. -- 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]
