SIGTERM should not log as Error

[carste98]

Hi!

When running varnish in for example a Kubernetes environment, the container is regularly restarted as part of upgrades.

I have noticed that everytime the container receives a SIGTERM signal (which it gets everytime it is restarted) it prints.

Error: Manager got SIGTERM

I would not classify it as an "application error" when the container receives a SIGTERM signal during an expected rolling upgrade.

This "Error" is then also sent to the logging stack and creates noise in the logging dashboards.

It is fully possible to get around this with some custom log filtering, but it would be nice if the log level got changed at source!

[nigoroll]

You could implement an explicit varnishadm stop instead of sending the signal.

[carste98]

I'm unsure if I follow what you are suggesting?
The default behaviour of Kubernetes Kubelet is to send a SIGTERM to allow process to gracefully terminate. https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-termination.

Is there a reason why it is classified as "Error" when receiving a SIGTERM?

[dridi]

@carste98 this has almost always been treated as an error, going from stdout to stderr early on, and eventually gaining the "Error:" prefix in 6e5ffc0 10 years ago.

I agree that this does not have to be reported as an error, and it could be output as a simple information that a signal was received. It wouldn't be difficult:

--- i/bin/varnishd/mgt/mgt_main.c
+++ w/bin/varnishd/mgt/mgt_main.c
@@ -449,7 +449,8 @@ mgt_sigint(const struct vev *e, int what)
 {
 
        (void)what;
-       MGT_Complain(C_ERR, "Manager got %s from PID %jd", e->name, (intmax_t)e->siginfo->si_pid);
+       MGT_Complain(C_INFO, "Manager got %s from PID %jd",
+           e->name, (intmax_t)e->siginfo->si_pid);
        (void)fflush(stdout);
        if (MCH_Running())
                MCH_Stop_Child();

The mgt_sigint() function is used for both SIGINT and SIGTERM signals, in case you were wondering.

Out of curiosity, could you please elaborate on your current stack? Varnish runs in a pod, and then how are you ingesting MGT "complaints"? What logging stack and dashboard system are you using?

[carste98]

Hi!
Cool! I figured it might be a quick fix, and to be even less disruptive it could be separate handling for SIGTERM only if that seems more reasonable. My 2cents are that SIGTERM is a gentle nudge to terminate and should not classify as an Error in the application, but that does not mean that you guys have to agree!

Regarding setup:
I run varnish in various environments with different log stacks.

But as per a "Kubernetes standard way" the environments share the common workflow of continuously retrieving all logs that are printed to stdout, stderr in each container in the Kubernetes Cluster and sending them to some log database (Opensearch, ElasticSearch) which makes them searchable through some log ui (Opensearch-dashboards/Kibana).

Management of containers on each worker node in Kubernetes is done by the Kubelet process, so If i declaratively tell the kubernetes api-server that I would like to do a rollout restart or upgrade to a new version, the kubelet will send a SIGTERM to allow it a graceful termination before actually removing the workload and spinning up a new version.

[dridi]

If it were up to me I would have already pushed the diff above. Please note that MGT complaints are also syslog'd, but I'm not sure what that entails in a kube environment.