Does kubernetes_logs cause tokio task leaks after config reloading?

[xdatcloud]

I found current codes of source kubernetes_logs spawn tokio task directly but not respect the shutdown signal:
(https://github.com/vectordotdev/vector/blob/master/src/sources/kubernetes_logs/mod.rs#L310-L331)

        let pods = Api::<Pod>::all(client.clone());
        let pod_watcher = watcher(
            pods,
            ListParams {
                field_selector: Some(field_selector),
                label_selector: Some(label_selector),
                ..Default::default()
            },
        );
        let pod_store_w = reflector::store::Writer::default();
        let pod_state = pod_store_w.as_reader();

        tokio::spawn(custom_reflector(pod_store_w, pod_watcher, delay_deletion));

        // -----------------------------------------------------------------

        let namespaces = Api::<Namespace>::all(client);
        let ns_watcher = watcher(namespaces, ListParams::default());
        let ns_store_w = reflector::store::Writer::default();
        let ns_state = ns_store_w.as_reader();

        tokio::spawn(custom_reflector(ns_store_w, ns_watcher, delay_deletion));

And inside custom_reflector is an infinite loop, I wonder if it will cause tokio task leaks after kubernetes_logs config modified & reloads in-flight? - Source will destruct and rebuild, but previous reflector task doesn't respect the shutdown signal.

[jszwedko]

Yeah, that's a good question. It does seem like they should be wired up to the source shutdown signal. cc/ @spencergilbert @lukesteensen

[xdatcloud]

🤔 I seems like it could, though I'm not sure how config reloading "works" in Kubernetes at this time - there's not really a reliable way to replace the config file/s without also just redeploying the entire pod (unless one is running a sidecar application that does the replacing).

Hi @spencergilbert , I assemble vector configs as Kubernetes ConfigMap, and attach them into Pod's inner filesystem through volumeMounts. Vector (as DaemonSet) is started with -w argument, thus it will reloads configs if I changed the content of ConfigMap (changes will sync to pod's inner filesystem).

[spencergilbert]

Fair - I've traditionally avoided that since when the ConfigMap actually gets updated is a little "loose" (docs: ConfigMap).

@jszwedko unless @lukesteensen sees something I don't this is probably worth opening an issue to track and resolve.

[jszwedko]

Yeah, it seems likely these tasks are leaking. I created #12472 to track this. I think they just need to be wired up to the shutdown signal.

[lukesteensen]

Yeah, that sounds right to me. Whenever we spawn something that would otherwise run indefinitely, we should pass along the shutdown signal.

[nabokihms]

Sorry, I was just passing by and wanted to share a real-world example of reloading vector process in Kubernetes on the fly.

https://github.com/deckhouse/deckhouse/blob/99e33c509fe621fe79cc3b8a9ed35d3d17ee14e0/modules/460-log-shipper/images/vector/reloader

Vector config consists of two parts - static configmap and dynamic secret. This secret is mounted to the pod and updated automatically by kubelet. There are two containers running in a shared process namespace from the same vector image - vector running the vector process and vector-reloader running the reloader script I attached above.

Reloader logs:

Tue May  3 07:42:50 UTC 2022 | Start watching the /opt/vector/vector.json file.
Tue May  3 09:26:57 UTC 2022 | Start reloading a config.
√ Loaded ["/etc/vector/default/defaults.json", "/tmp/tmp_vector_conf/vector.json"]
√ Component configuration
√ Health check "prometheus_sink"
~ Health check disabled for "d8_cluster_sink_flant-integration-loki-storage"
----------------------------------------------------------------------------------
                                                                         Validated
--- /etc/vector/dynamic/vector.json	2022-05-03 07:42:50.170079581 +0000
+++ /tmp/tmp_vector_conf/vector.json	2022-05-03 09:26:57.436183874 +0000
@@ -1,21 +1,5 @@
 {
   "sources": {
-    "d8_clusterns_source_d8-cni-cilium_flant-integration-d8-logs_flant-integration-loki-storage": {
-      "type": "kubernetes_logs",
-      "extra_label_selector": "app=deckhouse",
-      "extra_field_selector": "metadata.name!=log-shipper-agent-6j97w,metadata.namespace=d8-cni-cilium",
-      "annotation_fields": {
-        "container_image": "image",
-        "container_name": "container",
-        "pod_ip": "pod_ip",
-        "pod_labels": "pod_labels",
-        "pod_name": "pod",
-        "pod_namespace": "namespace",
-        "pod_node_name": "node",
-        "pod_owner": "pod_owner"
-      },
-      "glob_minimum_cooldown_ms": 1000
-    },
     "d8_clusterns_source_d8-monitoring_flant-integration-d8-logs_flant-integration-loki-storage": {
       "type": "kubernetes_logs",
       "extra_label_selector": "app=deckhouse",
@@ -54,8 +38,7 @@
       "drop_on_abort": false,
       "inputs": [
         "d8_clusterns_source_d8-system_flant-integration-d8-logs_flant-integration-loki-storage",
-        "d8_clusterns_source_d8-monitoring_flant-integration-d8-logs_flant-integration-loki-storage",
-        "d8_clusterns_source_d8-cni-cilium_flant-integration-d8-logs_flant-integration-loki-storage"
+        "d8_clusterns_source_d8-monitoring_flant-integration-d8-logs_flant-integration-loki-storage"
       ],
       "source": "if exists(.pod_labels.\"controller-revision-hash\") {\n    del(.pod_labels.\"controller-revision-hash\")\n}\nif exists(.pod_labels.\"pod-template-hash\") {\n    del(.pod_labels.\"pod-template-hash\")\n}\nif exists(.kubernetes) {\n    del(.kubernetes)\n}\nif exists(.file) {\n    del(.file)\n}",
       "type": "remap"
Tue May  3 09:26:57 UTC 2022 | Vector config has been reloaded.