Under certain conditions check_rules is very sluggish

[Mateusz Kowalski]

In Horizon we observe in certain projects function check_rules() taking up to 10 seconds while in the others maximum 2 seconds.

In order to remedy this, we would like to have check_rules function executed only if oslo.policy is configured to check the syntax, i.e. to introduce a config parameter defaulting to True but with a possibility to disable it. In that case (operator setting it to False) there wouldn't be any checks from oslo.policy side if the syntax of provided JSON file is correct.

Current behavior shouldn't be changed, i.e. syntax checks should be opt-out.

==========================================================================

SRU for UCA Queens

[Impact]
In Horizon, Admin->Network->Networks page takes longer time to load. One of the improvements is to avoid any redundant policy checks which consume time.

This fix optimizes the policy logic to avoid any redundant policy checks and further enhancing API response times.

[Test Case]
Create 240 networks and observe page load time before and after fix

1. Collect information before fix

1a. Increase network and subnet quota for the admin project
    openstack quota set --networks -1 <project id>
    openstack quota set --subnets -1 <project id>

1b. Create 240 networks using openstack cli
    for i in {1..240}; do openstack network create test$i; openstack subnet create --subnet-range 10.1.$i.0/24 --network test$i test$i; done

1c. Login to dashboard as admin user in a preferred browser
1d. Open Developer options --> Network tab. This should show all the outgoing network traffic information from browser.
1e. Open Admin->Network->Networks page
1f. Note down the time taken for /admin/networks in Network tab of browser
1g. Repeat 1c 3-5 times and take average time taken for /admin/networks

2. Install the package with the fixed code and restart apache service

3. Collect information after fix

3a. Repeat steps 1c-1g

3b.You should observe an improvement in time reduction of /admin/networks after the fix.

[Regression Potential]

Given the following indicators, the regression potential is negligible

a. Upstream CI passed with all tempest test cases passed. Indicates no break in functionality.
b. The fix is available on releases Rocky, Stein, Train since a year ago and no problems reported with this functionality.

There will be downtime of milliseconds during restart of apache services.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/511426
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=909a1ea3a7aceb6e0637058b9c6a53d14043d6d1
Submitter: Zuul
Branch: master

commit 909a1ea3a7aceb6e0637058b9c6a53d14043d6d1
Author: Mateusz Kowalski <email address hidden>
Date: Tue Oct 24 09:32:45 2017 +0200

    Avoid redundant policy syntax checks

    Introduce a private variable inside Enforcer class to remember
    status of the last policy syntax checks in order to avoid
    redundant calls to the check_rules() method.

    Having this flag makes the whole rules mechanism faster, as under
    certain conditions check_rules() method was being executed
    multiple times even when not needed.

    Change-Id: Id3992fc0cb567451049a12ebdc6851e737573bb8
    Closes-bug: #1723030
    Co-Authored-By: Ben Nemec <email address hidden>

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/583968

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/584082

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/oslo.policy 1.38.1 release.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/583968
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=77d30c3b5e06a4cc90195ca6f352b26c4bf2d2d8
Submitter: Zuul
Branch: stable/queens

commit 77d30c3b5e06a4cc90195ca6f352b26c4bf2d2d8
Author: Mateusz Kowalski <email address hidden>
Date: Tue Oct 24 09:32:45 2017 +0200

    Avoid redundant policy syntax checks

    Introduce a private variable inside Enforcer class to remember
    status of the last policy syntax checks in order to avoid
    redundant calls to the check_rules() method.

    Having this flag makes the whole rules mechanism faster, as under
    certain conditions check_rules() method was being executed
    multiple times even when not needed.

    Change-Id: Id3992fc0cb567451049a12ebdc6851e737573bb8
    Closes-bug: #1723030
    Co-Authored-By: Ben Nemec <email address hidden>
    (cherry picked from commit 909a1ea3a7aceb6e0637058b9c6a53d14043d6d1)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/584082
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=3e3820600174a67933cc1dd986066354bf3c25d3
Submitter: Zuul
Branch: stable/pike

commit 3e3820600174a67933cc1dd986066354bf3c25d3
Author: Mateusz Kowalski <email address hidden>
Date: Tue Oct 24 09:32:45 2017 +0200

    Avoid redundant policy syntax checks

    Introduce a private variable inside Enforcer class to remember
    status of the last policy syntax checks in order to avoid
    redundant calls to the check_rules() method.

    Having this flag makes the whole rules mechanism faster, as under
    certain conditions check_rules() method was being executed
    multiple times even when not needed.

    Change-Id: Id3992fc0cb567451049a12ebdc6851e737573bb8
    Closes-bug: #1723030
    Co-Authored-By: Ben Nemec <email address hidden>
    (cherry picked from commit 909a1ea3a7aceb6e0637058b9c6a53d14043d6d1)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/oslo.policy 1.33.2 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/oslo.policy 1.25.3 release.

[Lars Erik Pedersen (pedersen-larserik)]

Why is the 1.33.2 release not in the UCA for quees?

[Hemanth Nakkina (hemanth-n)]

Debdiff for xenial attached

[Hemanth Nakkina (hemanth-n)]

Attached debdiffs for xenial

[Corey Bryant (corey.bryant)]

This is fixed in Ubuntu as of Rocky. Needs fixing prior to Rocky.

[Corey Bryant (corey.bryant)]

@Hemanth, that debdiff unfortunately doesn't map to the version of python-oslo.policy that we have in xenial.

[Corey Bryant (corey.bryant)]

I've uploaded new versions of python-oslo.policy to the bionic unapproved queue [1] where it is awaiting review by the SRU team, and have uploaded to ocata-staging [2] where it will build and be promoted to ocata-proposed for testing.

[1] https://launchpad.net/ubuntu/bionic/+queue?queue_state=1&queue_text=python-oslo.policy
[2] https://launchpad.net/~ubuntu-cloud-archive/+archive/ubuntu/ocata-staging/+packages?field.name_filter=python-oslo.policy&field.status_filter=published&field.series_filter=

[Corey Bryant (corey.bryant)]

If someone wants to help get a xenial patch working that would be very much appreciated. 'pull-lp-source python-oslo.policy xenial' will get the package version for xenial.

[Hemanth Nakkina (hemanth-n)]

@Corey, Attached debdiff in #11 is UCA queens not xenial, sorry for the confusion.

[Hemanth Nakkina (hemanth-n)]

@Corey SRU is required only for UCA queens

[Corey Bryant (corey.bryant)]

Hello Mateusz, or anyone else affected,

Accepted python-oslo.policy into ocata-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:ocata-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-ocata-needed to verification-ocata-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-ocata-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Hemanth Nakkina (hemanth-n)]

Verified the test case on ocata and is successful.

/admin/networks took 11s-14s without the fix and took 10s-11s after the fix is applied. (The environment is built with stsstack-bundle with series xenial and release ocata).

[Timo Aaltonen (tjaalton)]

Hello Mateusz, or anyone else affected,

Accepted python-oslo.policy into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-oslo.policy/1.33.1-0ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted python-oslo.policy (1.33.1-0ubuntu2) for bionic have finished running.
The following regressions have been reported in tests triggered by the package:

cinder/2:12.0.9-0ubuntu1 (armhf)
gnocchi/4.2.5-0ubuntu1 (armhf)
manila/1:6.0.0-0ubuntu1 (s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/bionic/update_excuses.html#python-oslo.policy

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Hemanth Nakkina (hemanth-n)]

Verified the test case on Bionic and is successful.

/admin/networks took 17.5-19.5 seconds without the fix and 16.5-17.5 seconds with the fix. (Admin->Network->Networks page is clicked 10 times in each case to get better average)
The test environment is built with stsstack-bundle with series Bionic

[Corey Bryant (corey.bryant)]

Hello Mateusz, or anyone else affected,

Accepted python-oslo.policy into queens-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:queens-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-queens-needed to verification-queens-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-queens-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Hemanth Nakkina (hemanth-n)]

Verified the test case on xenial queens and is successful.

/admin/networks took 16-17.5 seconds without the fix and 14.7-16 seconds with the fix. (Admin->Network->Networks page is clicked 10 times in each case to get better average)
The test environment is built with stsstack-bundle with series xenial and release queens.

[Launchpad Janitor (janitor)]

This bug was fixed in the package python-oslo.policy - 1.33.1-0ubuntu2

---------------
python-oslo.policy (1.33.1-0ubuntu2) bionic; urgency=medium

  * d/gbp.conf: Create stable/queens branch.
  * d/p/avoid-redundant-policy-syntax-checks.patch: Cherry-picked from
    upstream to avoid redundant policy checks and improve dashboard
    performance (LP: #1723030).

 -- Corey Bryant <email address hidden> Thu, 26 Apr 2018 13:35:31 -0400

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for python-oslo.policy has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for python-oslo.policy has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

This bug was fixed in the package python-oslo.policy - 1.33.1-0ubuntu2~cloud0
---------------

 python-oslo.policy (1.33.1-0ubuntu2~cloud0) xenial-queens; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 python-oslo.policy (1.33.1-0ubuntu2) bionic; urgency=medium
 .
   * d/gbp.conf: Create stable/queens branch.
   * d/p/avoid-redundant-policy-syntax-checks.patch: Cherry-picked from
     upstream to avoid redundant policy checks and improve dashboard
     performance (LP: #1723030).

[Corey Bryant (corey.bryant)]

The verification of the Stable Release Update for python-oslo.policy has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Corey Bryant (corey.bryant)]

This bug was fixed in the package python-oslo.policy - 1.18.0-0ubuntu1~cloud1
---------------

 python-oslo.policy (1.18.0-0ubuntu1~cloud1) xenial-ocata; urgency=medium
 .
   * d/p/avoid-redundant-policy-syntax-checks.patch: Cherry-picked from
     upstream to avoid redundant policy checks and improve dashboard
     performance (LP: #1723030).