Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ContainerQuotaScope model, save them in save_inventory #16655

Merged
merged 3 commits into from
Dec 20, 2017
Merged

Add ContainerQuotaScope model, save them in save_inventory #16655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b525f4
Add ContainerQuotaScope model
cben Dec 12, 2017
c484247
save_inventory: save container quota scopes
cben Dec 12, 2017
70d840b
add :container_quota_scope factory
cben Dec 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/models/container_quota.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ class ContainerQuota < ApplicationRecord
belongs_to :ext_management_system, :foreign_key => "ems_id"
belongs_to :container_project

has_many :container_quota_scopes, :dependent => :destroy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cben you mention in the BZ keep history instead of overwriting in refresh. ref is that something that you want to accomplish in this PR? Should this disconnect instead of destroy the quota scopes?

What is a quota scope BTW?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see answer below #16655 (comment)

has_many :container_quota_items, :dependent => :destroy
end
3 changes: 3 additions & 0 deletions app/models/container_quota_scope.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class ContainerQuotaScope < ApplicationRecord
belongs_to :container_quota
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this class do ?
why we need this class ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from discussion in ManageIQ/manageiq-providers-kubernetes#190 is sounds like this class only hold one string field (:scope [string] the name of the scope ) that can currently hold only 4 possible strings ...
Do we need a class to hold a list of pre defined strings, shouldn't we use a text field here ?

Copy link
Contributor Author

@cben cben Dec 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a model class. We need it to access the table, because Rails ;-)
As we're discussing on that PR:

  1. schema might be suboptimal, but gaprindashvili schema is frozen and we need this feature there.
  2. since we're going to retain full quotas history by archiving instead of in-place updates, there is a case where this schema may be more effecient to refresh.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cben does a quata has only 4 possible set of scopes ?
a. terminating and best effort
b. terminating and not best effort
c. not terminating and best effort
d not terminating and not best effort

end
11 changes: 10 additions & 1 deletion app/models/ems_refresh/save_inventory_container.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,19 @@ def save_container_quotas_inventory(ems, hashes)
h[:container_project_id] = h.fetch_path(:project, :id)
end

save_inventory_multi(ems.container_quotas, hashes, :use_association, [:ems_ref], :container_quota_items, :project)
save_inventory_multi(ems.container_quotas, hashes, :use_association,
[:ems_ref], [:container_quota_items, :container_quota_scopes], :project)
store_ids_for_new_records(ems.container_quotas, hashes, :ems_ref)
end

def save_container_quota_scopes_inventory(container_quota, hashes)
return if hashes.nil?
container_quota.container_quota_scopes.reset

save_inventory_multi(container_quota.container_quota_scopes, hashes, :use_association, [:scope])
store_ids_for_new_records(container_quota.container_quota_scopes, hashes, :scope)
end

def save_container_quota_items_inventory(container_quota, hashes)
return if hashes.nil?
container_quota.container_quota_items.reset
Expand Down
1 change: 1 addition & 0 deletions app/models/manageiq/providers/container_manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class ContainerManager < BaseManager
has_many :security_contexts, :through => :containers
has_many :container_service_port_configs, :through => :container_services
has_many :container_routes, :through => :container_services
has_many :container_quota_scopes, :through => :container_quotas
has_many :container_quota_items, :through => :container_quotas
has_many :container_limit_items, :through => :container_limits
has_many :container_template_parameters, :through => :container_templates
Expand Down
5 changes: 5 additions & 0 deletions spec/factories/container_quota_scope.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FactoryGirl.define do
factory :container_quota_scope do
scope "NotTerminating"
end
end