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 a 'Container Project Discovered' event #16903

Merged
merged 2 commits into from
Feb 6, 2018
Merged

Add a 'Container Project Discovered' event #16903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40396c5
Add a 'Container Project Discovered' event
Jan 29, 2018
d7deca9
Added container project to alerts 'based on'
Feb 4, 2018
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
15 changes: 15 additions & 0 deletions app/models/container_project.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ class ContainerProject < ApplicationRecord
include SupportsFeatureMixin
include CustomAttributeMixin
include ArchivedMixin
include MiqPolicyMixin
include TenantIdentityMixin
include CustomActionsMixin
include_concern 'Purging'
belongs_to :ext_management_system, :foreign_key => "ems_id"
Expand All @@ -20,6 +22,7 @@ class ContainerProject < ApplicationRecord
has_many :container_templates
has_many :archived_container_groups, :foreign_key => "old_container_project_id", :class_name => "ContainerGroup"
has_many :persistent_volume_claims
has_many :miq_alert_statuses, :as => :resource, :dependent => :destroy

# Needed for metrics
has_many :metrics, :as => :resource
Expand All @@ -35,6 +38,8 @@ class ContainerProject < ApplicationRecord
virtual_total :containers_count, :containers
virtual_total :images_count, :container_images

after_create :raise_creation_event
Copy link
Contributor

Choose a reason for hiding this comment

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

In graph refresh batch saving, we do not call these, you can add it in:
https://github.com/Ladas/manageiq-providers-kubernetes/blob/fb3e5d8b1525d54319dcefecd914e1b135f75a1e/app/models/manageiq/providers/kubernetes/container_manager/refresher_mixin.rb#L86

so in similar way as for container_images, we can access persistor.container_projects.created_records.

Copy link
Author

Choose a reason for hiding this comment

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

👍 ManageIQ/manageiq-providers-kubernetes#226


include EventMixin
include Metric::CiMixin

Expand Down Expand Up @@ -67,4 +72,14 @@ def disconnect_inv
self.deleted_on = Time.now.utc
save
end

def self.raise_creation_events(container_project_ids)
where(:id => container_project_ids).find_each do |record|
MiqEvent.raise_evm_event(record, 'containerproject_created', {})
end
end

def raise_creation_event
MiqEvent.raise_evm_event(self, 'containerproject_created', {})
end
end
1 change: 1 addition & 0 deletions app/models/miq_alert.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class MiqAlert < ApplicationRecord
ExtManagementSystem
MiqServer
ContainerNode
ContainerProject
Copy link

Choose a reason for hiding this comment

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

You should add a miq_alert_statuses relation to container_project.rb

has_many :miq_alert_statuses, :as => :resource, :dependent => :destroy

Generally looks good, It would be helpful if you could separate the pieces related to the new event from the policy/alert bits.

Copy link
Author

Choose a reason for hiding this comment

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

Added the relation but I didnt split the PR since this is a small feature that already has 3 PRs associated with it.
I think its a little too much noise.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change caused ui-classic travis failures, fixed in ManageIQ/manageiq-ui-classic#3382

)

def self.base_tables
Expand Down
2 changes: 1 addition & 1 deletion app/models/miq_event.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class MiqEvent < EventStream
SUPPORTED_POLICY_AND_ALERT_CLASSES = [Host, VmOrTemplate, Storage,
EmsCluster, ResourcePool, MiqServer,
ExtManagementSystem,
ContainerReplicator, ContainerGroup,
ContainerReplicator, ContainerGroup, ContainerProject,
ContainerNode, ContainerImage, PhysicalServer].freeze

def self.raise_evm_event(target, raw_event, inputs = {}, options = {})
Expand Down
2 changes: 2 additions & 0 deletions db/fixtures/miq_event_definitions.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ containerreplicator_compliance_check,Replicator Compliance Check,Default,complia
containerreplicator_compliance_passed,Replicator Compliance Passed,Default,compliance
containerreplicator_compliance_failed,Replicator Compliance Failed,Default,compliance

containerproject_created,Container Project Discovered,Default,container_operations

#
# Physical Server Operations
#
Expand Down