StackStorm / stackstorm-k8s

K8s Helm Chart that codifies StackStorm (aka "IFTTT for Ops" https://stackstorm.com/) Highly Availability fleet as a simple to use reproducible infrastructure-as-code app
https://helm.stackstorm.com/
Apache License 2.0
105 stars 107 forks source link

proper graceful shutdown settings #381

Open guzzijones opened 10 months ago

guzzijones commented 10 months ago

We have all the proper graceful shutdown settings for actionrunner and workflow, but we are still seeing action executions get stuck in a running state. At a mininum they should be abandoned per actionrunner code.

I do notice that we are performing a query coordinator.get_members(service.encode("utf-8")).get()

and then adding to a counter to determine if we are past the expiration.
What may be happening is that if the action_runner.graceful_shutdown config is set to say 100 seconds over 100 seconds need to actually pass before the logic to abandon executions is called

This would mean that we could set the terminationGracePeriodSeconds to say 300 seconds longer (or some long time) than the action_runner.graceful_shutdown seconds to ensure that the pod is alive long enough to let the abandon process finish.

I have made this change so we can monitor our prod cluster.

A code fix would be to use the action_runner.graceful_shutdown config to calculate and end date time and then check against that in the while loop

guzzijones commented 10 months ago

After making this change for a shorter action_runner.graceful_shutdown we still see the actions getting stuck in a running state.

guzzijones commented 10 months ago

Let this be a lesson to everyone. Do NOT put inline comments in your config file:

...
 return opt.type(value)","host":"stackstorm","severity":"info","facility":"user",
{"timestamp":"2023-12-04T18:23:26+00:00","message":" 2023-12-04T18:23:19.7932543Z stdout F   File \"/opt/stackstorm/st2/lib/python3.8/site-packages/oslo_config/types.py\", line 145, in __call__","host":"st
{"timestamp":"2023-12-04T18:23:26+00:00","message":" 2023-12-04T18:23:19.79325789Z stdout F     value = int(value)","host":"stackstorm","severity":"info","facility":"user","sysl
{"timestamp":"2023-12-04T18:23:26+00:00","message":" 2023-12-04T18:23:19.793259422Z stdout F ValueError: invalid literal for int() with base 10: '610 # 10 mins'","host":"stackstorm
...
guzzijones commented 10 months ago

Never comment anything ever. EVER.

guzzijones commented 10 months ago

Whelp it turns out I STILL am not seeing graceful shutdowns. All executions immediately get abandoned. I did some more digging. Here we check the st2actionrunner service if it has any members. But in the config for st2 coordination | service_registry defaults to FALSE. So set service_registry = True in the coorination settings in the config. We should probably make this the default?

guzzijones commented 10 months ago

I can confirm turning on the service registry fixed my graceful shutdown.

cognifloyd commented 6 months ago

But in the config for st2 coordination | service_registry defaults to FALSE. So set service_registry = True in the coorination settings in the config. We should probably make this the default?

This looks like the right place to add anything in the chart: https://github.com/StackStorm/stackstorm-k8s/blob/cc94bd05f367e1863451d963c11521d50cd3c126/templates/configmaps_st2-conf.yaml#L18-L21

We could do this:

     {{- if index .Values "redis" "enabled" }}
     [coordination]
+    service_registry = True
     url = redis://{{ template "stackstorm-ha.redis-password" $ }}{{ template "stackstorm-ha.redis-nodes" $ }}
     {{- end }}

I do not use the redis subchart, so that would not change the default for me. I pass this in via st2.config in values. So, we could also do something like:

+    [coordination]
+    service_registry = True
     {{- if index .Values "redis" "enabled" }}
-    [coordination]
     url = redis://{{ template "stackstorm-ha.redis-password" $ }}{{ template "stackstorm-ha.redis-nodes" $ }}
     {{- end }}

Which option would you prefer? Or something else entirely?

ajjonen commented 1 month ago

second option is probably best