rbd: add ParentInTrash parameter in rbdImage struct #4522

Rakshith-R · 2024-03-29T09:37:43Z

rbd: add ParentInTrash parameter in rbdImage struct

This commit adds ParentInTrash parameter in rbdImage struct
and makes use of it in getParent() function in order to avoid
error in case the parent is present but in trash.
This commit modifies a test case to check creation of PVC-PVC clone of a restored PVC when parent snapshot is deleted.

Signed-off-by: Rakshith R [email protected]

Describe what this PR does

Provide some context for the reviewer

Is there anything that requires special attention

Do you have any questions?

Is the change backward compatible?

Are there concerns around backward compatibility?

Provide any external context for the change, if any.

For example:

Kubernetes links that explain why the change is required
CSI spec related changes/catch-up that necessitates this patch
golang related practices that necessitates this change

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Fixes: #issue_number

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

Checklist:

Commit Message Formatting: Commit titles and messages follow
guidelines in the developer
guide.
Reviewed the developer guide on Submitting a Pull
Request
Pending release
notes
updated with breaking and/or notable changes for the next major release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

/retest ci/centos/<job-name>: retest the <job-name> after unrelated
failure (please report the failure too!)

Rakshith-R · 2024-03-29T09:38:19Z

/test ci/centos/mini-e2e/k8s-1.28/rbd

Rakshith-R · 2024-03-29T09:38:57Z

/test ci/centos/mini-e2e/k8s-1.28/rbd

Rakshith-R · 2024-03-29T09:40:22Z

/test ci/centos/mini-e2e/k8s-1.28-rbd

Rakshith-R · 2024-03-29T09:41:24Z

/test ci/centos/mini-e2e/k8s-1.28/test-type-rbd

Rakshith-R · 2024-03-29T09:41:53Z

/test ci/centos/mini-e2e/k8s-1.28

Rakshith-R · 2024-03-29T09:42:35Z

/test ci/centos/mini-e2e/k8s-1.29/test_type-rbd

Rakshith-R · 2024-03-29T09:44:31Z

hey @karthik-us ,
I was trying to choose only particular test type= rbd to run .
Can you check what's the correct command to run ?

karthik-us · 2024-03-29T10:34:01Z

Hey @Rakshith-R, the command you used is correct. Let me check why it is not triggering the e2e. I am triggring the helm e2e as well to see whether this is the case there as well (should be same I think).

karthik-us · 2024-03-29T10:34:31Z

/test ci/centos/mini-e2e-helm/k8s-1.29/test_type-rbd

Rakshith-R · 2024-03-29T11:24:20Z

/test ci/centos/mini-e2e/k8s-1.28

Rakshith-R · 2024-03-29T12:30:42Z

  I0329 11:41:23.428730       1 utils.go:199] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 GRPC request: {"accessibility_requirements":{"preferred":[{"segments":{"topology.rbd.csi.ceph.com/region":"testregion","topology.rbd.csi.ceph.com/zone":"testzone"}}],"requisite":[{"segments":{"topology.rbd.csi.ceph.com/region":"testregion","topology.rbd.csi.ceph.com/zone":"testzone"}}]},"capacity_range":{"required_bytes":1073741824},"name":"pvc-863ffdf5-1603-4384-9a6b-b194a628bf18","parameters":{"clusterID":"9623e912-eaaa-49a3-8dec-6a81cb4e948f","csi.storage.k8s.io/pv/name":"pvc-863ffdf5-1603-4384-9a6b-b194a628bf18","csi.storage.k8s.io/pvc/name":"rbd-pvc-clone-0","csi.storage.k8s.io/pvc/namespace":"rbd-5575","encrypted":"true","encryptionKMSID":"vault-test","imageFeatures":"layering","pool":"replicapool"},"secrets":"***stripped***","volume_capabilities":[{"AccessType":{"Mount":{"fs_type":"ext4","mount_flags":["discard"]}},"access_mode":{"mode":1}}],"volume_content_source":{"Type":{"Volume":{"volume_id":"0001-0024-9623e912-eaaa-49a3-8dec-6a81cb4e948f-0000000000000002-635b285e-0323-4d46-a9ce-bc88b8d66954"}}}}
  I0329 11:41:23.428890       1 rbd_util.go:1315] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 setting disableInUseChecks: false image features: [layering] mounter: rbd
  time="2024-03-29T11:41:23Z" level=info msg="Authenticated to Vault with kubernetes\n"
  I0329 11:41:23.442444       1 omap.go:89] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 got omap values: (pool="replicapool", namespace="", name="csi.volume.635b285e-0323-4d46-a9ce-bc88b8d66954"): map[csi.imageid:11721dcb9c86 csi.imagename:csi-vol-635b285e-0323-4d46-a9ce-bc88b8d66954 csi.volname:pvc-1a53d90e-eaa2-4856-b26c-e8c7ece7fa65 csi.volume.encryptKMS:vault-test csi.volume.encryptionType:block csi.volume.owner:rbd-5575]
  time="2024-03-29T11:41:23Z" level=info msg="Authenticated to Vault with kubernetes\n"
  I0329 11:41:23.483103       1 omap.go:89] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 got omap values: (pool="replicapool", namespace="", name="csi.volumes.default"): map[]
  E0329 11:41:23.513084       1 utils.go:203] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 GRPC error: rpc error: code = Internal desc = image not found: RBD image not found

https://jenkins-ceph-csi.apps.ocp.cloud.ci.centos.org/blue/organizations/jenkins/mini-e2e_k8s-1.28/detail/mini-e2e_k8s-1.28/507/pipeline/90

Rakshith-R · 2024-03-29T13:09:27Z

/test ci/centos/mini-e2e/k8s-1.28

Rakshith-R · 2024-04-01T09:04:18Z

  I0329 11:41:23.428730       1 utils.go:199] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 GRPC request: {"accessibility_requirements":{"preferred":[{"segments":{"topology.rbd.csi.ceph.com/region":"testregion","topology.rbd.csi.ceph.com/zone":"testzone"}}],"requisite":[{"segments":{"topology.rbd.csi.ceph.com/region":"testregion","topology.rbd.csi.ceph.com/zone":"testzone"}}]},"capacity_range":{"required_bytes":1073741824},"name":"pvc-863ffdf5-1603-4384-9a6b-b194a628bf18","parameters":{"clusterID":"9623e912-eaaa-49a3-8dec-6a81cb4e948f","csi.storage.k8s.io/pv/name":"pvc-863ffdf5-1603-4384-9a6b-b194a628bf18","csi.storage.k8s.io/pvc/name":"rbd-pvc-clone-0","csi.storage.k8s.io/pvc/namespace":"rbd-5575","encrypted":"true","encryptionKMSID":"vault-test","imageFeatures":"layering","pool":"replicapool"},"secrets":"***stripped***","volume_capabilities":[{"AccessType":{"Mount":{"fs_type":"ext4","mount_flags":["discard"]}},"access_mode":{"mode":1}}],"volume_content_source":{"Type":{"Volume":{"volume_id":"0001-0024-9623e912-eaaa-49a3-8dec-6a81cb4e948f-0000000000000002-635b285e-0323-4d46-a9ce-bc88b8d66954"}}}}
  I0329 11:41:23.428890       1 rbd_util.go:1315] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 setting disableInUseChecks: false image features: [layering] mounter: rbd
  time="2024-03-29T11:41:23Z" level=info msg="Authenticated to Vault with kubernetes\n"
  I0329 11:41:23.442444       1 omap.go:89] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 got omap values: (pool="replicapool", namespace="", name="csi.volume.635b285e-0323-4d46-a9ce-bc88b8d66954"): map[csi.imageid:11721dcb9c86 csi.imagename:csi-vol-635b285e-0323-4d46-a9ce-bc88b8d66954 csi.volname:pvc-1a53d90e-eaa2-4856-b26c-e8c7ece7fa65 csi.volume.encryptKMS:vault-test csi.volume.encryptionType:block csi.volume.owner:rbd-5575]
  time="2024-03-29T11:41:23Z" level=info msg="Authenticated to Vault with kubernetes\n"
  I0329 11:41:23.483103       1 omap.go:89] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 got omap values: (pool="replicapool", namespace="", name="csi.volumes.default"): map[]
  E0329 11:41:23.513084       1 utils.go:203] ID: 32 Req-ID: pvc-863ffdf5-1603-4384-9a6b-b194a628bf18 GRPC error: rpc error: code = Internal desc = image not found: RBD image not found

https://jenkins-ceph-csi.apps.ocp.cloud.ci.centos.org/blue/organizations/jenkins/mini-e2e_k8s-1.28/detail/mini-e2e_k8s-1.28/507/pipeline/90

https://jenkins-ceph-csi.apps.ocp.cloud.ci.centos.org/blue/organizations/jenkins/mini-e2e_k8s-1.28/detail/mini-e2e_k8s-1.28/508/pipeline/90

The ci passes with the fix of checking if the parent is in trash.

Madhu-1 · 2024-04-02T06:39:17Z

e2e/rbd.go

@@ -3609,6 +3609,27 @@ var _ = Describe("RBD", func() {
 						validateOmapCount(f, 1, rbdType, defaultRBDPool, volumesType)
 						validateOmapCount(f, 0, rbdType, defaultRBDPool, snapsType)

+						// create pvc-pvc clone


Add a comment on what we are trying to test here.

Madhu-1 · 2024-04-02T06:39:48Z

e2e/rbd.go

+							framework.Failf("failed to create smart clone PVC %q: %v",
+								smartClonePVC.Name, err)
+						}
+


validate the omap

Madhu-1 · 2024-04-02T06:39:59Z

e2e/rbd.go

+							framework.Failf("failed to delete smart clone PVC %q: %v",
+								smartClonePVC.Name, err)
+						}
+


validate the omap after deletion as well

Madhu-1 · 2024-04-02T06:40:59Z

internal/rbd/rbd_util.go

@@ -1631,7 +1634,7 @@ func (ri *rbdImage) getParent() (*rbdImage, error) {
 	if err != nil {
 		return nil, err
 	}
-	if ri.ParentName == "" {
+	if ri.ParentName == "" || ri.ParentInTrash {


Looks like we are going to get ParentName even it its in trash, pleas add a comment here as well.

Yeah, that looks weird. An image in trash does not have a name, I think?

So, does this check actually make a difference? If a parent image does not have a name, it will be set to "" anyway, and the new ParentInTrash is not needed.

It does have name.
We just can't refer to it by its name when it is in trash.
The failed ci that I linked in the comment above proves it.

thats strange, can you please add a comment about it.

Pull request has been modified.

mergify · 2024-04-23T09:33:14Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T09:33:54Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T09:35:08Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T09:38:10Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T09:52:14Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T09:56:34Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:18:58Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:19:49Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:21:27Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:22:33Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:24:25Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:26:27Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:29:20Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:39:11Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T10:40:12Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T11:05:37Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

nixpanic · 2024-04-23T11:07:01Z

/test ci/centos/mini-e2e-helm/k8s-1.26

nixpanic · 2024-04-23T11:07:12Z

/test ci/centos/k8s-e2e-external-storage/1.26

nixpanic · 2024-04-23T11:07:25Z

/test ci/centos/mini-e2e/k8s-1.26

mergify · 2024-04-23T11:07:44Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T11:22:16Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T11:29:33Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T11:55:28Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T11:56:04Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T12:02:34Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T12:03:30Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify · 2024-04-23T12:04:57Z

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

mergify bot added the component/testing Additional test cases or CI work label Mar 29, 2024

Rakshith-R force-pushed the fix-deleted-snap-bug branch 2 times, most recently from 57f704b to 8aed1e9 Compare March 29, 2024 11:23

Rakshith-R force-pushed the fix-deleted-snap-bug branch from 8aed1e9 to efe7ece Compare March 29, 2024 13:08

Rakshith-R force-pushed the fix-deleted-snap-bug branch 2 times, most recently from d48e200 to d460276 Compare April 1, 2024 06:47

Rakshith-R marked this pull request as ready for review April 1, 2024 09:01

Rakshith-R changed the title ~~[WIP]e2e: validate PVC-PVC clone creation with deleted parent snap~~ rbd: add ParentInTrash parameter in rbdImage struct Apr 1, 2024

mergify bot added the component/rbd Issues related to RBD label Apr 1, 2024

Rakshith-R requested a review from a team April 1, 2024 09:04

yati1998 previously approved these changes Apr 1, 2024

View reviewed changes

Madhu-1 reviewed Apr 2, 2024

View reviewed changes

Rakshith-R force-pushed the fix-deleted-snap-bug branch from d460276 to 78707a0 Compare April 16, 2024 11:28

Rakshith-R requested review from nixpanic and Madhu-1 April 16, 2024 11:28

Rakshith-R requested a review from a team April 16, 2024 14:58

mergify bot merged commit 1bb78fd into ceph:devel Apr 23, 2024
35 of 38 checks passed

mergify bot mentioned this pull request Apr 23, 2024

rbd: add ParentInTrash parameter in rbdImage struct (backport #4522) #4579

Merged

6 tasks

Madhu-1 mentioned this pull request Apr 24, 2024

BUG 2264900: rbd: add ParentInTrash parameter in rbdImage struct red-hat-storage/ceph-csi#298

Merged

rbd: add ParentInTrash parameter in rbdImage struct #4522

rbd: add ParentInTrash parameter in rbdImage struct #4522

Conversation

Rakshith-R commented Mar 29, 2024 • edited Loading

Describe what this PR does

Is there anything that requires special attention

Related issues

Future concerns

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

karthik-us commented Mar 29, 2024

karthik-us commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Mar 29, 2024

Rakshith-R commented Apr 1, 2024

Madhu-1 Apr 2, 2024

Choose a reason for hiding this comment

Madhu-1 Apr 2, 2024

Choose a reason for hiding this comment

Madhu-1 Apr 2, 2024

Choose a reason for hiding this comment

Madhu-1 Apr 2, 2024

Choose a reason for hiding this comment

nixpanic Apr 2, 2024

Choose a reason for hiding this comment

Rakshith-R Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Madhu-1 Apr 2, 2024

Choose a reason for hiding this comment

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

nixpanic commented Apr 23, 2024

nixpanic commented Apr 23, 2024

nixpanic commented Apr 23, 2024

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

mergify bot commented Apr 23, 2024

🛑 Command queue default cancelled because of a new queue command with different arguments

Rakshith-R commented Mar 29, 2024 •

edited

Loading

Rakshith-R Apr 2, 2024 •

edited

Loading

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments

🛑 Command `queue default` cancelled because of a new `queue` command with different arguments