-
Notifications
You must be signed in to change notification settings - Fork 555
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
rbd: check parent image while mirroring of child #4513
Conversation
This commit adds code to check parent image while enabling mirroring on child image. It errors out if the parent is in trash or not mirror enabled. This enables better communication with the user on why mirroring cannot be enabled on the child image. Signed-off-by: Rakshith R <[email protected]>
@@ -33,6 +33,39 @@ func (ri *rbdImage) EnableImageMirroring(mode librbd.ImageMirrorMode) error { | |||
} | |||
defer image.Close() | |||
|
|||
parentInfo, err := image.GetParent() | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if there is no parent for the image this will fail with librbd.ErrNotFound
error isn't it?
if parentInfo.Image.Trash { | ||
return fmt.Errorf("failed to enable mirroring on image %q:"+ | ||
" parent %q is in trash", | ||
parentInfo.Image.ImageID, ri) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returning ImageID is not useful, if possible return parent name or just discard parent id or log ImageName
parentInfo.Image.ImageID, ri) | ||
} | ||
|
||
parent, err := ri.getParent() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add Trash
to the rbdImage
struct and make only one call getParent()
(need to populate Trash value as well) which gives all the details. we don't need to call getParent
multiple times.
} | ||
|
||
if parentMirroringInfo.State != librbd.MirrorImageEnabled { | ||
return fmt.Errorf("failed to enable mirroring on image %q:"+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to return Invalid RPC code when parent in not mirror enabled or when the parent is in trash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/csi-addons/spec/tree/main/replication#enablevolumereplication
https://github.com/container-storage-interface/spec/blob/master/spec.md#createvolume-errors
So I'll modify csi addons spec EnableVolReplication call errors' INVALID_ARGUEMENT description to include something similar to createvolume_errors indicating we cannot enable replication on certain volumes.
Will replace this with a new pr. |
This commit adds code to check parent image while enabling mirroring on child image. It errors out if the parent is in trash or not mirror enabled.
This enables better communication with the user on why mirroring cannot be enabled on the child image.