StackStorm / st2

StackStorm (aka "IFTTT for Ops") is event-driven automation for auto-remediation, incident responses, troubleshooting, deployments, and more for DevOps and SREs. Includes rules engine, workflow, 160 integration packs with 6000+ actions (see https://exchange.stackstorm.org) and ChatOps. Installer at https://docs.stackstorm.com/install/index.html
https://stackstorm.com/
Apache License 2.0
6.09k stars 747 forks source link

Jinja not rendered in rule trigger parameters #3681

Open nmaludy opened 7 years ago

nmaludy commented 7 years ago

I have a rule that i'm trying to define using a cron timer with values from the key/value store.

---
name: full_backup_cron
pack: backups
description: "Executes a full backup on a cron schedule."
enabled: true

trigger:
  type: "core.st2.CronTimer"
  # http://apscheduler.readthedocs.io/en/3.0/modules/triggers/cron.html#api
  parameters:
      timezone: "{{ st2kv.system.backups.full_backup_cron.timezone }}"
      day_of_week: "{{ st2kv.system.backups.full_backup_cron.day_of_week }}"
      hour: "{{ st2kv.system.backups.full_backup_cron.hour }}"
      minute: "{{ st2kv.system.backups.full_backup_cron.minute }}"
      second: "{{ st2kv.system.backups.full_backup_cron.second }}"

action:
  ref: "backups.full_backup"

When i try to register the rule i get the following error in st2api logs.

2017-08-16 21:28:18,008 83527792 AUDIT auth [-] Token with id "59946e2aa814c0066070ade8" is validated.
2017-08-16 21:28:18,020 83527792 ERROR router [-] Failed to call controller function "put" for operation "st2api.controllers.v1.rules:rule_controller.put": '{{ st2kv.system.backups.full_backup_cron.timezone }}'
Traceback (most recent call last):
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2common/router.py", line 426, in __call__
    resp = func(**kw)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2api/controllers/v1/rules.py", line 145, in put
    rule_db = RuleAPI.to_model(rule)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2common/models/api/rule.py", line 238, in to_model
    parameters=parameters)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2common/validators/api/reactor.py", line 106, in validate_trigger_parameters
    CronTrigger(**parameters)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/apscheduler/triggers/cron/__init__.py", line 50, in __init__
    self.timezone = astimezone(timezone)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/apscheduler/util.py", line 78, in astimezone
    return timezone(obj)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/pytz/__init__.py", line 181, in timezone
    raise UnknownTimeZoneError(zone)
UnknownTimeZoneError: '{{ st2kv.system.backups.full_backup_cron.timezone }}'
2017-08-16 21:28:18,020 83527792 ERROR error_handling [-] API call failed: '{{ st2kv.system.backups.full_backup_cron.timezone }}'
Traceback (most recent call last):
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2common/middleware/error_handling.py", line 46, in __call__
    return self.app(environ, start_response)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2common/router.py", line 472, in as_wsgi
    resp = self(req)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2common/router.py", line 430, in __call__
    raise e
UnknownTimeZoneError: '{{ st2kv.system.backups.full_backup_cron.timezone }}' (_exception_data={},_exception_class='UnknownTimeZoneError',_exception_message="'{{ st2kv.system.backups.full_backup_cron.timezone }}'")
2017-08-16 21:28:18,021 83527792 INFO logging [-] b1a61d2e-ad0c-4776-b680-dab9240ba8d0 - 500 46 14.663ms (content_length=46,request_id='b1a61d2e-ad0c-4776-b680-dab9240ba8d0',runtime=14.663,remote_addr='127.0.0.1',status=500,method='PUT',path='/v1/rules/5994ea75a814c02ce9813285')
lakshmi-kannan commented 7 years ago

@nmaludy Interesting. We don't resolve jinja aprior because some of those parameters for the rule can come from the payload of incoming trigger. Looks like we need to do jinja resolution prior to registration and prior to "enforcing" the rule. I am looking into this but wanted to ack that I see the same stack trace.

nmaludy commented 7 years ago

@lakshmi-kannan

Use case, and why this originated:

I wrote (and still am in the process of finalizing) a pack that performs a backup of StackStorm. I would really love to publish this on the exchange, but in doing so i'm going to need to make an "executive decision" as to what cron schedule the backup should be performed on. If the user downloads/installs this pack and modifies the rule manually, then the next update they do will overwrite this change back to the value in the pack from exchange.

Ideally what i would like to do is store this information in either a config option or datastore key/value. Then, the user can install the pack and either update a config and/or a datastore key/value to set their schedule. Now, when the pack is update it will maintain the schedule the user wants for their backups.

This happens to be a common pattern not just for backups, but for several other (currently 6 on my plate) scheduled/cron tasks that i am working on.

dzimine commented 7 years ago

@nmaludy

What is your expectations on changing config or KV parameters used in the rules for triggers? Would it be confusing to you if you changed KV and find that the trigger parameters have not updated? Will you easily find the current acting values of trigger parameters? Will you know what to do for the new KV or config parameters to take effect?

cc @lakshmi-kannan - we want to clear our concerns with potential confusion the JINJA parameters may cause so best it to hear from the real user :)

Thanks!

nmaludy commented 7 years ago

@dzimine @lakshmi-kannan

Q: What is your expectations on changing config or KV parameters used in the rules for triggers? A: I think it's reasonable to treat this similar to loading/reloading config values for actions. Recently we added a note to many of the pack README files to say something along the lines of "when changing values in the config, please make sure to run an st2ctl reload --register-all". If we document this properly, i think the confusion would be minimized.

Q: Would it be confusing to you if you changed KV and find that the trigger parameters have not updated? A: I don't think so, as long as it's documented properly. Trying to think about how to implement this by "automatically discovering", and you would have to either do it on a polling basis (easy, but semi-painful), or some magic with dependency graphs that know to update certain rules when specific K/V change (hard).

Q: Will you easily find the current acting values of trigger parameters? A: If the current values are rendered in st2 rule get xxx i don't think it would be to difficult to diagnose. Also would be interesting if this could show what Jinja they're rendered from, to differentiate them in some way from "static values" (not a requirement, but just a thought)?

Q: Will you know what to do for the new KV or config parameters to take effect? A: Again, i think this is a documentation and/or training piece if it's not handled in an automatic way.

stale[bot] commented 5 years ago

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.

nmaludy commented 5 years ago

Still an issue

stale[bot] commented 5 years ago

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.

nmaludy commented 5 years ago

Still an issue.

stale[bot] commented 5 years ago

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.