Merge branch 'fix-scheduler_agent'

Akinori MUSHA vor 10 Jahren
Ursprung
Commit
85283deee8

+ 34 - 9
app/concerns/agent_controller_concern.rb

@@ -12,11 +12,11 @@ module AgentControllerConcern
12 12
   end
13 13
 
14 14
   def control_action
15
-    options['action'].presence || 'run'
15
+    interpolated['action']
16 16
   end
17 17
 
18 18
   def validate_control_action
19
-    case control_action
19
+    case options['action']
20 20
     when 'run'
21 21
       control_targets.each { |target|
22 22
         if target.cannot_be_scheduled?
@@ -24,24 +24,49 @@ module AgentControllerConcern
24 24
         end
25 25
       }
26 26
     when 'enable', 'disable'
27
+    when nil
28
+      errors.add(:base, "action must be specified")
29
+    when /\{[%{]/
30
+      # Liquid template
27 31
     else
28 32
       errors.add(:base, 'invalid action')
29 33
     end
30 34
   end
31 35
 
32 36
   def control!
33
-    control_targets.active.each { |target|
37
+    control_targets.each { |target|
34 38
       begin
35 39
         case control_action
36 40
         when 'run'
37
-          log "Agent run queued for '#{target.name}'"
38
-          Agent.async_check(target.id)
41
+          case
42
+          when target.cannot_be_scheduled?
43
+            error "'#{target.name}' cannot run without an incoming event"
44
+          when target.disabled?
45
+            log "Agent run ignored for disabled Agent '#{target.name}'"
46
+          else
47
+            Agent.async_check(target.id)
48
+            log "Agent run queued for '#{target.name}'"
49
+          end
39 50
         when 'enable'
40
-          log "Enabling the Agent '#{target.name}'"
41
-          target.update!(disable: false) if target.disabled?
51
+          case
52
+          when target.disabled?
53
+            target.update!(disabled: false)
54
+            log "Agent '#{target.name}' is enabled"
55
+          else
56
+            log "Agent '#{target.name}' is already enabled"
57
+          end
42 58
         when 'disable'
43
-          log "Disabling the Agent '#{target.name}'"
44
-          target.update!(disable: true) unless target.disabled?
59
+          case
60
+          when target.disabled?
61
+            log "Agent '#{target.name}' is alread disabled"
62
+          else
63
+            target.update!(disabled: true)
64
+            log "Agent '#{target.name}' is disabled"
65
+          end
66
+        when ''
67
+          # Do nothing
68
+        else
69
+          error "Unsupported action '#{control_action}' ignored for '#{target.name}'"
45 70
         end
46 71
       rescue => e
47 72
         error "Failed to #{control_action} '#{target.name}': #{e.message}"

+ 1 - 1
app/models/agents/scheduler_agent.rb

@@ -19,7 +19,7 @@ module Agents
19 19
 
20 20
       Set `action` to one of the action types below:
21 21
 
22
-      * `run`: This is the default.  Target Agents are run at intervals.
22
+      * `run`: Target Agents are run at intervals, except for those disabled.
23 23
 
24 24
       * `disable`: Target Agents are disabled (if not) at intervals.
25 25
 

+ 2 - 2
spec/lib/huginn_scheduler_spec.rb

@@ -86,11 +86,11 @@ describe Rufus::Scheduler do
86 86
 
87 87
     stub.any_instance_of(Agents::SchedulerAgent).second_precision_enabled { true }
88 88
 
89
-    @agent1 = Agents::SchedulerAgent.new(name: 'Scheduler 1', options: { schedule: '*/1 * * * * *' }).tap { |a|
89
+    @agent1 = Agents::SchedulerAgent.new(name: 'Scheduler 1', options: { action: 'run', schedule: '*/1 * * * * *' }).tap { |a|
90 90
       a.user = users(:bob)
91 91
       a.save!
92 92
     }
93
-    @agent2 = Agents::SchedulerAgent.new(name: 'Scheduler 2', options: { schedule: '*/1 * * * * *' }).tap { |a|
93
+    @agent2 = Agents::SchedulerAgent.new(name: 'Scheduler 2', options: { action: 'run', schedule: '*/1 * * * * *' }).tap { |a|
94 94
       a.user = users(:bob)
95 95
       a.save!
96 96
     }

+ 59 - 105
spec/models/agents/scheduler_agent_spec.rb

@@ -1,96 +1,68 @@
1 1
 require 'spec_helper'
2 2
 
3 3
 describe Agents::SchedulerAgent do
4
-  before do
5
-    @agent = Agents::SchedulerAgent.new(name: 'Example', options: { 'schedule' => '0 * * * *' })
6
-    @agent.user = users(:bob)
7
-    @agent.save
8
-  end
4
+  let(:valid_params) {
5
+    {
6
+      name: 'Example',
7
+      options: {
8
+        'action' => 'run',
9
+        'schedule' => '0 * * * *'
10
+      },
11
+    }
12
+  }
13
+
14
+  let(:agent) {
15
+    described_class.create!(valid_params) { |agent|
16
+      agent.user = users(:bob)
17
+    }
18
+  }
19
+
20
+  it_behaves_like AgentControllerConcern
9 21
 
10 22
   describe "validation" do
11
-    it "should validate action" do
12
-      ['run', 'enable', 'disable', '', nil].each { |action|
13
-        @agent.options['action'] = action
14
-        expect(@agent).to be_valid
15
-      }
16
-
17
-      ['delete', 1, true].each { |action|
18
-        @agent.options['action'] = action
19
-        expect(@agent).not_to be_valid
20
-      }
21
-    end
22
-
23 23
     it "should validate schedule" do
24
-      expect(@agent).to be_valid
24
+      expect(agent).to be_valid
25 25
 
26
-      @agent.options.delete('schedule')
27
-      expect(@agent).not_to be_valid
26
+      agent.options.delete('schedule')
27
+      expect(agent).not_to be_valid
28 28
 
29
-      @agent.options['schedule'] = nil
30
-      expect(@agent).not_to be_valid
29
+      agent.options['schedule'] = nil
30
+      expect(agent).not_to be_valid
31 31
 
32
-      @agent.options['schedule'] = ''
33
-      expect(@agent).not_to be_valid
32
+      agent.options['schedule'] = ''
33
+      expect(agent).not_to be_valid
34 34
 
35
-      @agent.options['schedule'] = '0'
36
-      expect(@agent).not_to be_valid
35
+      agent.options['schedule'] = '0'
36
+      expect(agent).not_to be_valid
37 37
 
38
-      @agent.options['schedule'] = '*/15 * * * * * *'
39
-      expect(@agent).not_to be_valid
38
+      agent.options['schedule'] = '*/15 * * * * * *'
39
+      expect(agent).not_to be_valid
40 40
 
41
-      @agent.options['schedule'] = '*/1 * * * *'
42
-      expect(@agent).to be_valid
41
+      agent.options['schedule'] = '*/1 * * * *'
42
+      expect(agent).to be_valid
43 43
 
44
-      @agent.options['schedule'] = '*/1 * * *'
45
-      expect(@agent).not_to be_valid
44
+      agent.options['schedule'] = '*/1 * * *'
45
+      expect(agent).not_to be_valid
46 46
 
47
-      stub(@agent).second_precision_enabled { true }
48
-      @agent.options['schedule'] = '*/15 * * * * *'
49
-      expect(@agent).to be_valid
47
+      stub(agent).second_precision_enabled { true }
48
+      agent.options['schedule'] = '*/15 * * * * *'
49
+      expect(agent).to be_valid
50 50
 
51
-      stub(@agent).second_precision_enabled { false }
52
-      @agent.options['schedule'] = '*/10 * * * * *'
53
-      expect(@agent).not_to be_valid
51
+      stub(agent).second_precision_enabled { false }
52
+      agent.options['schedule'] = '*/10 * * * * *'
53
+      expect(agent).not_to be_valid
54 54
 
55
-      @agent.options['schedule'] = '5/30 * * * * *'
56
-      expect(@agent).not_to be_valid
55
+      agent.options['schedule'] = '5/30 * * * * *'
56
+      expect(agent).not_to be_valid
57 57
 
58
-      @agent.options['schedule'] = '*/15 * * * * *'
59
-      expect(@agent).to be_valid
58
+      agent.options['schedule'] = '*/15 * * * * *'
59
+      expect(agent).to be_valid
60 60
 
61
-      @agent.options['schedule'] = '15,45 * * * * *'
62
-      expect(@agent).to be_valid
61
+      agent.options['schedule'] = '15,45 * * * * *'
62
+      expect(agent).to be_valid
63 63
 
64
-      @agent.options['schedule'] = '0 * * * * *'
65
-      expect(@agent).to be_valid
66
-    end
67
-  end
68
-
69
-  describe 'control_action' do
70
-    it "should be one of the supported values" do
71
-      ['run', '', nil].each { |action|
72
-        @agent.options['action'] = action
73
-        expect(@agent.control_action).to eq('run')
74
-      }
75
-
76
-      ['enable', 'disable'].each { |action|
77
-        @agent.options['action'] = action
78
-        expect(@agent.control_action).to eq(action)
79
-      }
80
-    end
81
-
82
-    it "cannot be 'run' if any of the control targets cannot be scheduled" do
83
-      expect(@agent.control_action).to eq('run')
84
-      @agent.control_targets = [agents(:bob_rain_notifier_agent)]
85
-      expect(@agent).not_to be_valid
86
-    end
87
-
88
-    it "can be 'enable' or 'disable' no matter if control targets can be scheduled or not" do
89
-      ['enable', 'disable'].each { |action|
90
-        @agent.options['action'] = action
91
-        @agent.control_targets = [agents(:bob_rain_notifier_agent)]
92
-        expect(@agent).to be_valid
93
-      }
64
+      agent.options['schedule'] = '0 * * * * *'
65
+      expect(agent).to be_valid
94 66
     end
95 67
   end
96 68
 
@@ -98,43 +70,25 @@ describe Agents::SchedulerAgent do
98 70
     it "should delete memory['scheduled_at'] if and only if options is changed" do
99 71
       time = Time.now.to_i
100 72
 
101
-      @agent.memory['scheduled_at'] = time
102
-      @agent.save
103
-      expect(@agent.memory['scheduled_at']).to eq(time)
73
+      agent.memory['scheduled_at'] = time
74
+      agent.save
75
+      expect(agent.memory['scheduled_at']).to eq(time)
104 76
 
105
-      @agent.memory['scheduled_at'] = time
106
-      # Currently @agent.options[]= is not detected
107
-      @agent.options = { 'schedule' => '*/5 * * * *' }
108
-      @agent.save
109
-      expect(@agent.memory['scheduled_at']).to be_nil
77
+      agent.memory['scheduled_at'] = time
78
+      # Currently agent.options[]= is not detected
79
+      agent.options = {
80
+        'action' => 'run',
81
+        'schedule' => '*/5 * * * *'
82
+      }
83
+      agent.save
84
+      expect(agent.memory['scheduled_at']).to be_nil
110 85
     end
111 86
   end
112 87
 
113 88
   describe "check!" do
114 89
     it "should control targets" do
115
-      control_targets = [agents(:bob_website_agent), agents(:bob_weather_agent)]
116
-      @agent.control_targets = control_targets
117
-      @agent.save!
118
-
119
-      control_target_ids = control_targets.map(&:id)
120
-      stub(Agent).async_check(anything) { |id|
121
-        control_target_ids.delete(id)
122
-      }
123
-
124
-      @agent.check!
125
-      expect(control_target_ids).to be_empty
126
-
127
-      @agent.options['action'] = 'disable'
128
-      @agent.save!
129
-
130
-      @agent.check!
131
-      control_targets.all? { |control_target| control_target.disabled? }
132
-
133
-      @agent.options['action'] = 'enable'
134
-      @agent.save!
135
-
136
-      @agent.check!
137
-      control_targets.all? { |control_target| !control_target.disabled? }
90
+      stub(agent).control!.once { nil }
91
+      agent.check!
138 92
     end
139 93
   end
140 94
 end

+ 111 - 0
spec/support/shared_examples/agent_controller_concern.rb

@@ -0,0 +1,111 @@
1
+require 'spec_helper'
2
+
3
+shared_examples_for AgentControllerConcern do
4
+  describe "preconditions" do
5
+    it "must be satisfied for these shared examples" do
6
+      expect(agent.user).to eq(users(:bob))
7
+      expect(agent.control_action).to eq('run')
8
+    end
9
+  end
10
+
11
+  describe "validation" do
12
+    describe "of action" do
13
+      it "should allow certain values" do
14
+        ['run', 'enable', 'disable', '{{ action }}'].each { |action|
15
+          agent.options['action'] = action
16
+          expect(agent).to be_valid
17
+        }
18
+      end
19
+
20
+      it "should disallow obviously bad values" do
21
+        ['delete', nil, 1, true].each { |action|
22
+          agent.options['action'] = action
23
+          expect(agent).not_to be_valid
24
+        }
25
+      end
26
+
27
+      it "should accept 'run' if all target agents are schedulable" do
28
+        agent.control_targets = [agents(:bob_website_agent)]
29
+        expect(agent).to be_valid
30
+      end
31
+
32
+      it "should reject 'run' if targets include an unschedulable agent" do
33
+        agent.control_targets = [agents(:bob_rain_notifier_agent)]
34
+        expect(agent).not_to be_valid
35
+      end
36
+
37
+      it "should not reject 'enable' or 'disable' no matter if targets include an unschedulable agent" do
38
+        ['enable', 'disable'].each { |action|
39
+          agent.options['action'] = action
40
+          agent.control_targets = [agents(:bob_rain_notifier_agent)]
41
+          expect(agent).to be_valid
42
+        }
43
+      end
44
+    end
45
+  end
46
+
47
+  describe 'control_action' do
48
+    it "returns options['action']" do
49
+      expect(agent.control_action).to eq('run')
50
+
51
+      ['run', 'enable', 'disable'].each { |action|
52
+        agent.options['action'] = action
53
+        expect(agent.control_action).to eq(action)
54
+      }
55
+    end
56
+
57
+    it "returns the result of interpolation" do
58
+      expect(agent.control_action).to eq('run')
59
+
60
+      agent.options['action'] = '{{ "enable" }}'
61
+      expect(agent.control_action).to eq('enable')
62
+    end
63
+  end
64
+
65
+  describe "control!" do
66
+    before do
67
+      agent.control_targets = [agents(:bob_website_agent), agents(:bob_weather_agent)]
68
+      agent.save!
69
+    end
70
+
71
+    it "should run targets" do
72
+      control_target_ids = agent.control_targets.map(&:id)
73
+      stub(Agent).async_check(anything) { |id|
74
+        control_target_ids.delete(id)
75
+      }
76
+
77
+      agent.control!
78
+      expect(control_target_ids).to be_empty
79
+    end
80
+
81
+    it "should not run disabled targets" do
82
+      control_target_ids = agent.control_targets.map(&:id)
83
+      stub(Agent).async_check(anything) { |id|
84
+        control_target_ids.delete(id)
85
+      }
86
+
87
+      agent.control_targets.last.update!(disabled: true)
88
+
89
+      agent.control!
90
+      expect(control_target_ids).to eq [agent.control_targets.last.id]
91
+    end
92
+
93
+    it "should enable targets" do
94
+      agent.options['action'] = 'disable'
95
+      agent.save!
96
+      agent.control_targets.first.update!(disabled: true)
97
+
98
+      agent.control!
99
+      expect(agent.control_targets.reload).to all(be_disabled)
100
+    end
101
+
102
+    it "should disable targets" do
103
+      agent.options['action'] = 'enable'
104
+      agent.save!
105
+      agent.control_targets.first.update!(disabled: true)
106
+
107
+      agent.control!
108
+      expect(agent.control_targets.reload).to all(satisfy { |a| !a.disabled? })
109
+    end
110
+  end
111
+end