Use HTTP liveness probe instead of tetra status

[tpapagian]

Please ensure your pull request adheres to the following guidelines:

• For first time contributors, read Submitting a pull request.
• All code is covered by unit and/or end-to-end tests tests where feasible.
• All commits contain a well written commit message including a title,
  description and a Fixes: #XXX line if the commit addresses a particular
  GitHub issue.
• All commits are signed off. See the section Developer’s Certificate of Origin
• Provide a title or release-note blurb suitable for the release notes.
• Are you a user of Tetragon? Please add yourself to the Users doc in the Cilium repository.
• Thanks for contributing!

Now, we use tetra status command to report the status of tetragon agent. This comes with some overheads as tetra binary has a lot of additional functionality and it seems like an overkill to use that for status reporting.

On the other hand, k8s supports liveness probes by using an HTTP endpoint (i.e. https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#http-probes). This patch first creates a new HTTP endpoint to report agent status and then change helm to make use of that.

Use HTTP liveness probe instead of tetra status.

[netlify]

✅ Deploy Preview for tetragon ready!

Name	Link
🔨 Latest commit	d9bd92c
🔍 Latest deploy log	https://app.netlify.com/sites/tetragon/deploys/6655a0fd9b19f20009e73930
😎 Deploy Preview	https://deploy-preview-2474--tetragon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[kkourt]

I think it would better to have this under livenessProbe somehow.

CC: @michi-covalent @lambdanis

[tpapagian]

Maybe we could do something like:

{{- if .Values.tetragon.livenessProbe.enabled }}
  livenessProbe:
    timeoutSeconds: 60
{{- if .Values.tetragon.livenessProbe.custom }}  
  {{- toYaml .Values.tetragon.livenessProbe.custom | nindent 4 }}
{{- else if .Values.tetragon.livenessProbe.http.enabled }}
     httpGet:
       path: /liveness
       port: 6789
{{- if .Values.tetragon.livenessProbe.http.host }}
       host: {{ .Values.tetragon.livenessProbe.http.host }}
{{- end }}
{{- else if .Values.tetragon.grpc.enabled }}
     exec:
       command:
       - tetra
       - status
       - --server-address
       - {{ .Values.tetragon.grpc.address }}
       - --retries
       - "5"
{{- end -}}
{{- end -}}

[tpapagian]

Using that we can have:

---

livenessProbe:
  enabled: false

to disable liveness probe.

---

livenessProbe:
  enabled: true
  custom:
    grpc:
      port: 54321

to define a custom liveness probe.

---

livenessProbe:
  enabled: true
  http:
    enabled: true

to define an http-based liveness probe.

---

livenessProbe:
  enabled: true

to define our legacy "tetra status"-based liveness probe (assuming tetragon.grpc.enabled is true).

[lambdanis]

Maybe I'm missing something, but can we just change the default livenessProbe and keep everything else the same? That is, change the default values to:

livenessProbe:
  timeoutSeconds: 60
  httpGet:
    path: /liveness
    port: 6789

and keep _container_tetragon.tpl unchanged. Then users can still use the tetra probe by setting livenessProbe: {}.

[tpapagian]

Yes, this makes sense (easier compared to what I am trying to do). But what I am not able to achieve is setting livenessProbe: {}. Possibly I am missing something.

First, change the existing values.yaml with:

@@ -64,9 +64,11 @@ tetragon:
   securityContext:
     privileged: true
   # -- Overrides the default livenessProbe for the tetragon container.
-  livenessProbe: {}
-  #  grpc:
-  #    port: 54321
+  livenessProbe:
+    timeoutSeconds: 60
+    httpGet:
+      path: "/liveness"
+      port: 6789

   # Tetragon puts processes in an LRU cache. The cache is used to find ancestors
   # for subsequently exec'ed processes.

Then I run:

$ cat values.yaml
tetragon:
  livenessProbe: {}
$ helm template ./install/kubernetes/tetragon -f ./values.yaml
[...]
        livenessProbe:
          httpGet:
            path: /liveness
            port: 6789
          timeoutSeconds: 60
[...]

Which seems to be the same.

[tpapagian]

ok, I think I found a way to do that. Updating the PR now.

[tpapagian]

We should define

$ cat values.yaml
tetragon:
  livenessProbe: null

instead of

$ cat values.yaml
tetragon:
  livenessProbe: {}

to use the old tetra-based liveness probe.

PR is ready for review.

[michi-covalent]

i have my opinion about this. i'll add my review here tomorrow 🚀 🙏

[lambdanis]

If the gRPC server is disabled then this new healthcheck will always fail, right? I think it shouldn't.

I don't have context on why the GetHealth endpoint was introduced, but it looks a bit awkward to me. gRPC itself defines the health checking service, so I would use the upstream proto and start the health service alongside the regular Tetragon gRPC server.

Also, Kubernetes now supports gRPC liveness probes, so unless we want to support old Kubernetes versions, we can use them instead of starting another HTTP server.

[tpapagian]

If the gRPC server is disabled then this new healthcheck will always fail, right? I think it shouldn't.

Not sure why with gRPC server disabled the new check will fail. How is that related?

I don't have context on why the GetHealth endpoint was introduced, but it looks a bit awkward to me. gRPC itself defines the health checking service, so I would use the upstream proto and start the health service alongside the regular Tetragon gRPC server.

Also, Kubernetes now supports gRPC liveness probes, so unless we want to support old Kubernetes versions, we can use them instead of starting another HTTP server.

I also don't have context about the details of GetHealth function that we use. But I believe that this is not directly related to this PR. The gRPC liveness probe in k8s is stable in 1.27. The purpose of this PR is to remove the need to have a liveness probe that is related to tetra status and use http-based liveness probe that also supports older k8s versions. We can also implement and use grpc-based liveness probe on systems that supports that, but this can be also done in a followup PR.

[lambdanis]

Not sure why with gRPC server disabled the new check will fail. How is that related?

Ok, I actually checked GetHealth implementation - it always says it's fine. So you're right, it won't fail, it's just a confusing implementation. I think it's fine to follow-up with the new health checks, just wanted to check I'm not missing something.

[tpapagian]

Closing in favor of #2478 as we choose to proceed with a gRPC-based liveness probe.