Converting SMB1 tests to SMB2

[Ralph Boehme]

Hi Noel,

On 11/22/19 4:27 PM, Noel Power wrote:
> I really need help with
> https://gitlab.com/samba-team/samba/merge_requests/902
> 
> Currently it is marked WIP as I hoped there could be some
> discussion/agreement as to the approach outlined there. In summary this
> merge request imo satisfies the general discussions we had about the
> approach to providing a way to iteratively push ported smb1 tests into
> the codebase. As mentioned I didn't have much luck trying to make the
> '_smb1' test environments fully independent, in the end it seemed this
> would probably require more work than it was worth (afterall in the end
> we will get rid of the smb1 tests and associated envs).
> 
> With this in mind I only create 'shallow copies' of the test envs that
> have tests that fail against the new default environments which don't
> support negotiation of SMB1.

These won't work in make test. You need at least IP addresses.

But worse is that these 18 additional envs may severely impact load on
sn-devel. Remember, on sn-devel all this stuff runs on one machine, not
on seperate containers spread across a datacenter.

The good new is, that I don't believe we need all those new envs.

> I added 2 new CI jobs to run the tests that
> only run in environments that can negotiate SMB1. This passes CI currently
> 
> I'd really like to get this (with whatever changes are needed) upstream
> so we can start on the porting proper. David is currently starting to
> try and port some of the base tests and I started going through the
> skip_smb1_fail list we have to split tests that mix SMB1 & >= SMB2, fix
> tests that should run against SMB2 but dont't etc.
> 
> I also wanted to go through the motions of porting a test, marking it as
> ported/done etc. Ralph, you had the idea of using alias environments
> '_done' that I liked so I created a new branch
> 
> https://gitlab.com/samba-team/devel/samba/commits/npower_smb1_with_porting

This looks promising.

> This branch also includes the changes from
> https://gitlab.com/samba-team/samba/merge_requests/902, on top of those
> changes it
> 
> * creates alias '_smb1_done' envs that a test can be moved to when
> 'processed'
> * fixes or splits tests that currently only run in a test env that can
> negotiate SMB1
>   e.g. where a test that mixes testing SMB1 & >=SMB2 protocols then the
> test is modified so it can take a param to run either protocol, then
> existing test is;
>       a) modified to provide param to run SMB1 & test moved from '_smb1'
> env to '_smb1_done' env
>       b) copied & modified to provide a param to run >= SMB2 and test
> now additionally runs against appropriate non '_smb1' env

ok.

>   There are quite a few tests in the skip file that really are SMB1 only
> tests,   in this case
>       a) move the test from '_smb1' env to '_smb1_done' env

Just beware of tests where SMB1 is just the transport and the test tests
FSA layer logic, eg unlink.

>   In both cases the skip_smb1_fail file is updated with comments saying
> the test
>   has been processed
> 
> Currently ~70 tests have been 'processed' trivially in that branch. I
> fear that the work there might be wasted (if the wrong approach is being
> followed) so really it would be great to get this nailed down before
> more complex changes happen

Stepping back a little I think we only need the following:

1. The file with the list of failing tests should not be used as a skip
list. Just maintain a private branch with the changes needed to generate
the list. The list can go into master as eg selftest/smb1-todo or
similarily named.

Ideally the commit that adds the list also included instructions and the
patchset as file that is needed to regenerate the list on sn-devel.

2. Don't disable SMB1 in the ad_dc_ntvfs fileserver. This also applies
when generating the todo list in step 1.

3. Reduce the number of envs that use the s4 NTVFS fileserver. This will
get us rid of a few failing tests from the list. I'm currently running
pipeplines with patches that implement this:

https://gitlab.com/samba-team/devel/samba/pipelines/98598769
https://gitlab.com/samba-team/devel/samba/commits/slow-ad_dc_ntvfs

With 2 and 3 together ~200 test will be removed from the list:

$ 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.

5. Add correspondong _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

   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.

8. Again go through the todo list and convert the test to work with
smb1, updating the todo list with an env name change _smb1 -> smb1_done.

Steps 6 and 8 can be intertwined.

metze, does this sound like what we discussed? Did I miss anything?

-slow

-- 
Ralph Boehme, Samba Team                https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG-Fingerprint   FAE2C6088A24252051C559E4AA1E9B7126399E46