Skip to content

Commit

Permalink
Merge pull request #13686 from zeari/cpature_dead_containers
Browse files Browse the repository at this point in the history
Collect metrics for archived containers
  • Loading branch information
Fryguy authored Feb 24, 2017
2 parents 8fbadb0 + 4ad0054 commit 5c98cf7
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 13 deletions.
4 changes: 1 addition & 3 deletions app/models/container.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class Container < ApplicationRecord
include SupportsFeatureMixin
include NewWithTypeStiMixin
include ArchivedMixin

has_one :container_group, :through => :container_definition
belongs_to :ext_management_system, :foreign_key => :ems_id
Expand All @@ -18,9 +19,6 @@ class Container < ApplicationRecord
has_many :metric_rollups, :as => :resource
has_many :vim_performance_states, :as => :resource

# Needed for metrics
delegate :my_zone, :to => :ext_management_system

include EventMixin
include Metric::CiMixin

Expand Down
1 change: 1 addition & 0 deletions app/models/container_definition.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class ContainerDefinition < ApplicationRecord
include ArchivedMixin
# :name, :image, :image_pull_policy, :memory, :cpu
belongs_to :container_group
belongs_to :ext_management_system, :foreign_key => :ems_id
Expand Down
4 changes: 1 addition & 3 deletions app/models/container_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class ContainerGroup < ApplicationRecord
include MiqPolicyMixin
include NewWithTypeStiMixin
include TenantIdentityMixin
include ArchivedMixin

# :name, :uid, :creation_timestamp, :resource_version, :namespace
# :labels, :restart_policy, :dns_policy
Expand Down Expand Up @@ -33,9 +34,6 @@ class ContainerGroup < ApplicationRecord
virtual_column :ready_condition_status, :type => :string, :uses => :container_conditions
virtual_column :running_containers_summary, :type => :string

# Needed for metrics
delegate :my_zone, :to => :ext_management_system

def ready_condition
container_conditions.find_by(:name => "Ready")
end
Expand Down
6 changes: 1 addition & 5 deletions app/models/container_project.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class ContainerProject < ApplicationRecord
include SupportsFeatureMixin
include CustomAttributeMixin
include ArchivedMixin
belongs_to :ext_management_system, :foreign_key => "ems_id"
has_many :container_groups
has_many :container_routes
Expand All @@ -21,7 +22,6 @@ class ContainerProject < ApplicationRecord
has_many :metrics, :as => :resource
has_many :metric_rollups, :as => :resource
has_many :vim_performance_states, :as => :resource
delegate :my_zone, :to => :ext_management_system

has_many :labels, -> { where(:section => "labels") }, :class_name => "CustomAttribute", :as => :resource, :dependent => :destroy

Expand Down Expand Up @@ -66,8 +66,4 @@ def disconnect_inv
self.deleted_on = Time.now.utc
save
end

def archived?
ems_id.nil?
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def initialize(target, start_time, end_time, interval)
@end_time = end_time
@interval = interval
@tenant = target.try(:container_project).try(:name) || '_system'
@ext_management_system = @target.ext_management_system
@ext_management_system = @target.ext_management_system || @target.try(:old_ext_management_system)
@ts_values = Hash.new { |h, k| h[k] = {} }
@metrics = []

Expand Down
4 changes: 3 additions & 1 deletion app/models/metric/ci_mixin/capture.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ def perf_capture_object
def queue_name_for_metrics_collection
ems = if self.kind_of?(ExtManagementSystem)
self
elsif self.respond_to?(:ext_management_system)
elsif respond_to?(:ext_management_system) && ext_management_system.present?
ext_management_system
elsif respond_to?(:old_ext_management_system) && old_ext_management_system.present?
old_ext_management_system
end
raise _("Unsupported type %{name} (id: %{number})") % {:name => self.class.name, :number => id} if ems.nil?
ems.metrics_collector_queue_name
Expand Down
22 changes: 22 additions & 0 deletions app/models/mixins/archived_mixin.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module ArchivedMixin
extend ActiveSupport::Concern

included do
belongs_to :old_ext_management_system, :foreign_key => :old_ems_id, :class_name => 'ExtManagementSystem'
end

def archived?
ems_id.nil?
end

# Needed for metrics
def my_zone
if ext_management_system.present?
ext_management_system.my_zone
elsif old_ext_management_system.present?
# Archived container entities need to retain their zone for metric collection
# This makes the association more complex and might need a performance fix
old_ext_management_system.my_zone
end
end
end
25 changes: 25 additions & 0 deletions spec/models/metric/ci_mixin/capture_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,29 @@ def parse_datetime(datetime)
datetime << "Z" if datetime.size == 19
Time.parse(datetime).utc
end

context "handles archived container entities" do
it "get the correct queue name and zone from archived container entities" do
ems = FactoryGirl.create(:ems_openshift, :name => 'OpenShiftProvider')
group = FactoryGirl.create(:container_group, :name => "group", :ext_management_system => ems)
container = FactoryGirl.create(:container,
:name => "container",
:container_group => group,
:ext_management_system => ems)
project = FactoryGirl.create(:container_project,
:name => "project",
:ext_management_system => ems)
container.disconnect_inv
group.disconnect_inv
project.disconnect_inv

expect(container.queue_name_for_metrics_collection).to eq ems.metrics_collector_queue_name
expect(group.queue_name_for_metrics_collection).to eq ems.metrics_collector_queue_name
expect(project.queue_name_for_metrics_collection).to eq ems.metrics_collector_queue_name

expect(container.my_zone).to eq ems.my_zone
expect(group.my_zone).to eq ems.my_zone
expect(project.my_zone).to eq ems.my_zone
end
end
end

0 comments on commit 5c98cf7

Please sign in to comment.