Converting SMB1 tests to SMB2

[Noel Power]

On 03/12/2019 14:20, Ralph Boehme wrote:
> Hi Noel,
>
> sorry for late reply, but I've got my head wrapped in
> <https://gitlab.com/samba-team/samba/merge_requests/937>.
understood
>
> Sure. Btw, how did you figure out which tests hang?
Looking in the CI logs
>  Do you have a list
> of those?
In the last mail I sent, there were instructions for generating the list
tests that fail when all the envs by default do not negotiate SMB1, part
of the instructions included an initial skip list, that list was
attached to the mail, this list also is the list of hanging tests. As
mentioned my impression was the amount of hanging tests was greatly
reduced by your ntvfs branch.
>  I imagine only a few tests will hang, luckily most of them
> because it's in one of the AD DC envs where we don't have to disable
> SMB1 as discussed in MR
looks like most (if not all) are in this category
>
> https://gitlab.com/samba-team/samba/merge_requests/941
>
> Ideally we can make the skiplist a knownfail list. Then every commit
> that moves a failing test to a new *_smb1 env can comment out the list
> entry.
well that would mean starting from a point where we have tests not
running, I don't really like that
>> Removing ad_dc_ntvfs from the mix afaics removes ~10 environments e.g.
>>
>> directly dependant
>>
>> ad_dc_default
>> ad_dc_slowtests
>> s4member
>> rpc_proxy
>> vampire_dc
>>
>> indirectly dependant
>>
>> chgdcpass
>> s4member
>> fl2000dc
>> fl2003dc
>> fl2008r2d
> of these only ad_dc_ntvfs, s4member and rpc_proxy should still use the
> s4 NTVFS fileserver, all other should use s3 smbd. Alongside, these
> three envs will remain with smb1 enabled.
I'll generate a new fail list based on your ntfvs changes plus changes
to disable SMB1 negotiation (except for those three)
>
> And most tests that fail against any of the other environments with smb1
> disabled needs checking why this happens and not just be moved to a
> *_smb1 env. Eg "samba4.blackbox.kinit_trust(fl2008r2dc_smb1:local)"
> seems to check some kerberos stuff, so this shouldn't be affected by
> disabling smb1. As there are only a few of those test failing in these
> envs, we should investigate this upfront.
I expect this is due to use of smbclient4
>
>>> $ egrep 'ad_dc_ntvfs|ad_dc_default|ad_dc_slowtest|chgdc'
>>> selftest/skip_smb1_fails  | wc -l
>>> 199
>>>
>>> 4. Add three new envs that support smb1: ad_dc_smb1, ad_member_smb1,
>>> fileserver_smbd1, maybe more if featurewise required.
>> when you say adding envs do you mean just 'shallow' copies or do we
>> really need to try and go the extra mile to make these new environments
>> properly independent (my previous attempt failed and it looked like
>> perhaps alot of changes could ripple from trying this for little gain
>> since the environments will be removed)
> yes, just normal/full envs.
I do believe (from my previous experience) that this is a can of worms,
admittedly I may have been trying to do this the wrong way or just don't
see the 'easy' solution, I'll try again
>
>> also afaics instead of 3 we probably need 8 unless you mean some tests
>> can be moved from existing envs to one of the above
> Yes, we should move/consolidate as much as possible.
>
> I'd start with the smbtorture tests and move the in a microcommits fashion.
>
> Running make -j test TESTS=".*smb1" > tests.log 2>&1
> on your branch from MR902 on sn-devel and processing the log to see
> which smb1 envs are used, I get
>
> $ cat tests.log | sed 's/.*(\(.*\)).*/\1/' tests.txt | \
>   sed s/:local// | sed s/:client// | sort -u
> ad_dc_default_smb1
> ad_dc_ntvfs_smb1
> ad_dc_slowtests_smb1
> ad_dc_smb1
> ad_member_smb1
> chgdcpass_smb1
> fileserver_smb1
> fl2000dc_smb1
> fl2003dc_smb1
> fl2008r2dc_smb1
> maptoguest_smb1
> nt4_dc_schannel_smb1
> nt4_dc_smb1
> nt4_member_smb1
> rpc_proxy_smb1
> s4member_smb1
> simpleserver_smb1
> vampire_dc_smb1
>
> Of these only
>
> ad_dc_smb1
> ad_member_smb1
> fileserver_smb1
>
> are really needed plus possibly nt4_dc_smb1 to avoid many changes to
> tests.py
>
> As for the other ones, given my MR to tweask the AD DC envs gets merged:
>
> ad_dc_default_smb1 -> should be an alias to ad_dc_smb1
> ad_dc_ntvfs_smb1 -> not needed, ad_dc_ntvfs has smb1 enabled
> ad_dc_slowtests_smb1 ->  not needed, failing tests must be fixed upfront
> chgdcpass_smb1 ->  not needed, failing tests must be fixed upfront
> fl2000dc_smb1 ->  not needed, failing tests must be fixed upfront
> fl2003dc_smb1 ->  not needed, failing tests must be fixed upfront
> fl2008r2dc_smb1 ->  not needed, failing tests must be fixed upfront
> maptoguest_smb1 ->  not needed, failing tests must be fixed upfront
> nt4_dc_schannel_smb1 ->  not needed, failing tests must be fixed upfront
> nt4_member_smb1 -> fix tests upfront
> rpc_proxy_smb1 -> not needed, still has smb1
> s4member_smb1 -> dito
> simpleserver_smb1 -> move tests to fileserver env
> vampire_dc_smb1 -> fix tests upfront
>
>
>>> 5. Add coresspondong _smb1_done alias envs.
>>>
>>> 6. Go through the todo list, either
>>>    a) just fixing the test if it should genuinely
>>>       work with smb2 => remove test from todo list
>>>    b) convert failing smb1 tests to use the
>>>       envs from 4 => update used test env in todo list
>> I guess this answers my question above wrt. above
>>>    Step 6 can be spread across volunteers.
>>>
>>> 7. When the list is fully processed and all remaining tests on the todo
>>> list that still need smb1 use one of the new _smb1 envs, disable smb1 in
>>> all other envs except ad_dc_ntvfs and s4member.
>> I don't like that disabling smb1 by default for the 'normal'
>> environments is now a final step, 
> yeah, that's true, but I see no other way. We must split
> e50b9b9ebddef304caf232c92b8fc83bc1003b1e into pieces, so I don't see a
> way without breaking bisectabiliry having smb1 disabled in the envs
> causing test failures in the commit series.
>
>
>> that isn't as nice as starting from an
>> initial point where we can clearly see what runs >=SMB2 and what
>> doesn't, not only see but actually know (because the default env setup
>> would prevent any accidently SMB1 usage) and similarly when you port a
>> test you know it isn't still using some SMB1 that you didn't notice.
> Well, we'll notice at the end and are forces to fixup then anyway.
>
> The test
>
> samba4.ldap.vlv.python(ad_dc_slowtests)_smb1(ad_dc_slowtests_smb1)
>
> is another example of a test failure that must be investigated instead
> of adding a new env.

I think quite of few of those tests wont exist with the new ad_dc_ntvfs
reorg reality (when it hits)

thanks,

Noel