Bug 1504560 - [RFE] Collect Container Project Quota Historical data in Project Roll-up
Summary: [RFE] Collect Container Project Quota Historical data in Project Roll-up
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: Providers
Version: 5.8.0
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: GA
: 5.10.0
Assignee: Beni Paskin-Cherniavsky
QA Contact: juwatts
URL:
Whiteboard: container
Depends On:
Blocks: 1559544
TreeView+ depends on / blocked
 
Reported: 2017-10-20 08:24 UTC by Loic Avenel
Modified: 2019-02-11 13:54 UTC (History)
8 users (show)

Fixed In Version: 5.10.0.0
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1559544 (view as bug list)
Environment:
Last Closed: 2019-02-11 13:54:31 UTC
Category: ---
Cloudforms Team: Container Management
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Loic Avenel 2017-10-20 08:24:19 UTC
Description of problem: Collect Container Project Quota Historical data in Project Roll-up

Comment 2 Dave Johnson 2017-10-20 08:44:20 UTC
Please assess the impact of this issue and update the severity accordingly.  Please refer to https://bugzilla.redhat.com/page.cgi?id=fields.html#bug_severity for a reminder on each severity's definition.

If it's something like a tracker bug where it doesn't matter, please set the severity to Low.

Comment 6 CFME Bot 2017-10-26 17:55:57 UTC
New commit detected on ManageIQ/manageiq-schema/master:
https://github.com/ManageIQ/manageiq-schema/commit/fbf007721fad587641ddcdf7fabf9c45c81f697a

commit fbf007721fad587641ddcdf7fabf9c45c81f697a
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Thu Oct 26 14:57:58 2017 +0300
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Thu Oct 26 20:17:12 2017 +0300

    Add deleted_on (with index) to container_quotas, container_quota_items
    
    We need to store history of quotas during project lifetime,
    and are going to (ab)use archiving to store history in same tables -
    not just when quota is deleted but also when it's changed.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 ...171026114327_add_deleted_on_to_container_quota_and_items.rb | 10 ++++++++++
 db/schema.yml                                                  |  4 ++++
 2 files changed, 14 insertions(+)
 create mode 100644 db/migrate/20171026114327_add_deleted_on_to_container_quota_and_items.rb

Comment 7 CFME Bot 2017-10-27 16:43:25 UTC
New commit detected on ManageIQ/manageiq-schema/master:
https://github.com/ManageIQ/manageiq-schema/commit/c8f0a5b53880cbb27f7b46706ea7da2ab4bae7ed

commit c8f0a5b53880cbb27f7b46706ea7da2ab4bae7ed
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Thu Oct 26 14:02:55 2017 +0300
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Fri Oct 27 00:17:22 2017 +0300

    Create container_quota_scopes table
    
    https://kubernetes.io/docs/concepts/policy/resource-quotas/#quota-scopes
    We don't yet store them, but should.
    ContainerQuota has_many (0 or more) container_quota_scopes.
    Each scope is a string (currently enum with 4 possible values).
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 db/migrate/20171026142653_create_container_quota_scopes.rb | 10 ++++++++++
 db/schema.yml                                              |  7 +++++++
 2 files changed, 17 insertions(+)
 create mode 100644 db/migrate/20171026142653_create_container_quota_scopes.rb

Comment 8 Beni Paskin-Cherniavsky 2017-10-29 14:46:49 UTC
one of these migrations broke everyone, fixing in
https://github.com/ManageIQ/manageiq-schema/pull/115

Comment 9 CFME Bot 2017-10-30 15:18:39 UTC
New commit detected on ManageIQ/manageiq-schema/master:
https://github.com/ManageIQ/manageiq-schema/commit/23af564f190d828eba253e33ca9038dc88e35cb9

commit 23af564f190d828eba253e33ca9038dc88e35cb9
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Sun Oct 29 11:18:21 2017 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Mon Oct 30 16:22:52 2017 +0200

    Test for broken migration as merged
    
    20171026114327_add_deleted_on_to_container_quota_and_items
    used add_timestamps on existing container_quota_items table.
    For poeple with any records in container_quota_items, it failed with:
    
      PG::NotNullViolation: ERROR:  column "created_at" contains null values
      : ALTER TABLE "container_quota_items" ADD "created_at" timestamp NOT NULL
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560
    fbf0077

 ..._add_deleted_on_to_container_quota_and_items.rb |   3 +
 ...deleted_on_to_container_quota_and_items_spec.rb | 101 +++++++++++++++++++++
 2 files changed, 104 insertions(+)
 create mode 100644 spec/migrations/20171026114327_add_deleted_on_to_container_quota_and_items_spec.rb

Comment 10 CFME Bot 2017-10-30 15:18:45 UTC
New commit detected on ManageIQ/manageiq-schema/master:
https://github.com/ManageIQ/manageiq-schema/commit/52c644e3a46623de593c100f5ea057829fa5438c

commit 52c644e3a46623de593c100f5ea057829fa5438c
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Sun Oct 29 15:57:17 2017 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Mon Oct 30 16:22:52 2017 +0200

    Retroactive fix to broken 20171026114327_add_deleted_on_to_container_quota_and_items
    
    Backfill container_quota_items timestamps to satisfy NOT NULL.
    created_at copied from parent container_quota.created_on.
    updated_at set to migration moment.
    
    This is also good because future reporting will rely on
    created_at..deleted_on ranges to be known.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560
    fbf0077

 ..._add_deleted_on_to_container_quota_and_items.rb | 23 ++++++++++++++++++++--
 ...deleted_on_to_container_quota_and_items_spec.rb |  2 +-
 2 files changed, 22 insertions(+), 3 deletions(-)

Comment 20 Beni Paskin-Cherniavsky 2017-12-19 09:56:39 UTC
Just discovered there are also ClusterResourceQuota, which serve to define quotas shared by multiple projects:
https://docs.openshift.com/container-platform/3.5/admin_guide/multiproject_quota.html

We don't collect those at all.

Loic, do we need them for this customer?
We can't really do them in 5.9 because they'll need schema additions.
Do we need them at all?  I suppose for the product, if regular quotas make sense, these also make sense.

Food for thought: how would multi-project quotas fit into reporting, say Chargeback by Project???

Comment 21 Beni Paskin-Cherniavsky 2017-12-19 10:01:52 UTC
(implementation note for future: there are 2 entities to collect:
https://docs.openshift.com/container-platform/3.7/rest_api/apis-quota.openshift.io/v1.ClusterResourceQuota.html
https://docs.openshift.com/container-platform/3.7/rest_api/apis-quota.openshift.io/v1.AppliedClusterResourceQuota.html
the latter help us figure out which quotas actually affect a given project,
so we don't need to do the selector/labels matching ourself)

Comment 22 Loic Avenel 2017-12-19 12:59:45 UTC
(In reply to Beni Paskin-Cherniavsky from comment #20)
> Just discovered there are also ClusterResourceQuota, which serve to define
> quotas shared by multiple projects:
> https://docs.openshift.com/container-platform/3.5/admin_guide/
> multiproject_quota.html
> 
> We don't collect those at all.
> 
> Loic, do we need them for this customer?
> We can't really do them in 5.9 because they'll need schema additions.
> Do we need them at all?  I suppose for the product, if regular quotas make
> sense, these also make sense.
> 
> Food for thought: how would multi-project quotas fit into reporting, say
> Chargeback by Project???

No we don't need it for now.

Comment 23 CFME Bot 2017-12-20 13:07:08 UTC
New commit detected on ManageIQ/manageiq/master:
https://github.com/ManageIQ/manageiq/commit/6b525f4319ad965800e8c67ae8b2a61f5be1431f

commit 6b525f4319ad965800e8c67ae8b2a61f5be1431f
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Tue Dec 12 19:50:24 2017 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Wed Dec 13 13:45:48 2017 +0200

    Add ContainerQuotaScope model
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 app/models/container_quota.rb                      | 1 +
 app/models/container_quota_scope.rb                | 3 +++
 app/models/manageiq/providers/container_manager.rb | 1 +
 3 files changed, 5 insertions(+)
 create mode 100644 app/models/container_quota_scope.rb

Comment 24 CFME Bot 2017-12-20 13:07:13 UTC
New commit detected on ManageIQ/manageiq/master:
https://github.com/ManageIQ/manageiq/commit/c48424726ed51f7f0c4e0ae5dc742309c9f62c9d

commit c48424726ed51f7f0c4e0ae5dc742309c9f62c9d
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Tue Dec 12 19:51:24 2017 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Wed Dec 13 14:10:19 2017 +0200

    save_inventory: save container quota scopes
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 app/models/ems_refresh/save_inventory_container.rb | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comment 25 CFME Bot 2017-12-20 17:01:23 UTC
New commit detected on ManageIQ/manageiq-providers-kubernetes/master:
https://github.com/ManageIQ/manageiq-providers-kubernetes/commit/29ef338ed5e6bf55ddec60c5a84d3ec56c15d7a4

commit 29ef338ed5e6bf55ddec60c5a84d3ec56c15d7a4
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Wed Dec 13 13:45:27 2017 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Tue Dec 19 15:47:26 2017 +0200

    Parse and save quota scopes
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 .../container_manager/inventory_collections.rb     |  8 ++++
 .../kubernetes/container_manager/refresh_parser.rb | 19 ++++++--
 .../container_manager/refresh_parser_spec.rb       | 54 ++++++++++++++--------
 .../kubernetes/container_manager/refresher_spec.rb |  3 +-
 4 files changed, 58 insertions(+), 26 deletions(-)

Comment 26 Beni Paskin-Cherniavsky 2017-12-27 17:53:45 UTC
A simplification: scopes field is immutable once quota has been created.

* It does allow edits that change order of scopes! Doesn't matter semantically, but will test refresh with this.

Kubernetes unfortunately doesn't document which fields are immutable.

The code prevents changing the set of scopes:
https://github.com/kubernetes/kubernetes/blob/v1.10.0-alpha.0/pkg/apis/core/validation/validation.go#L4170-L4172
ever since scopes were added in kubernetes 1.2.0:
https://github.com/kubernetes/kubernetes/pull/20446/commits/df064bd53d258380711d8365dac4d1985e313e14#diff-f19a7777a36882029db668a427f950feR2397

Comment 30 Beni Paskin-Cherniavsky 2018-01-08 12:15:00 UTC
Encountered several problems with floating point causing unnecessary archiving:

1. There was a silly float/int mismatch, solved.
2. Parsing `700m` strings into Ruby floats introduces errors.  Solvable.
3. Saving a Ruby float into Postgres float column and reading it back sometimes returns slightly different value.

For next version will change schema to Decimal column.
We're still evaluating several ideas for 5.9...
Full details on https://github.com/ManageIQ/manageiq-providers-kubernetes/issues/208.


Also noticed that k8s has equivalent ways to spell request quotas, `cpu` vs `requests.cpu`, `memory` vs `request.memory`, and what's worse can set both to different values at the same time!
Ignoring for now, but will be a problem in reporting, unless we figure out right way to normalize it in refresh.
https://github.com/ManageIQ/manageiq-providers-kubernetes/issues/207

Comment 31 CFME Bot 2018-01-09 09:37:44 UTC
New commit detected on ManageIQ/manageiq-providers-kubernetes/master:
https://github.com/ManageIQ/manageiq-providers-kubernetes/commit/208f3726c58467abb1ee6000343e7b8f6d0514ab

commit 208f3726c58467abb1ee6000343e7b8f6d0514ab
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Sun Jan 7 13:07:45 2018 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Sun Jan 7 13:14:40 2018 +0200

    Cast quota values to float in parser, to match float columns
    
    This will allow container quota archiving to not hit a bug in classic
    refresh where find_key values weren't cast to column types,
    so we can proceed (and backport) independently of
    https://github.com/ManageIQ/manageiq/pull/16723
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 .../providers/kubernetes/container_manager/refresh_parser.rb        | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comment 32 Beni Paskin-Cherniavsky 2018-01-09 14:41:31 UTC
TODO: implement purging on the container quota tables
(otherwise archiving will grow them without bound).
Will do in separate PR.

Comment 34 CFME Bot 2018-01-18 20:35:31 UTC
New commit detected on ManageIQ/manageiq-schema/master:
https://github.com/ManageIQ/manageiq-schema/commit/06227a5d29ca574fddbc6faede1b5ae0782d5ad1

commit 06227a5d29ca574fddbc6faede1b5ae0782d5ad1
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Wed Jan 10 14:27:34 2018 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Thu Jan 18 15:20:43 2018 +0200

    Change container_quota_items float columns to decimals
    
    Fixes https://github.com/ManageIQ/manageiq-providers-kubernetes/issues/208
    Kubernetes quotas are mostly integers but cpu-related quotas are
    in "millicores", multiples of 0.001.
    With decimal columns, we'll be able to round-trip quotas exactly to DB
    and back to Ruby, so refresh can determine whether they changed or not.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 ...180118131417_change_container_quota_items_to_decimals.rb | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 db/migrate/20180118131417_change_container_quota_items_to_decimals.rb

Comment 38 CFME Bot 2018-01-29 05:49:05 UTC
New commit detected on ManageIQ/manageiq-providers-openshift/master:
https://github.com/ManageIQ/manageiq-providers-openshift/commit/28c274f405a094e3c8a1dad71df0abb4ccb7ee95

commit 28c274f405a094e3c8a1dad71df0abb4ccb7ee95
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Wed Jan 17 11:26:37 2018 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Sun Jan 28 22:42:46 2018 +0200

    Quota changes for 1st and 2nd recording
    
    - Add a memory quota to test binary suffixes (Mi, Gi).
    - Delete all quotas from  my-project-1 before 2nd recording, as test expects.
    - Modify quota items for 2nd recording to cover delete, update, no-change.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 .../openshift/container_manager/test_objects_record.sh         | 10 ++++++++++
 .../openshift/container_manager/test_objects_template.yml      |  5 +++--
 2 files changed, 13 insertions(+), 2 deletions(-)

Comment 40 CFME Bot 2018-02-26 14:46:46 UTC
New commit detected on ManageIQ/manageiq/master:
https://github.com/ManageIQ/manageiq/commit/500fb84d69877449954905fa3be0f528e8aed686

commit 500fb84d69877449954905fa3be0f528e8aed686
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Tue Jan 16 15:17:09 2018 +0200
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Mon Feb 19 18:14:28 2018 +0200

    Compare decimal columns correctly in batch saver
    
    Will allow container_quota_items.quota_* columns to be used in manager_ref.
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 .../manager_refresh/save_collection/saver/base.rb  | 17 ++++++---
 .../save_collection/saver/concurrent_safe_batch.rb | 15 ++++++--
 .../manager_refresh/helpers/spec_parsed_data.rb    | 15 ++++++++
 .../save_inventory/init_data_helper.rb             | 19 ++++++++++
 .../save_inventory/saver_strategies_spec.rb        | 40 ++++++++++++++++++++++
 5 files changed, 99 insertions(+), 7 deletions(-)

Comment 41 CFME Bot 2018-03-13 19:47:31 UTC
New commit detected on ManageIQ/manageiq/master:

https://github.com/ManageIQ/manageiq/commit/4c9aebbb98368148ba30786ceda3b5e0e9b92445
commit 4c9aebbb98368148ba30786ceda3b5e0e9b92445
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Sun Dec 31 07:53:03 2017 -0500
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Sun Dec 31 07:53:03 2017 -0500

    Keep quota history by archiving

    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 app/models/container_project.rb | 3 +-
 app/models/container_quota.rb | 25 +-
 app/models/container_quota_item.rb | 14 +
 app/models/ems_refresh/save_inventory_container.rb | 8 +-
 app/models/manageiq/providers/container_manager.rb | 3 +-
 5 files changed, 48 insertions(+), 5 deletions(-)

Comment 42 CFME Bot 2018-03-13 19:53:53 UTC
New commit detected on ManageIQ/manageiq-providers-kubernetes/master:

https://github.com/ManageIQ/manageiq-providers-kubernetes/commit/3bf315f1e91d6e4c8256d288ba47c798da353de9
commit 3bf315f1e91d6e4c8256d288ba47c798da353de9
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Sun Dec 31 08:53:41 2017 -0500
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Sun Dec 31 08:53:41 2017 -0500

    Keep quota history by archiving

    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb | 11 +-
 1 file changed, 7 insertions(+), 4 deletions(-)

Comment 44 Beni Paskin-Cherniavsky 2018-03-21 09:33:24 UTC
Update: Last PR https://github.com/ManageIQ/manageiq/pull/17167 implementing purging is still in review but otherwise quota history already works!

Comment 45 CFME Bot 2018-03-22 17:11:04 UTC
New commit detected on ManageIQ/manageiq/master:

https://github.com/ManageIQ/manageiq/commit/022e15256fd07fa7bf5b3ade7ce16b13daa87b84
commit 022e15256fd07fa7bf5b3ade7ce16b13daa87b84
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Sun Mar 18 06:25:43 2018 -0400
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Sun Mar 18 06:25:43 2018 -0400

    Purging of ContainerQuota & ContainerQuotaItem

    Both are necessary because ContainerQuotaItem may be archived due to edits
    to parent ContainerQuota that is still alive.

    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

 app/models/container_quota.rb | 1 +
 app/models/container_quota/purging.rb | 25 +
 app/models/container_quota_item.rb | 1 +
 app/models/container_quota_item/purging.rb | 20 +
 app/models/miq_schedule_worker/jobs.rb | 2 +
 config/settings.yml | 1 +
 spec/models/container_quota/purging_spec.rb | 63 +
 spec/models/container_quota_item/purging_spec.rb | 70 +
 8 files changed, 183 insertions(+)

Comment 48 juwatts 2018-10-31 19:01:52 UTC
Verified in 5.10.0.21.20181023151612_4c11b0d. 

Test steps:

Adding resource quota test:

On the Openshift provider, create different namespaces and assign them Resource Quotas. Assign different fractional values to the CPU quotas like  `1.152` or `0.087` or `87m`.	
Resource Quota assigned successfully. 
Refresh the CFME appliance	
Appliance refreshed successfully.
Check the containers_quota_items table in the database	Database in `container_quota_items` table should contain both old and current values. 
Newly added quotas should have `created_at` field set.
Navigate to the namespace in the CFME GUI.	 
Verify the GUI displays the correct data and the fractional values are formatted properly. 
Refresh the CFME appliance	
Appliance refreshed successfully.
Check the containers_quota_items table in the database	
Verify duplicate records were not added to the table

Modifying Resource quota test:

Modify the existing Resource Quota
Verify the update was successful
Refresh the CFME appliance	
Appliance refreshed successfully.
Check the containers_quota_items table in the database	
Verify the modified values (eg. increased cpu) should appear as multiple rows — old value with `deleted_on` field set, and new value with approximately same `created_at`.
Navigate to the namespace in the CFME GUI.
Verify the GUI displays the correct data and the fractional values are formatted properly. 
Modify the existing Resource Quota again	
Verify the update was successful
Refresh the CFME appliance	
Appliance refreshed successfully.
Check the containers_quota_items table in the database	
Verify multiple values with both (created_at, deleted_on) set and only last one with deleted_on=null.
Navigate to the namespace in the CFME GUI.	
Verify the GUI displays the correct data and the fractional values are formatted properly. 

Deleting Resource quota test:
Delete the existing Resource Quota 
Verify the Resource Quota was deleted successfully
Refresh the CFME appliance	
Appliance refreshed successfully.
Check the containers_quota_items table in the database	
Verify the deleted Resource Quotas should still exist, with `deleted_on` field set.
Navigate to the namespace in the CFME GUI.	
Verify the GUI displays no data referencing Resource Quota.


Note You need to log in before you can comment on or make changes to this bug.