Bug 1564078

Summary: [Ganesha]Pynfs WRT5b (st_write.testTooLargeData) test case is failing
Product: [Red Hat Storage] Red Hat Gluster Storage Reporter: Manisha Saini <msaini>
Component: nfs-ganeshaAssignee: Kaleb KEITHLEY <kkeithle>
Status: CLOSED NOTABUG QA Contact: Manisha Saini <msaini>
Severity: high Docs Contact:
Priority: unspecified    
Version: rhgs-3.4CC: amukherj, bfields, dang, ffilz, grajoria, jthottan, pasik, rhs-bugs, storage-qa-internal
Target Milestone: ---Keywords: ZStream
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-11-19 10:35:04 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Manisha Saini 2018-04-05 10:03:06 UTC
Description of problem:

WRT5b test in pynfs test suit is failing consistently.
Seems WRT5a and WRT5b are the new cases added in pynfs test suit.

Version-Release number of selected component (if applicable):

# rpm -qa | grep ganesha
glusterfs-ganesha-3.12.2-7.el7rhgs.x86_64
nfs-ganesha-2.5.5-3.el7rhgs.x86_64
nfs-ganesha-gluster-2.5.5-3.el7rhgs.x86_64



How reproducible:
3/3

Steps to Reproduce:
1.Create 4 node ganesha cluster
2.Create 4 x (2 + 1) = 12 Arbiter volume
3.Export the volume via nfs-ganesha
4.Run pynfs test suit from client
 ./testserver.py 10.70.46.32:/Ganeshavol1 -v --outfile ~/pynfs.run --maketree --showomit --rundep all > /tmp/pynfs.log


# cat /tmp/pynfs.log | grep FAILURE
SEC7     st_secinfo.testRPCSEC_GSS                                : FAILURE
WRT5b    st_write.testTooLargeData                                : FAILURE
WRT18    st_write.testChangeGranularityWrite                      : FAILURE


Actual results:

WRT5b  test is failing in pynfs test suit.SEC7  and WRT18  are known failures

Expected results:

WRT5b should pass

Additional info:

Comment 3 Frank Filz 2018-04-06 23:04:59 UTC
This test is a new test that expects the server to not process an overly large request and only expects the server to not crash.

I will check with Bruce Fields

Comment 4 Frank Filz 2018-04-06 23:06:51 UTC
Oops, that comment went out before I was done...

I added Bruce Fields, our older ntirpc chokes on the overly large request (upstream with newer ntirpc passes this test case), maybe we can add this failure mode as acceptable.

In any case, the failure of this test is not an issue, if we can't change the test, we can just accept this as an expected failure.

Comment 5 Manisha Saini 2018-04-07 08:31:35 UTC
(In reply to Frank Filz from comment #4)
> Oops, that comment went out before I was done...
> 
> I added Bruce Fields, our older ntirpc chokes on the overly large request
> (upstream with newer ntirpc passes this test case), maybe we can add this
> failure mode as acceptable.
> 
> In any case, the failure of this test is not an issue, if we can't change
> the test, we can just accept this as an expected failure.

This is a new test added in pynfs test suit which is failing for earlier released RHGS versions as well and is not a regression.

As discussed in nfs scrum and as mentioned in comment#4 by Frank,the failure is is not causing any functionality impact from application point of view.We can live with this in RHGS 3.4.

Comment 6 J. Bruce Fields 2018-04-09 13:56:45 UTC
This is just a bug in the pynfs test, it should succeed unconditionally.  The "except:" should probably be catching a larger set of exceptions.  (How exactly is it failing in your case?)

Comment 8 J. Bruce Fields 2018-04-09 21:11:16 UTC
So I guess RPC-level errors raise exceptions instead of returning errors.  I think this will allow pretty much anything, maybe that's what we should do:

diff --git a/nfs4.0/servertests/st_write.py b/nfs4.0/servertests/st_write.py
index b5ad291f2cba..036ccec1a5c7 100644
--- a/nfs4.0/servertests/st_write.py
+++ b/nfs4.0/servertests/st_write.py
@@ -171,7 +171,7 @@ def testTooLargeData(t, env):
         # We don't care much what the server does, this is just a check
         # to make sure it doesn't crash.
         res = c.write_file(fh, data, 0, stateid)
-    except IOError:
+    except:
         # Linux knfsd closes the socket when the write is too large.
         # That's OK.
         pass

Comment 9 Frank Filz 2018-04-10 14:01:04 UTC
(In reply to J. Bruce Fields from comment #8)
> So I guess RPC-level errors raise exceptions instead of returning errors.  I
> think this will allow pretty much anything, maybe that's what we should do:
> 
> diff --git a/nfs4.0/servertests/st_write.py b/nfs4.0/servertests/st_write.py
> index b5ad291f2cba..036ccec1a5c7 100644
> --- a/nfs4.0/servertests/st_write.py
> +++ b/nfs4.0/servertests/st_write.py
> @@ -171,7 +171,7 @@ def testTooLargeData(t, env):
>          # We don't care much what the server does, this is just a check
>          # to make sure it doesn't crash.
>          res = c.write_file(fh, data, 0, stateid)
> -    except IOError:
> +    except:
>          # Linux knfsd closes the socket when the write is too large.
>          # That's OK.
>          pass

Yea, that sounds good. The only purpose of this test case is to poke the server with something horrible and make sure it doesn't crash.

Hmm, will this detect if a server lies and says the write succeeded?

Comment 10 J. Bruce Fields 2018-04-10 15:40:30 UTC
(In reply to Frank Filz from comment #9)
> Yea, that sounds good. The only purpose of this test case is to poke the
> server with something horrible and make sure it doesn't crash.

OK, I've pushed out that pynfs change.  This bug can be closed with NOTABUG.

> Hmm, will this detect if a server lies and says the write succeeded?

I don't think it's worth it to check for that particular case.

It would be easy just to fail on a write that returns success, but on its own I'm not sure whether that's actually a server bug.