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

Server Reboot test for nfs-ganesha #27

Open
wants to merge 9 commits into
base: centos-ci
Choose a base branch
from

Conversation

grajoria
Copy link

Perform mount from client, start i/o from the client. Reboot the server. Check whether IO got resumed. Test runs on Mount V3, V4.0 and V4.1

Signed-off-by: Girjesh Rajoria [email protected]

Signed-off-by: Girjesh Rajoria <[email protected]>
#Parsing export id from volume export conf file
export_id=$(grep 'Export_Id' ${conf_file} | sed 's/^[[:space:]]*Export_Id.*=[[:space:]]*\([0-9]*\).*/\1/')

dbus-send --type=method_call --print-reply --system --dest=org.ganesha.nfsd /org/ganesha/nfsd/ExportMgr org.ganesha.nfsd.exportmgr.UpdateExport string:${conf_file} string:"EXPORT(Export_Id = ${export_id})"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not recommend UpdateExport to enable/disable ACL. Please move these checks at the beginning itself i.e, in between line:29 and line:30

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done as https://github.com/nfs-ganesha/ci-tests/blob/centos-ci/common-scripts/basic-gluster.sh which is used by other ci-tests. Should we change there also?

arch=os.getenv("CENTOS_ARCH")
count=2
server_script=os.getenv("SERVER_TEST_SCRIPT")
server_setup_script=os.getenv("SERVER_SETUP_SCRIPT")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between server_script and server_setup_script?

# disable the firewall on server, otherwise the client can not connect
cmd="""ssh -t -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no root@%s '
curl %s | bash -
'""" % (b['hosts'][0], server_setup_script)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post reboot, cant we just restart nfs-ganesha process which should take care of exporting the gluster volume too?

@grajoria
Copy link
Author

@soumyakoduri
Please review this again.

@@ -0,0 +1,110 @@
import json, urllib, subprocess, sys, os, time

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think this file should be included, can't you use the common-scripts/basic-gluster-duffy.py instead? If you made modifications, maybe those can be merged into that one?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modification in these are only required for this particular test. https://github.com/nfs-ganesha/ci-tests/blob/centos-ci/common-scripts/basic-gluster-duffy.py is used by other tests so can not modify it.

@@ -0,0 +1,209 @@
#!/bin/sh

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same counts for this one, is it different from common-scripts/basic-gluster.sh?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, all scripts should have a #! for the interpreter (/bin/sh in this case, since they're sh scripts)

Copy link

@dang dang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit.

@@ -1,209 +1,54 @@
#!/bin/sh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line, at least, should stay. Also, some kind of block comment telling what the file does...

@rakshithakamath94 rakshithakamath94 force-pushed the centos-ci branch 14 times, most recently from 245d4b6 to 19df33f Compare June 19, 2023 11:56
@rakshithakamath94 rakshithakamath94 force-pushed the centos-ci branch 2 times, most recently from 801b37d to c770b94 Compare March 28, 2024 06:29
@rakshithakamath94 rakshithakamath94 force-pushed the centos-ci branch 17 times, most recently from e9b2ca4 to a09a2e5 Compare April 24, 2024 08:00
@rakshithakamath94 rakshithakamath94 force-pushed the centos-ci branch 7 times, most recently from e93af3b to ade304e Compare July 5, 2024 05:02
@rakshithakamath94 rakshithakamath94 force-pushed the centos-ci branch 3 times, most recently from 1a86ec1 to 1ed5d17 Compare October 10, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants