[Alerting] Alerting now uses Task Manager intervals internally

x-pack/legacy/plugins/alerting/server/alerts_client.test.ts

@@ -195,24 +195,25 @@ describe('create()', () => {
                                                                             `);
     expect(taskManager.schedule).toHaveBeenCalledTimes(1);
     expect(taskManager.schedule.mock.calls[0]).toMatchInlineSnapshot(`
-                                                                        Array [
-                                                                          Object {
-                                                                            "params": Object {
-                                                                              "alertId": "1",
-                                                                              "spaceId": "default",
-                                                                            },
-                                                                            "scope": Array [
-                                                                              "alerting",
-                                                                            ],
-                                                                            "state": Object {
-                                                                              "alertInstances": Object {},
-                                                                              "alertTypeState": Object {},
-                                                                              "previousStartedAt": null,
-                                                                            },
-                                                                            "taskType": "alerting:123",
-                                                                          },
-                                                                        ]
-                                                `);
+      Array [
+        Object {
+          "interval": "10s",
+          "params": Object {
+            "alertId": "1",
+            "spaceId": "default",
+          },
+          "scope": Array [
+            "alerting",
+          ],
+          "state": Object {
+            "alertInstances": Object {},
+            "alertTypeState": Object {},
+            "previousStartedAt": null,
+          },
+          "taskType": "alerting:123",
+        },
+      ]
+    `);
     expect(savedObjectsClient.update).toHaveBeenCalledTimes(1);
     expect(savedObjectsClient.update.mock.calls[0]).toHaveLength(3);
     expect(savedObjectsClient.update.mock.calls[0][0]).toEqual('alert');
@@ -583,6 +584,7 @@ describe('enable()', () => {
     );
     expect(taskManager.schedule).toHaveBeenCalledWith({
       taskType: `alerting:2`,
+      interval: '10s',
       params: {
         alertId: '1',
         spaceId: 'default',
@@ -664,6 +666,7 @@ describe('enable()', () => {
     );
     expect(taskManager.schedule).toHaveBeenCalledWith({
       taskType: `alerting:2`,
+      interval: '10s',
       params: {
         alertId: '1',
         spaceId: 'default',
@@ -1262,6 +1265,154 @@ describe('update()', () => {
                                                                             `);
   });
 
+  test('reschedules the Alert Task if the interval is changed', async () => {
+    const alertsClient = new AlertsClient(alertsClientParams);
+    alertTypeRegistry.get.mockReturnValueOnce({
+      id: '123',
+      name: 'Test',
+      actionGroups: ['default'],
+      async executor() {},
+    });
+    savedObjectsClient.get.mockResolvedValueOnce({
+      id: '1',
+      type: 'alert',
+      attributes: {
+        enabled: true,
+        alertTypeId: '123',
+        scheduledTaskId: 'task-123',
+      },
+      references: [],
+      version: '123',
+    });
+    savedObjectsClient.update.mockResolvedValueOnce({
+      id: '1',
+      type: 'alert',
+      attributes: {
+        enabled: true,
+        interval: '10s',
+        alertTypeParams: {
+          bar: true,
+        },
+        actions: [
+          {
+            group: 'default',
+            actionRef: 'action_0',
+            params: {
+              foo: true,
+            },
+          },
+        ],
+        scheduledTaskId: 'task-123',
+      },
+      references: [
+        {
+          name: 'action_0',
+          type: 'action',
+          id: '1',
+        },
+      ],
+    });
+    await alertsClient.update({
+      id: '1',
+      data: {
+        interval: '100s',
+        name: 'abc',
+        tags: ['foo'],
+        alertTypeParams: {
+          bar: true,
+        },
+        actions: [
+          {
+            group: 'default',
+            id: '1',
+            params: {
+              foo: true,
+            },
+          },
+        ],
+      },
+    });
+
+    expect(taskManager.reschedule.mock.calls[0][0]).toMatchInlineSnapshot(`
+      Object {
+        "id": "task-123",
+        "interval": "100s",
+      }
+    `);
+  });
+
+  test('does not reschedule the Alert Task if the interval is unchanged', async () => {
+    const alertsClient = new AlertsClient(alertsClientParams);
+    alertTypeRegistry.get.mockReturnValueOnce({
+      id: '123',
+      name: 'Test',
+      actionGroups: ['default'],
+      async executor() {},
+    });
+    savedObjectsClient.get.mockResolvedValueOnce({
+      id: '1',
+      type: 'alert',
+      attributes: {
+        enabled: true,
+        interval: '10s',
+        alertTypeId: '123',
+        scheduledTaskId: 'task-123',
+      },
+      references: [],
+      version: '123',
+    });
+    savedObjectsClient.update.mockResolvedValueOnce({
+      id: '1',
+      type: 'alert',
+      attributes: {
+        enabled: true,
+        interval: '10s',
+        alertTypeParams: {
+          bar: true,
+        },
+        actions: [
+          {
+            group: 'default',
+            actionRef: 'action_0',
+            params: {
+              foo: true,
+            },
+          },
+        ],
+        scheduledTaskId: 'task-123',
+      },
+      references: [
+        {
+          name: 'action_0',
+          type: 'action',
+          id: '1',
+        },
+      ],
+    });
+    await alertsClient.update({
+      id: '1',
+      data: {
+        interval: '10s',
+        name: 'abc',
+        tags: ['foo'],
+        alertTypeParams: {
+          bar: true,
+        },
+        actions: [
+          {
+            group: 'default',
+            id: '1',
+            params: {
+              foo: true,
+            },
+          },
+        ],
+      },
+    });
+
+    expect(taskManager.reschedule).not.toHaveBeenCalled();
+  });
+
   it('calls the createApiKey function', async () => {
     const alertsClient = new AlertsClient(alertsClientParams);
     alertTypeRegistry.get.mockReturnValueOnce({

x-pack/legacy/plugins/alerting/server/alerts_client.ts

@@ -139,7 +139,7 @@ export class AlertsClient {
         scheduledTask = await this.scheduleAlert(
           createdAlert.id,
           rawAlert.alertTypeId,
-          rawAlert.interval
+          data.interval
         );
       } catch (e) {
         // Cleanup data, something went wrong scheduling the task
@@ -223,6 +223,10 @@ export class AlertsClient {
         references,
       }
     );
+    if (data.interval !== attributes.interval) {
+      // if interval has changed, we should reschedule the task
+      await this.rescheduleAlert(attributes.scheduledTaskId, data.interval);
+    }
     return this.getAlertFromRaw(id, updatedObject.attributes, updatedObject.references);
   }
 
@@ -358,6 +362,7 @@ export class AlertsClient {
   private async scheduleAlert(id: string, alertTypeId: string, interval: string) {
     return await this.taskManager.schedule({
       taskType: `alerting:${alertTypeId}`,
+      interval,
       params: {
         alertId: id,
         spaceId: this.spaceId,
@@ -371,6 +376,13 @@ export class AlertsClient {
     });
   }
 
+  private async rescheduleAlert(scheduledTaskId: string, interval: string) {
+    return await this.taskManager.reschedule({
+      id: scheduledTaskId,
+      interval,
+    });
+  }
+
   private extractReferences(actions: Alert['actions']) {
     const references: SavedObjectReference[] = [];
     const rawActions = actions.map((action, i) => {

x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.test.ts

@@ -134,7 +134,6 @@ test('successfully executes the task', async () => {
   const runnerResult = await taskRunner.run();
   expect(runnerResult).toMatchInlineSnapshot(`
                                 Object {
-                                  "runAt": 1970-01-01T00:00:10.000Z,
                                   "state": Object {
                                     "alertInstances": Object {},
                                     "alertTypeState": undefined,

x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.ts

@@ -9,7 +9,6 @@ import { RunContext } from '../../../task_manager';
 import { createExecutionHandler } from './create_execution_handler';
 import { createAlertInstanceFactory } from './create_alert_instance_factory';
 import { AlertInstance } from './alert_instance';
-import { getNextRunAt } from './get_next_run_at';
 import { validateAlertTypeParams } from './validate_alert_type_params';
 import { PluginStartContract as EncryptedSavedObjectsStartContract } from '../../../../../plugins/encrypted_saved_objects/server';
 import { PluginStartContract as ActionsPluginStartContract } from '../../../actions';
@@ -94,7 +93,7 @@ export class TaskRunnerFactory {
         const services = getServices(fakeRequest);
         // Ensure API key is still valid and user has access
         const {
-          attributes: { alertTypeParams, actions, interval, throttle, muteAll, mutedInstanceIds },
+          attributes: { alertTypeParams, actions, throttle, muteAll, mutedInstanceIds },
           references,
         } = await services.savedObjectsClient.get<RawAlert>('alert', alertId);
 
@@ -167,15 +166,12 @@ export class TaskRunnerFactory {
           })
         );
 
-        const nextRunAt = getNextRunAt(new Date(taskInstance.startedAt!), interval);
-
         return {
           state: {
             alertTypeState,
             alertInstances,
             previousStartedAt: taskInstance.startedAt!,
           },
-          runAt: nextRunAt,
         };
       },
     };

x-pack/legacy/plugins/alerting/server/shim.ts

@@ -40,7 +40,10 @@ export interface Server extends Legacy.Server {
 /**
  * Shim what we're thinking setup and start contracts will look like
  */
-export type TaskManagerStartContract = Pick<TaskManager, 'schedule' | 'fetch' | 'remove'>;
+export type TaskManagerStartContract = Pick<
+  TaskManager,
+  'schedule' | 'reschedule' | 'fetch' | 'remove'
+>;
 export type SecurityPluginSetupContract = Pick<SecurityPlugin, '__legacyCompat'>;
 export type SecurityPluginStartContract = Pick<SecurityPlugin, 'authc'>;
 export type XPackMainPluginSetupContract = Pick<XPackMainPlugin, 'registerFeature'>;

x-pack/legacy/plugins/task_manager/README.md

@@ -265,6 +265,19 @@ The danger is that in such a situation, a Task with that same `id` might already
 
 To achieve this you should use the `ensureScheduling` api which has the exact same behavior as `schedule`, except it allows the scheduling of a Task with an `id` that's already in assigned to another Task and it will assume that the existing Task is the one you wished to `schedule`, treating this as a successful operation.
 
+### reschedule
+The `reschedule` api allows a developer to reconfigure an existing Task's `runAt` and `interval` fields.
+Given the `id` of an existing Task, you may specify either, or both, of these fields and the Task's fields will be updated accordingly, returning the updated Task.
+
+One thing to note is that this api behaves mildly differently depending on the state of the Task.
+
+If a request is made to `reschedule` the `interval` field of an `idle` task, the task's field will be updated and the task will run immediately and the new `interval` will be applied to scheduling the _next_ run.
+If a request is made to `reschedule` the `runAt` field of an `idle` task, irrespective of whether the `interval` field is also specified, the task's field(s) will be updated and the task will only run when the newly specified `runAt` expires (after which, any new `interval` that may also have been specified, will be applied to the _next_ scheduled run).
+These behaviors mirrors how `schedule` treats these two fields.
+
+Where this behavior diverges is if a request is made to `reschedule` a non `idle` task, such as a `running` task or a `failed` task.
+In such a case, `reschedule` will avoid changing the `runAt` field, as it is used internally to manage Task Manager lifecycles and changing that field in such a case could be considered dangerous. `interval` on the other hand, will be updated in such a case, and we recommend using the Task returned by the `reschedule` api to assess whether the fields have been updated as expected.
+
 ### more options
 
 More custom access to the tasks can be done directly via Elasticsearch, though that won't be officially supported, as we can change the document structure at any time.