Converting SMB1 tests to SMB2
NoPower at suse.com
Tue Nov 26 16:09:27 UTC 2019
On 26/11/2019 11:45, Ralph Boehme via samba-technical wrote:
> Hi Noel,
> On 11/22/19 4:27 PM, Noel Power wrote:
>> I really need help with
>> 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.
yes that is true, however I am not sure even if these days make test
normally will work (or if anyone actually even use it instead of autobuild)
certainly during the python3 transition I am sure make test couldn't
work as tests in the same job even were interfering with each other
resulting in some tests split across various jobs. I would nearly bet
there are currently tests that if all run together with a 'raw' make
test would fail, I'd try it except I never ever had make test work for
> 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.
I didn't think about this, this is a valid concern and certainly this
could be an issue
> 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
> 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
>> * 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
>> 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.
? not entirely sure what you mean here, I guess what I was referring to
there are many tests in the skip file that already have both SMB1 & >=
SMB2 tests where the test name just differs in some indicator of the
protocol used e.g. test.something.NT1 & test.something.SMB2, in these
cases nothing needs to be happen other than just mark the test done
>> 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.
currently the skip file is effectively documentation, the commits around
this at the start of the patch set are just to prove it was a valid list
(after migrating the envs to default only negotiate >= SMB2)
There are no 'changes' to generate the the list of skip tests other than
making all envs by default not negotiate SMB1 and then trying to see
what fails (and unfortunately this isn't as straight forward as running
the tests with FAIL_IMMEDIATELY=0 as some tests hang etc.
So I am not sure which changes you mean should be in a private branch. I
guess I misunderstand what you mean
> 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:
> With 2 and 3 together ~200 test will be removed from the list:
I know you mentioned previously ignoring for the moment ad_dc_ntvfs
tests but I had already identified the ones failing already so I didn't
see the harm in having them in the skip file (and having the environment
to run them) as they need to be dealt with eventually.
However If it helps to reduce environments then of course removing them
entirely makes sense.
Removing ad_dc_ntvfs from the mix afaics removes ~10 environments e.g.
> $ egrep 'ad_dc_ntvfs|ad_dc_default|ad_dc_slowtest|chgdc'
> selftest/skip_smb1_fails | wc -l
> 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)
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
> 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
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, 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.
I wonder how far off we would be if I merged your slow-ad_dc_ntvfs in
terms of what extra (if any) envs are needed, I will try
> 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?
More information about the samba-technical