Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove code for Protect/Unprotect snapshot in CephFS #4169

Closed
Madhu-1 opened this issue Oct 9, 2023 · 5 comments · Fixed by #4202
Closed

Remove code for Protect/Unprotect snapshot in CephFS #4169

Madhu-1 opened this issue Oct 9, 2023 · 5 comments · Fixed by #4202
Assignees
Labels
component/cephfs Issues related to CephFS

Comments

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Oct 9, 2023

Based on the supported ceph version, check the feasibility of removing the protect/unprotect code

// ProtectSnapshot protects the snapshot of the subvolume.
func (s *snapshotClient) ProtectSnapshot(ctx context.Context) error {
// If "snapshot-autoprotect" feature is present, The ProtectSnapshot
// call should be treated as a no-op.
if checkSubvolumeHasFeature(autoProtect, s.Features) {
return nil
}
fsa, err := s.conn.GetFSAdmin()
if err != nil {
log.ErrorLog(ctx, "could not get FSAdmin: %s", err)
return err
}
err = fsa.ProtectSubVolumeSnapshot(s.FsName, s.SubvolumeGroup, s.VolID, s.SnapshotID)
if err != nil {
if errors.Is(err, rados.ErrObjectExists) {
return nil
}
log.ErrorLog(
ctx,
"failed to protect subvolume snapshot %s %s in fs %s with error: %s",
s.VolID,
s.SnapshotID,
s.FsName,
err)
return err
}
return nil
}
// UnprotectSnapshot unprotects the snapshot of the subvolume.
func (s *snapshotClient) UnprotectSnapshot(ctx context.Context) error {
// If "snapshot-autoprotect" feature is present, The UnprotectSnapshot
// call should be treated as a no-op.
if checkSubvolumeHasFeature(autoProtect, s.Features) {
return nil
}
fsa, err := s.conn.GetFSAdmin()
if err != nil {
log.ErrorLog(ctx, "could not get FSAdmin: %s", err)
return err
}
err = fsa.UnprotectSubVolumeSnapshot(s.FsName, s.SubvolumeGroup, s.VolID,
s.SnapshotID)
if err != nil {
// In case the snap is already unprotected we get ErrSnapProtectionExist error code
// in that case we are safe and we could discard this error.
if errors.Is(err, rados.ErrObjectExists) {
return nil
}
log.ErrorLog(
ctx,
"failed to unprotect subvolume snapshot %s %s in fs %s with error: %s",
s.VolID,
s.SnapshotID,
s.FsName,
err)
return err
}
return nil
}

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Oct 9, 2023

// check are we able to retrieve the size of parent
// ceph fs subvolume info command got added in 14.2.10 and 15.+
// as we are not able to retrieve the parent size we are rejecting the
// request to create snapshot.
// TODO: For this purpose we could make use of cached clusterAdditionalInfo
// too.
This comment and its relavent code can also be removed

@iPraveenParihar
Copy link
Contributor

I'll take a look at this.

@iPraveenParihar
Copy link
Contributor

/assign

@iPraveenParihar
Copy link
Contributor

iPraveenParihar commented Oct 10, 2023

From docs - https://docs.ceph.com/en/latest/cephfs/fs-volumes/#cloning-snapshots

Protecting snapshots prior to cloning was a prerequisite in the Nautilus release, and the commands to protect/unprotect snapshots were introduced for this purpose. This prerequisite, and hence the commands to protect/unprotect, is being deprecated and may be removed from a future release.

The commands being deprecated are:

ceph fs subvolume snapshot protect <vol_name> <subvol_name> <snap_name> [--group_name <subvol_group_name>]
ceph fs subvolume snapshot unprotect <vol_name> <subvol_name> <snap_name> [--group_name <subvol_group_name>]

Since, we have support >=Octopus we can remove the protect/unprotect code.
@Madhu-1

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Oct 10, 2023

Even Octopus is EOL, IMO we can remove the above code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cephfs Issues related to CephFS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants