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.

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 SchedulerAgent, which periodically runs other agents. #459

Merged
merged 34 commits into from
Sep 8, 2014
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
c7e67a3
Add "chains" to represent runner-target relationships between Agents.
knu Aug 21, 2014
b4636f8
Provide a target agent select box for an agent that can run other age…
knu Aug 21, 2014
0ae0f1a
Show target-runner relationships in a diagram.
knu Aug 21, 2014
e9fa1f2
Add SchedulerAgent, which periodically runs other agents.
knu Aug 21, 2014
26564c5
Add a spec for how the before_save hook should work.
knu Aug 22, 2014
f6d1966
Store an agent ID in the local storage rather than stashing it in a tag.
knu Aug 22, 2014
56fa4cb
Fix the schema version.
knu Aug 22, 2014
de5619f
Rename "runner" to "controller".
knu Aug 22, 2014
a069fa6
Improve the description.
knu Aug 22, 2014
0f66669
Disable second precision schedule by default to prevent service abuse.
knu Aug 22, 2014
a479573
Add control action types "enable" and "disable".
knu Aug 22, 2014
a2e1348
Use error for error, not log.
knu Aug 23, 2014
806e41d
Merge branch 'master' into scheduler_agent
knu Aug 27, 2014
d0d9c8e
Fix specs for the new stuff in DOT output.
knu Aug 29, 2014
3004170
Rename "target" to "control target".
knu Aug 29, 2014
1cea2ba
Merge remote-tracking branch 'origin/master' into scheduler_agent
knu Aug 29, 2014
05dd52d
Fix a bug with edge line styling.
knu Aug 29, 2014
8065721
Rename "chain" to "control link" for clarity.
knu Sep 1, 2014
5306726
Allow multiples of fifteen as seconds.
knu Sep 2, 2014
95a067a
Compare user_id's rather than user objects.
knu Sep 3, 2014
06ae684
Use length rather than count when the relation should already be mate…
knu Sep 3, 2014
46497f0
Fix control-link-region display toggling.
knu Sep 3, 2014
69d2273
Add can_control_other_agents to the response hash of type_details.
knu Sep 3, 2014
706ce8d
Clone controller_target_ids too.
knu Sep 3, 2014
9a7f7fe
Add NOT NULL contraints.
knu Sep 3, 2014
51f8c8b
Add some specs for our Rufus::Scheduler extension.
knu Sep 3, 2014
53eb531
Set a created job's scheduler_agent_id before it is first triggered.
knu Sep 3, 2014
77f03da
Parenthesize the argument.
knu Sep 3, 2014
8dc5244
Merge remote-tracking branch 'origin/master' into scheduler_agent
knu Sep 3, 2014
99c3fc0
Update db/schema.rb.
knu Sep 3, 2014
12f1839
Squash migrations during the development of the scheduler_agent branch.
knu Sep 3, 2014
38de860
Make the "Controllers" field read-only.
knu Sep 4, 2014
786b0f3
Oops, 30 is a multiple of fifteen.
knu Sep 4, 2014
23b0741
When the control action is `run`, control targets must be schedulable.
knu Sep 4, 2014
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
6 changes: 6 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ ALLOW_JSONPATH_EVAL=false
# when you trust everyone using your Huginn installation.
ENABLE_INSECURE_AGENTS=false

# Enable this setting to allow second precision schedule in
# SchedulerAgent. By default, the use of the "second" field is
# restricted so that any value other than a single zero (which means
# "on the minute") is disallowed.
ENABLE_SECOND_PRECISION_SCHEDULE=false
Copy link
Member

Choose a reason for hiding this comment

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

Is this to prevent DOS?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you can still create as many agents as you want, though, I suppose only allowing an agent to run on the minute would demotivate potential abusers. Should I note that here?

Copy link
Member

Choose a reason for hiding this comment

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

I guess? I suppose we could allow every 15 or 30 seconds. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is fine as long as you can document that nicely in the description. I cheated by omitting the seconds part completely. 😁

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, multiples of fifteen are now allowed even when the option is disabled (default).


# Use Graphviz for generating diagrams instead of using Google Chart
# Tools. Specify a dot(1) command path built with SVG support
# enabled.
Expand Down
26 changes: 23 additions & 3 deletions app/assets/javascripts/application.js.coffee.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,13 @@ window.setupJsonEditor = ($editors = $(".live-json-editor")) ->
return editors

hideSchedule = ->
$(".schedule-region select").hide()
$(".schedule-region .can-be-scheduled").hide()
$(".schedule-region .cannot-be-scheduled").show()

showSchedule = (defaultSchedule = null) ->
$(".schedule-region select").show()
if defaultSchedule?
$(".schedule-region select").val(defaultSchedule).change()
$(".schedule-region select").show()
$(".schedule-region .can-be-scheduled").show()
$(".schedule-region .cannot-be-scheduled").hide()

hideLinks = ->
Expand All @@ -43,6 +42,16 @@ showLinks = ->
$(".link-region .cannot-receive-events").hide()
showEventDescriptions()

hideChains = ->
$(".chain-region .select2-container").hide()
$(".chain-region .propagate-immediately").hide()
$(".chain-region .cannot-receive-events").show()

showChains = ->
$(".chain-region .select2-container").show()
$(".chain-region .propagate-immediately").show()
$(".chain-region .cannot-receive-events").hide()

hideEventCreation = ->
$(".event-related-region").hide()

Expand Down Expand Up @@ -161,6 +170,11 @@ $(document).ready ->
else
hideLinks()

if json.can_control_other_agents
showChains()
else
hideChains()

if json.can_create_events
showEventCreation()
else
Expand Down Expand Up @@ -191,6 +205,12 @@ $(document).ready ->
else
hideLinks()

if $(".chain-region")
if $(".chain-region").data("can-control-other-agents") == true
showChains()
else
hideChains()

if $(".event-related-region")
if $(".event-related-region").data("can-create-events") == true
showEventCreation()
Expand Down
45 changes: 45 additions & 0 deletions app/concerns/agent_controller_concern.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
module AgentControllerConcern
extend ActiveSupport::Concern

included do
validate :validate_control_action
end

def default_options
{
'action' => 'run',
}
end

def control_action
options['action'].presence || 'run'
end

def validate_control_action
case control_action
when 'run', 'enable', 'disable'
else
errors.add(:base, 'invalid action')
end
end

def control_targets!
targets.active.each { |target|
begin
case control_action
when 'run'
log "Agent run queued for '#{target.name}'"
Agent.async_check(target.id)
when 'enable'
log "Enabling the Agent '#{target.name}'"
target.update!(disable: false) if target.disabled?
when 'disable'
log "Disabling the Agent '#{target.name}'"
target.update!(disable: true) unless target.disabled?
Copy link
Member

Choose a reason for hiding this comment

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

Other ideas:

  • grep target's AgentLogs
  • clone target (seems like a bad idea 😉 agent event flow meta programming!)

Copy link
Member Author

Choose a reason for hiding this comment

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

Each of them sounds interesting and useful.

  • AgentDiagnosticAgent would watch AgentLogs and the working? status to notify user of potential troubles and recovery
  • AgentCreatorAgent would create a new agent on an event using a template which will feed events to targets.

Lots of ideas, like it's unlimited. 😆

end
rescue => e
log "Failed to #{control_action} '#{target.name}': #{e.message}"
Copy link
Member

Choose a reason for hiding this comment

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

use error, not log

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, of course!

end
}
end
end
12 changes: 11 additions & 1 deletion app/helpers/agent_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,14 @@ def scenario_links(agent)
def agent_show_class(agent)
agent.short_type.underscore.dasherize
end
end

def agent_schedule(agent, delimiter = ', ')
return 'n/a' unless agent.can_be_scheduled?

controllers = agent.controllers
[
*(CGI.escape_html(agent.schedule.humanize.titleize) unless agent.schedule == 'never' && agent.controllers.count > 0),
Copy link
Member

Choose a reason for hiding this comment

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

Very minor: I'd do agent.controllers.length here, since we're materializing them anyway in the map, so no need to do an extra SQL query via count.

Copy link
Member Author

Choose a reason for hiding this comment

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

All right.

*controllers.map { |agent| link_to(agent.name, agent_path(agent)) },
].join(delimiter).html_safe
end
end
13 changes: 11 additions & 2 deletions app/helpers/dot_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ def agent_node(agent)
def agent_edge(agent, receiver)
edge(agent_id[agent],
agent_id[receiver],
style: ('dashed' unless receiver.propagate_immediately),
style: ('dashed' unless agent.can_control_other_agents? || !receiver.propagate_immediately?),
label: (" #{agent.control_action}s " if agent.can_control_other_agents?),
Copy link
Member

Choose a reason for hiding this comment

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

Is the "s" to pluralize it? If so, maybe agent.control_action.pluralize?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an "s" of the third person singular present form.
I thought pluralize would work in practice, but I just didn't feel right. Shall I change it to pluralize + a comment?

Copy link
Member

Choose a reason for hiding this comment

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

I don't feel strongly.

arrowhead: ('empty' if agent.can_control_other_agents?),
color: (@disabled if agent.disabled? || receiver.disabled?))
end

Expand All @@ -151,10 +153,17 @@ def agent_edge(agent, receiver)
fontsize: 10,
fontname: ('Helvetica' if rich)

statement 'edge',
fontsize: 10,
fontname: ('Helvetica' if rich)

agents.each.with_index { |agent, index|
agent_node(agent)

agent.receivers.each { |receiver|
[
*agent.receivers,
*(agent.targets if agent.can_control_other_agents?)
].each { |receiver|
agent_edge(agent, receiver) if agents.include?(receiver)
}
}
Expand Down
28 changes: 26 additions & 2 deletions app/models/agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ class Agent < ActiveRecord::Base

EVENT_RETENTION_SCHEDULES = [["Forever", 0], ["1 day", 1], *([2, 3, 4, 5, 7, 14, 21, 30, 45, 90, 180, 365].map {|n| ["#{n} days", n] })]

attr_accessible :options, :memory, :name, :type, :schedule, :disabled, :source_ids, :scenario_ids, :keep_events_for, :propagate_immediately
attr_accessible :options, :memory, :name, :type, :schedule, :controller_ids, :target_ids, :disabled, :source_ids, :scenario_ids, :keep_events_for, :propagate_immediately

json_serialize :options, :memory

validates_presence_of :name, :user
validates_inclusion_of :keep_events_for, :in => EVENT_RETENTION_SCHEDULES.map(&:last)
validate :sources_are_owned
validate :controllers_are_owned
validate :targets_are_owned
Copy link
Member

Choose a reason for hiding this comment

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

The word target is pretty generic, and seems like it could get confused with receivers. Not sure what's better, though. Maybe control_target?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or maybe... is "controllee" a word?

Copy link
Member Author

Choose a reason for hiding this comment

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

@cantino If not, what about "members"? Should neither work, I'll use control_targets in code and "Control Targets" in views.

Copy link
Member

Choose a reason for hiding this comment

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

controllee is... sort of a word :) I'd probably do control_target, but I don't feel strongly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it. Now, how about the table name? I named it chains without much consideration, but there should be a better name. paths, controller_targets, control_links, etc.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, how about control_links?

Copy link
Member Author

Choose a reason for hiding this comment

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

The table has been renamed.

validate :scenarios_are_owned
validate :validate_schedule
validate :validate_options
Expand All @@ -52,6 +54,10 @@ class Agent < ActiveRecord::Base
has_many :links_as_receiver, :dependent => :delete_all, :foreign_key => "receiver_id", :class_name => "Link", :inverse_of => :receiver
has_many :sources, :through => :links_as_receiver, :class_name => "Agent", :inverse_of => :receivers
has_many :receivers, :through => :links_as_source, :class_name => "Agent", :inverse_of => :sources
has_many :chains_as_controller, dependent: :delete_all, foreign_key: 'controller_id', class_name: 'Chain', inverse_of: :controller
has_many :chains_as_target, dependent: :delete_all, foreign_key: 'target_id', class_name: 'Chain', inverse_of: :target
has_many :controllers, through: :chains_as_target, class_name: "Agent", inverse_of: :targets
has_many :targets, through: :chains_as_controller, class_name: "Agent", inverse_of: :controllers
has_many :scenario_memberships, :dependent => :destroy, :inverse_of => :agent
has_many :scenarios, :through => :scenario_memberships, :inverse_of => :agents

Expand Down Expand Up @@ -176,6 +182,10 @@ def can_create_events?
!cannot_create_events?
end

def can_control_other_agents?
self.class.can_control_other_agents?
end

def log(message, options = {})
puts "Agent##{id}: #{message}" unless Rails.env.test?
AgentLog.log_for_agent(self, message, options)
Expand Down Expand Up @@ -218,6 +228,14 @@ def sources_are_owned
errors.add(:sources, "must be owned by you") unless sources.all? {|s| s.user == user }
end

def controllers_are_owned
errors.add(:controllers, "must be owned by you") unless controllers.all? {|s| s.user == user }
Copy link
Member

Choose a reason for hiding this comment

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

Slightly faster to do user_id == user.id

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'll change it along with the existing code above.

end

def targets_are_owned
errors.add(:targets, "must be owned by you") unless targets.all? {|s| s.user == user }
end

def scenarios_are_owned
errors.add(:scenarios, "must be owned by you") unless scenarios.all? {|s| s.user == user }
Copy link
Member

Choose a reason for hiding this comment

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

We could do that here too.

end
Expand Down Expand Up @@ -249,7 +267,7 @@ def boolify(option_value)

class << self
def build_clone(original)
new(original.slice(:type, :options, :schedule, :source_ids, :keep_events_for, :propagate_immediately)) { |clone|
new(original.slice(:type, :options, :schedule, :controller_ids, :source_ids, :keep_events_for, :propagate_immediately)) { |clone|
Copy link
Member

Choose a reason for hiding this comment

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

Do we need control_target_ids here too? (I'm not sure.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added!

# Give it a unique name
2.upto(count) do |i|
name = '%s (%d)' % [original.name, i]
Expand Down Expand Up @@ -290,6 +308,10 @@ def cannot_receive_events?
!!@cannot_receive_events
end

def can_control_other_agents?
include? AgentControllerConcern
end

# Find all Agents that have received Events since the last execution of this method. Update those Agents with
# their new `last_checked_event_id` and queue each of the Agents to be called with #receive using `async_receive`.
# This is called by bin/schedule.rb periodically.
Expand Down Expand Up @@ -399,6 +421,8 @@ def type
:sources,
:receivers,
:schedule,
:controllers,
:targets,
:disabled,
:keep_events_for,
:propagate_immediately,
Expand Down
115 changes: 115 additions & 0 deletions app/models/agents/scheduler_agent.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
require 'rufus-scheduler'

module Agents
class SchedulerAgent < Agent
include AgentControllerConcern

cannot_be_scheduled!
cannot_receive_events!
cannot_create_events!

@@second_precision_enabled = ENV['ENABLE_SECOND_PRECISION_SCHEDULE'] == 'true'

cattr_reader :second_precision_enabled

description <<-MD % { seconds: (<<-MD_SECONDS if second_precision_enabled) }
This agent periodically takes an action on target Agents according to a user-defined schedule.

# Action types

Set `action` to one of the action types below:

* `run`: This is the default. Target Agents are run at intervals.

* `disable`: Target Agents are disabled (if not) at intervals.

* `enable`: Target Agents are enabled (if not) at intervals.

# Targets

Select Agents that you want to run periodically by this SchedulerAgent.

# Schedule

Set `schedule` to a schedule specification in the [cron](http://en.wikipedia.org/wiki/Cron) format.
For example:

* `0 22 * * 1-5`: every day of the week at 22:00 (10pm)

* `*/10 8-11 * * *`: every 10 minutes from 8:00 to and not including 12:00

This variant has several extensions as explained below.

## Timezones

You can optionally specify a timezone (default: `#{Time.zone.name}`) after the day-of-week field.

* `0 22 * * 1-5 Europe/Paris`: every day of the week when it's 22:00 in Paris

* `0 22 * * 1-5 Etc/GMT+2`: every day of the week when it's 22:00 in GMT+2

%{seconds}

## Last day of month

`L` signifies "last day of month" in `day-of-month`.

* `0 22 L * *`: every month on the last day at 22:00

## Weekday names

You can use three letter names instead of numbers in the `weekdays` field.

* `0 22 * * Sat,Sun`: every Saturday and Sunday, at 22:00

## Nth weekday of the month

You can specify "nth weekday of the month" like this.

* `0 22 * * Sun#1,Sun#2`: every first and second Sunday of the month, at 22:00

* `0 22 * * Sun#L1`: every last Sunday of the month, at 22:00
MD

## Seconds

You can optionally specify seconds before the minute field.

* `*/30 * * * * *`: every 30 seconds

MD_SECONDS

def default_options
super.update({
'schedule' => '0 * * * *',
})
end

def working?
true
end

def check!
control_targets!
end

def validate_options
if (spec = options['schedule']).present?
begin
cron = Rufus::Scheduler::CronLine.new(spec)
if !second_precision_enabled && cron.seconds != [0]
errors.add(:base, "second precision schedule is not allowed in this service")
end
rescue ArgumentError
errors.add(:base, "invalid schedule")
end
else
errors.add(:base, "schedule is missing")
end
end

before_save do
self.memory.delete('scheduled_at') if self.options_changed?
end
end
end
7 changes: 7 additions & 0 deletions app/models/chain.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# A Chain connects Agents in a run chain from the `controller` to the `target`.
class Chain < ActiveRecord::Base
attr_accessible :controller_id, :target_id

belongs_to :controller, class_name: 'Agent', inverse_of: :chains_as_controller
belongs_to :target, class_name: 'Agent', inverse_of: :chains_as_target
end
Loading