On 11/04/2015 04:25 PM, Michal Židek wrote:
On 11/03/2015 06:30 PM, Nikolai Kondrashov wrote:
> Now, I think I got confused here. I'm not actually testing negative
> cache, but
> only the fact that the changes to LDAP don't appear before the caches
> expire.
> Since the negative cache timeout equals the timeout of positive caches,
> there
> are really no negative cache-specific tests.
>
> That said, I can still remove the tests you mention in your patch, if you
> think we should remove them. However, just for the record, I didn't see any
> issues with them.
>
> Another thing is, I couldn't make the tests work with those sleep(4)'s
> removed
> as you did in your patch up-thread. In fact all *add_remove* tests failed
> except the add_remove_user_memcache_mix (delay results are not checked
> there).
>
> The problem being, the assertions following these delays check
> enumeration and
> enumeration timeout is still 4 seconds, so we need to wait for it. For that
> matter removing tests you mention doesn't have an impact on execution time.
I see where the confusion comes from now. But the enumeration should not
play a role here. You can think about enumeration as forced sysdb fill
operation, but it is not necessary in order to grab user from LDAP.
If the user is not found in the sysdb (and also not found in the
negcache) SSSD grabs the user from LDAP even before the next
enumeration.
Sure. However, ent.assert_passwd does pwd.getpwall, not only pwd.getpwnam and
pwd.getpwuid. Pwd.getpwall result check is the one that fails.
Ent.assert_group behaves similarly.
I think I remember Jakub asking to have this test without enumeration as well,
and I can do that, but I think enumeration test is also valuable.
> I'm not quite sure how you made it work. Was some other
change omitted from
> your patch accidentally? I'm talking about changes like this:
I do not think I omitted something. Did you really set
the negative_cache to 0 in sssd.conf?
Yes. I tried the exact patch you attached.
>
> def create_sssd_cleanup(request):
> @@ -462,12 +464,9 @@ def user_and_groups_rfc2307_bis(request, ldap_conn):
> def test_add_remove_user(ldap_conn, blank_rfc2307):
> """Test user addition and removal are reflected by
SSSD"""
> e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 1001, 2000)
> - time.sleep(2)
> # Add the user
> ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Here you fill the negative cache.
> ldap_conn.add_s(*e)
> - ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Here you are testing the negative cache. If there was no negcache
hit, SSSD would grab the user from LDAP.
Ah, yes, it seems this should fetch from the negative cache.
> - time.sleep(4)
^^^^^^^^^^^^^
So this sleep should not be necessary if the negcache is disabled.
> ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001)))
> # Remove the user
> ldap_conn.delete_s(e[0])
>
> All in all, setting memcache_timeout to zero and commenting-out memcache
> file
> removal was enough to make *all* tests work fine.
>
That is good! If the patches do not work for you without the sleep()s
you can send them with the sleep()s for now and I will take a look.
Alright.
Nick