Dummy driver breaks expected keystone functionality

[Boris Bobrov]

As part of fixing https://bugs.launchpad.net/mos/+bug/1566802, a "dummy" driver for revocations was introducted.

It should not be done. Using dummy driver is a bad approach. It removes a major part of functionality even with Fernet tokens. Here is the script for testing: http://paste.openstack.org/show/506324/ . It disables a domain, and the token with the disabled domain should be invalid. But with the dummy driver, the token is still valid.

There is another bad case i see -- token revoked by id. Or by audit_id. This is a major part of keystone functionality and it should not be cut out.

Both changes to master and stable/mitaka should be reverted. There should be another fix. Cutting out functionality from keystone is a bad solution to performance problems. Fuel should not be shipped with broken keystone.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/322933

[Ivan Berezovskiy (iberezovskiy)]

Keystone team, please find consensus about this and https://bugs.launchpad.net/mos/+bug/1566802/ bugs.

[Dina Belova (dbelova)]

Setting to Incomplete until future of https://bugs.launchpad.net/mos/+bug/1566802/ will be decided

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/323395

[Boris Bobrov (bbobrov)]

At the meeting with MOS-Keystone and scale team we decided that the patch to master needs to be reverted unconditionally. The patch for stable/mitaka needs to be checked on the scale lab, but without waiting for the merge process; however, it needs to be reverted too, because it breaks major functionality in keystone.

[Ivan Berezovskiy (iberezovskiy)]

Actually, patch for master doesn't use dummy driver, we've left only a comment that it should be used later (https://review.openstack.org/#/c/321086/). We'll wait for 9.0 test result, then will propose a revert.

[Alexander Makarov (amakarov)]

Ivan, this patch uses: https://review.openstack.org/#/c/321058/

[Denis Egorenko (degorenko)]

Alexander, then it was reverted here: https://review.openstack.org/#/c/322933/

[Alexander Makarov (amakarov)]

Not yet - it isn't merged

[Sofiia Andriichenko (sandriichenko)]

Blocked for 17 keystone api tempest tests

[Timur Nurlygayanov (tnurlygayanov)]

Dev team, could you please prepare the fix for 9.0?

[Ivan Berezovskiy (iberezovskiy)]

As it was discussed, keystone team should provide test results with previous revoke driver (not with dummy). When it's done, patch with switching to dummy driver should be reverted. Actually, revert patch is there https://review.openstack.org/#/c/322933/. I don't understand why assignee for 9.0 is MOS-puppet.

[Boris Bobrov (bbobrov)]

Because the patch for 9.0 must be reverted anyway. It is a tempest blocker, please see the tags. Even if the dummy driver fixes an issue in https://bugs.launchpad.net/mos/+bug/1566802, it creates another critical issue.

https://review.openstack.org/#/q/topic:bug/1566802 -- here are all original patches and their reverts.

[Ivan Berezovskiy (iberezovskiy)]

MOS puppet team won't revert anything till test results are not provided by keystone team (as it was discussed).
@Boris, actually you've already prepared those reverts, so I don't understand why assignee is still MOS Puppet.

[Boris Bobrov (bbobrov)]

I am not able to maintain patches in puppet since i don't know puppet. If you want, i will abandon the patches so that you could press the buttons yourself.

The patches need to be reverted regardless of the testing results.

[Timur Nurlygayanov (tnurlygayanov)]

Ok, QA team will test the related fix (with reverted commit) but it is important to note that we have -1 fro commit on review in 9.0 branch.

[Timur Nurlygayanov (tnurlygayanov)]

Here is the custom build with the fix (revert) for 9.0: https://custom-ci.infra.mirantis.net/view/9.0/job/9.0.custom.iso/197/

[Timur Nurlygayanov (tnurlygayanov)]

here is the fix for 9.0: https://review.openstack.org/#/c/322933/. Status changed to In Progress.

[Oleksiy Butenko (obutenko)]

I change in keystone.conf value driver to
"keystone.contrib.revoke.backends.sql.Revoke"

all tempest tests pass
======
Totals
======
Ran: 182 tests in 1772.0000 sec.
 - Passed: 182
 - Skipped: 0
 - Expected Fail: 0
 - Unexpected Success: 0
 - Failed: 0
Sum of execute time for each test: 463.6996 sec.

[Denis Egorenko (degorenko)]

The problem is not in keystone working, the problem is in unreliability on
big clusters [1].

Are we going to solve this issue or just to confirm that old driver works?

[1] https://bugs.launchpad.net/mos/+bug/1566802

2016-06-01 18:54 GMT+03:00 Oleksiy Butenko <email address hidden>:

> I change in keystone.conf value driver to
> "keystone.contrib.revoke.backends.sql.Revoke"
>
> all tempest tests pass
> ======
> Totals
> ======
> Ran: 182 tests in 1772.0000 sec.
> - Passed: 182
> - Skipped: 0
> - Expected Fail: 0
> - Unexpected Success: 0
> - Failed: 0
> Sum of execute time for each test: 463.6996 sec.
>
> --
> You received this bug notification because you are a member of MOS
> Puppet Team, which is a bug assignee.
> https://bugs.launchpad.net/bugs/1587136
>
> Title:
> Dummy driver breaks expected keystone functionality
>
> Status in Fuel for OpenStack:
> In Progress
> Status in Fuel for OpenStack mitaka series:
> In Progress
> Status in Fuel for OpenStack newton series:
> In Progress
>
> Bug description:
> As part of fixing https://bugs.launchpad.net/mos/+bug/1566802, a
> "dummy" driver for revocations was introducted.
>
> It should not be done. Using dummy driver is a bad approach. It
> removes a major part of functionality even with Fernet tokens. Here is
> the script for testing: http://paste.openstack.org/show/506324/ . It
> disables a domain, and the token with the disabled domain should be
> invalid. But with the dummy driver, the token is still valid.
>
> There is another bad case i see -- token revoked by id. Or by
> audit_id. This is a major part of keystone functionality and it should
> not be cut out.
>
> Both changes to master and stable/mitaka should be reverted. There
> should be another fix. Cutting out functionality from keystone is a
> bad solution to performance problems. Fuel should not be shipped with
> broken keystone.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/fuel/+bug/1587136/+subscriptions
>

--
Best Regards,
Egorenko Denis,
Senior Deployment Engineer
Mirantis

[Dina Belova (dbelova)]

Therefore we are not reverting until we'll ensure this code (reverted one) will work ok on the scale labs. Thanks Oleksiy for checking it from the tempest perspective.

[Boris Bobrov (bbobrov)]

OK, here is a better solution for you if you are happy with removing parts of code: remove keystone completely. Everything will work smoothly. And when neutron breaks, remove neutron and replace it with "dummy" neutron.

But you can't do it because you will leave yourself without authn and authz. Or without networking.

For exactly the same reason, both patches need to be reverted regardless of what testing shows. We cannot leave ourselves without revocations. Cutting out important functionality is not the way to go.

[Timur Nurlygayanov (tnurlygayanov)]

Boris, does it mean that it is not enough to just revert the commits with the issues? could you please describe the proper solution for the issue?

Thank you!

[Boris Bobrov (bbobrov)]

> Boris, does it mean that it is not enough to just revert the commits with the issues?

Reverting is enough.

[Denis Egorenko (degorenko)]

What then should we do with bad performance on big clusters?

2016-06-02 12:41 GMT+03:00 Boris Bobrov <email address hidden>:

> > Boris, does it mean that it is not enough to just revert the commits
> with the issues?
>
> Reverting is enough.
>
> --
> You received this bug notification because you are a member of MOS
> Puppet Team, which is a bug assignee.
> https://bugs.launchpad.net/bugs/1587136
>
> Title:
> Dummy driver breaks expected keystone functionality
>
> Status in Fuel for OpenStack:
> In Progress
> Status in Fuel for OpenStack mitaka series:
> In Progress
> Status in Fuel for OpenStack newton series:
> In Progress
>
> Bug description:
> As part of fixing https://bugs.launchpad.net/mos/+bug/1566802, a
> "dummy" driver for revocations was introducted.
>
> It should not be done. Using dummy driver is a bad approach. It
> removes a major part of functionality even with Fernet tokens. Here is
> the script for testing: http://paste.openstack.org/show/506324/ . It
> disables a domain, and the token with the disabled domain should be
> invalid. But with the dummy driver, the token is still valid.
>
> There is another bad case i see -- token revoked by id. Or by
> audit_id. This is a major part of keystone functionality and it should
> not be cut out.
>
> Both changes to master and stable/mitaka should be reverted. There
> should be another fix. Cutting out functionality from keystone is a
> bad solution to performance problems. Fuel should not be shipped with
> broken keystone.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/fuel/+bug/1587136/+subscriptions
>

--
Best Regards,
Egorenko Denis,
Senior Deployment Engineer
Mirantis

[Timur Nurlygayanov (tnurlygayanov)]

So then just let's merge the commit with revert when performance team will vote +1.

[Boris Bobrov (bbobrov)]

> What then should we do with bad performance on big clusters?

Get logs and investigate further.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/323395
Committed: https://git.openstack.org/cgit/openstack/fuel-library/commit/?id=105dfe5ccebdf3efd99e28331a0e2bb7b616d602
Submitter: Jenkins
Branch: master

commit 105dfe5ccebdf3efd99e28331a0e2bb7b616d602
Author: Boris Bobrov <email address hidden>
Date: Tue May 31 13:58:39 2016 +0000

    Revert "Prepare changing of keystone revoke driver"

    This reverts commit 6bbaf28eb0872b8b85f82e7d610ad666453da006.
    Closes-Bug: 1587136

    Change-Id: I4dfbff2bb8d72704b6a012159107711f09ddd666

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/322933
Committed: https://git.openstack.org/cgit/openstack/fuel-library/commit/?id=cd7898d78afce27ff15a45f1bae0f2178b97b9e2
Submitter: Jenkins
Branch: stable/mitaka

commit cd7898d78afce27ff15a45f1bae0f2178b97b9e2
Author: Boris Bobrov <email address hidden>
Date: Mon May 30 17:11:58 2016 +0000

    Revert "Change keystone revoke driver"

    This reverts commit 8a438dbf75ed97cdba91649c2a4b0e28fe0bb87a.
    The change being reverted removes major part of keystone functionality.

    Change-Id: Ib704ca197c2fa9c9ee9bd8f0b3e86e53c7803362
    Closes-Bug: 1587136

[Sofiia Andriichenko (sandriichenko)]

Checked on ISO 448

[Sofiia Andriichenko (sandriichenko)]

reproduced for 9.1 snapshot #69

[Ivan Berezovskiy (iberezovskiy)]

@sandriichenko, please attach diagnostic snapshot and provide env configuration

[Sofiia Andriichenko (sandriichenko)]

Unfortunately I can't attach diagnostic snapshot.
Tempest test test_delete_user_request_without_a_token is failed,
Trace: http://paste.openstack.org/show/545095/

[Ivan Berezovskiy (iberezovskiy)]

@sandriichenko, I don't understand what common has your failed tested with bug with dummy driver usage? Dummy driver isn't used, so it can't lead to any failures. I suggest you to file another bug with appropriate description.

[TatyanaGladysheva (tgladysheva)]

Verified on 10.0 build #1558.