e2e: add a test case for rbd-nbd mounter #1839

pkalever · 2021-01-21T11:39:31Z

Describe what this PR does

Validate the basic working of rbd-nbd

Test cases:

Create a PV with rbd-nbd backend and start the application pod using it
After the application pod is started, restart the node plugin and expect the IO to fail as the rbd-nbd process is killed
After restarting the node plugin, restart the rbd-nbd process by reattaching/re-mapping the device connection and expect the IO from the application pod to continue

Dependencies

Requires ceph/ceph:pacific which is not yet released
Depends on nbd.ko in minikube, see iso: enable Network Block Device support kubernetes/minikube#10217

Updates: #667

nixpanic · 2021-01-22T16:06:32Z

You may want to have a look at #1840 for an idea how to test it in the CI before the minikube PR is merged+released.

pkalever · 2021-01-25T08:30:27Z

@nixpanic I hope the CI picks the modifications to minikube iso URL as part of this PR testing, is that right?
Or CI will only pick once the changes are merged?

nixpanic · 2021-01-25T09:18:48Z

@nixpanic I hope the CI picks the modifications to minikube iso URL as part of this PR testing, is that right?
Or CI will only pick once the changes are merged?

The CI uses scripts/minikube.sh from the PR that modifies it, so --iso-url will be included in this testing.

nixpanic · 2021-01-25T09:21:02Z

Oh, note that #1831 contains a modification to the EXTRA_CONFIG options for minikube, done right. CI jobs will likely fail with the change you made here now.

pkalever · 2021-01-25T10:23:58Z

Oh, note that #1831 contains a modification to the EXTRA_CONFIG options for minikube, done right. CI jobs will likely fail with the change you made here now.

The PR#1831 seems to be in open state now, when it gets merged I shall rebase this, hope that looks like a good plan.
And yeah! while PR#1831 is still in the open state, I don't see a reason why this should fail?

Thanks!

Madhu-1 · 2021-01-27T07:29:48Z

Looks to be a PSP issue https://jenkins-ceph-csi.apps.ocp.ci.centos.org/blue/organizations/jenkins/mini-e2e-helm_k8s-1.19/detail/mini-e2e-helm_k8s-1.19/567/pipeline

pkalever · 2021-01-27T07:42:25Z

Looks to be a PSP issue https://jenkins-ceph-csi.apps.ocp.ci.centos.org/blue/organizations/jenkins/mini-e2e-helm_k8s-1.19/detail/mini-e2e-helm_k8s-1.19/567/pipeline

Thanks @Madhu-1, now I understand why @nixpanic was warning in the comments above.
At least until 73d9428 is merged, I should copy them to my PR, just to comfort CI.

nixpanic · 2021-02-10T15:54:46Z

With #1811 the minikube start commands in scripts/minikube.sh have been adapted. There is an additional --cni option now. You'll need to manually rebase this.

humblec · 2021-03-03T11:16:44Z

e2e/pod.go

 	cmd := []string{"/bin/sh", "-c", c}
 	podList, err := f.PodClientNS(ns).List(context.TODO(), *opt)
 	framework.ExpectNoError(err)
 	if len(podList.Items) == 0 {
 		return framework.ExecOptions{}, errors.New("podlist is empty")
 	}
+	found := false


find Container Name in the pod can be wrapped into a new function .

Thanks @pkalever 👍

humblec · 2021-03-03T11:19:40Z

e2e/pod.go

+}
+
+func execCommandInContainer(f *framework.Framework, c, ns string, cn string, opt *metav1.ListOptions) (string, string, error) {
+	podPot, err := getCommandInPodOpts(f, c, ns, cn, opt)


eventhough its not introduced here, Is above (podPot) a weird name ? may be we have to rename it as podOpt ?

addressed this one too. Thanks!

Cool. thanks !

pkalever · 2021-05-06T06:14:27Z

e2e/pod.go

@@ -305,7 +305,7 @@ func deletePod(name, ns string, c kubernetes.Interface, t int) error {
 	})
 }

-func deletePodWithLabel(label, ns string, skipNotFound bool) error {
+func deletePodWithLabel(label, ns string, skipNotFound bool) error { //nolint:unparam


@nixpanic this should fix the golint warnings!

pkalever · 2021-05-07T06:57:50Z

@Madhu-1 @nixpanic As we don't see any objections, could you please merge this PR now?

Thanks!

humblec · 2021-05-07T07:03:34Z

@pkalever one quick check: Have we tested this scenario when the actual node reboot or move to NOT READY state and come back after some time ?

pkalever · 2021-05-07T07:12:11Z

@pkalever one quick check: Have we tested this scenario when the actual node reboot or move to NOT READY state and come back after some time ?

When a node is rebooted, the application pod might be migrated to a new node or might be scheduled on the same node, in any case, a new NodeStageVolume call is expected to take care of mapping the device.

These tests are mainly focusing on the restart of the node plugin, where there is no NodeStageVolume call for now. We are working on implementing the missing gaps in parallel, where a new init container or a sidecar is supposed to get the list of volume attachments on a node (upon the restart of the node plugin) and make new NodeStageVolume grpc Calls per device for reattaching (remapping) rbd-nbd processes/devices.

Thanks!

humblec · 2021-05-07T07:20:15Z

@pkalever one quick check: Have we tested this scenario when the actual node reboot or move to NOT READY state and come back after some time ?

When a node is rebooted, the application pod might be migrated to a new node or might be scheduled on the same node, in any case, a new NodeStageVolume call is expected to take care of mapping the device.

There are some ( corner ) cases, like network error, kubelet goes wrong, node completely goes out..etc which are bit different from volume pov. We have also seen issues popping up with some race conditions from CO to detect and issues RPC calls in between..etc. The reason for asking node not ready scenario testing was to understand/make sure we are not landing into issues easily when we remount the shares with these options. So I think its good to check once if we have a setup :)

pkalever · 2021-05-07T07:46:23Z

There are some ( corner ) cases, like network error, kubelet goes wrong, node completely goes out..etc which are bit different from volume pov. We have also seen issues popping up with some race conditions from CO to detect and issues RPC calls in between..etc. The reason for asking node not ready scenario testing was to understand/make sure we are not landing into issues easily when we remount the shares with these options. So I think its good to check once if we have a setup :)

Thanks Humble, Got you. These test cases are less focused on the corner case. But I will make sure to test these corner cases with the logic we are implementing to address node plugin restart automatically.

pkalever · 2021-05-11T04:48:23Z

@Madhu-1 @nixpanic can you please help on taking this PR to merge?
I would like to see them running in the CI which will help us improve the confidence on rbd-nbd.

Thanks!

nixpanic

It is good to have this tested regularly, so let's get it in.

In case a manual rebase is needed, please correct the formatting of the // nolint: comments.

nixpanic · 2021-05-11T07:08:38Z

e2e/pod.go

@@ -204,7 +204,7 @@ func execCommandInPod(f *framework.Framework, c, ns string, opt *metav1.ListOpti
 	return stdOut, stdErr, err
 }

-func execCommandInContainer(f *framework.Framework, c, ns, cn string, opt *metav1.ListOptions) (string, string, error) {
+func execCommandInContainer(f *framework.Framework, c, ns, cn string, opt *metav1.ListOptions) (string, string, error) { //nolint:unparam,lll // cn can be used with different inputs later


We usually put these comments above the line that is affected, not after the line (this line becomes really long now).

I thought putting it above the line will affect the whole function block not just that line in this case?

Probably, but that is not really an issue.

In general, all nolint issues need to be addressed at one point. That means, either the argument is not needed and can be dropped, or the function gets used with other arguments somewhere.

pkalever · 2021-05-11T07:16:10Z

In case a manual rebase is needed, please correct the formatting of the // nolint: comments.

The recommendation was not to have space in between the slash and nolint

Madhu-1 · 2021-05-26T07:52:22Z

@Mergifyio rebase

mergify · 2021-05-26T07:53:25Z

Command rebase: success

Branch has been successfully rebased

To validate the basic working of rbd-nbd Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

This is a negative testcase to showcase as per current design the IO will fail because of the missing mappings Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

Bringup the rbd-nbd map/attach process on the rbd node plugin and expect the IO to continue uninterrupted. Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

This testcase tests journaling/exclusive-lock image-features with rbd-nbd mounter Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

Ignoring below warnings: e2e/pod.go:207:60: `execCommandInContainer` - `cn` always receives `"csi-rbdplugin"` (unparam) func execCommandInContainer(f *framework.Framework, c, ns, cn string, opt *metav1.ListOptions) (string, string, error) { ^ e2e/pod.go:308:43: `deletePodWithLabel` - `skipNotFound` always receives `false` (unparam) func deletePodWithLabel(label, ns string, skipNotFound bool) error { Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

pkalever marked this pull request as draft January 21, 2021 11:41

nixpanic added component/testing Additional test cases or CI work dependency/k8s depends on Kubernetes features labels Jan 22, 2021

pkalever force-pushed the e2e-nbd branch from 77eb6b1 to 85ba3f9 Compare January 25, 2021 08:25

pkalever force-pushed the e2e-nbd branch from 85ba3f9 to f79ebf3 Compare January 25, 2021 08:48

pkalever force-pushed the e2e-nbd branch from f79ebf3 to 51e9bb2 Compare January 27, 2021 09:02

pkalever force-pushed the e2e-nbd branch from 467a2d1 to 3136354 Compare February 9, 2021 13:41

pkalever force-pushed the e2e-nbd branch from 3136354 to 174e6b9 Compare February 11, 2021 06:50

pkalever force-pushed the e2e-nbd branch 3 times, most recently from 3938ced to c18459a Compare February 26, 2021 09:02

Base automatically changed from master to devel March 1, 2021 05:22

pkalever force-pushed the e2e-nbd branch 7 times, most recently from 6e8a43b to 728e5df Compare March 3, 2021 10:57

humblec reviewed Mar 3, 2021

View reviewed changes

pkalever force-pushed the e2e-nbd branch 2 times, most recently from 81706a7 to 9299867 Compare March 3, 2021 12:47

pkalever requested a review from Madhu-1 May 5, 2021 06:47

pkalever force-pushed the e2e-nbd branch 2 times, most recently from 77bf75d to 1009c06 Compare May 6, 2021 06:13

pkalever commented May 6, 2021

View reviewed changes

pkalever force-pushed the e2e-nbd branch 3 times, most recently from 1972041 to 0175b5c Compare May 6, 2021 11:55

Madhu-1 approved these changes May 11, 2021

View reviewed changes

nixpanic approved these changes May 11, 2021

View reviewed changes

leseb force-pushed the e2e-nbd branch from 0175b5c to 2415787 Compare May 11, 2021 07:10

leseb force-pushed the e2e-nbd branch from 2415787 to 30a05a5 Compare May 26, 2021 07:53

Prasanna Kumar Kalever added 6 commits May 26, 2021 09:21

e2e: add a test case for rbd-nbd mounter

6aaf337

To validate the basic working of rbd-nbd Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

e2e: Test IO after nodeplugin reboot

8aa84be

This is a negative testcase to showcase as per current design the IO will fail because of the missing mappings Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

e2e: add ability to run command inside specified container

5cdc62c

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

e2e: restart rbd-nbd process after nodeplugin reboot

6095ac2

Bringup the rbd-nbd map/attach process on the rbd node plugin and expect the IO to continue uninterrupted. Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

e2e: enable an old testcase as the ndb module is available

7b7a426

This testcase tests journaling/exclusive-lock image-features with rbd-nbd mounter Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

leseb force-pushed the e2e-nbd branch from 30a05a5 to aaf5414 Compare May 26, 2021 09:22

mergify bot merged commit 6984da5 into ceph:devel May 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

e2e: add a test case for rbd-nbd mounter #1839

e2e: add a test case for rbd-nbd mounter #1839

pkalever commented Jan 21, 2021 •

edited

Loading

nixpanic commented Jan 22, 2021

pkalever commented Jan 25, 2021

nixpanic commented Jan 25, 2021

nixpanic commented Jan 25, 2021

pkalever commented Jan 25, 2021

Madhu-1 commented Jan 27, 2021

pkalever commented Jan 27, 2021

nixpanic commented Feb 10, 2021

humblec Mar 3, 2021

pkalever Mar 5, 2021

humblec Mar 5, 2021

humblec Mar 3, 2021

pkalever Mar 5, 2021

humblec Mar 5, 2021

pkalever May 6, 2021

pkalever commented May 7, 2021

humblec commented May 7, 2021

pkalever commented May 7, 2021

humblec commented May 7, 2021 •

edited

Loading

pkalever commented May 7, 2021

pkalever commented May 11, 2021

nixpanic left a comment

nixpanic May 11, 2021

pkalever May 11, 2021

nixpanic May 11, 2021

pkalever commented May 11, 2021

Madhu-1 commented May 26, 2021

mergify bot commented May 26, 2021

e2e: add a test case for rbd-nbd mounter #1839

e2e: add a test case for rbd-nbd mounter #1839

Conversation

pkalever commented Jan 21, 2021 • edited Loading

Describe what this PR does

Dependencies

nixpanic commented Jan 22, 2021

pkalever commented Jan 25, 2021

nixpanic commented Jan 25, 2021

nixpanic commented Jan 25, 2021

pkalever commented Jan 25, 2021

Madhu-1 commented Jan 27, 2021

pkalever commented Jan 27, 2021

nixpanic commented Feb 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkalever commented May 7, 2021

humblec commented May 7, 2021

pkalever commented May 7, 2021

humblec commented May 7, 2021 • edited Loading

pkalever commented May 7, 2021

pkalever commented May 11, 2021

nixpanic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkalever commented May 11, 2021

Madhu-1 commented May 26, 2021

mergify bot commented May 26, 2021

pkalever commented Jan 21, 2021 •

edited

Loading

humblec commented May 7, 2021 •

edited

Loading