haxsaw / hikaru

Move smoothly between Kubernetes YAML and Python for creating/updating/componentizing configurations.
MIT License
204 stars 18 forks source link

Missing fields in Pod.spec.containers[*].livenessProbe #39

Closed aantn closed 5 months ago

aantn commented 5 months ago

Hikaru doesn't recognize the field Pod.spec.containers[*].livenessProbe.exec.

How to reproduce:

$ kubectl apply -f https://raw.githubusercontent.com/robusta-dev/kubernetes-demos/main/liveness_probe_fail/failing_liveness_probe.yaml

$ python3.9
>>> from kubernetes import client, config
>>> import hikaru
>>> from hikaru import *
>>> from hikaru.model.rel_1_25 import *
>>> config.load_kube_config()
>>> p = Pod().read(name='order-processor', namespace='default')
>>> print(p.spec.containers[0].livenessProbe)
Probe(exec=None, failureThreshold=1000, grpc=None, httpGet=None, initialDelaySeconds=5, periodSeconds=5, successThreshold=1, tcpSocket=None, terminationGracePeriodSeconds=None, timeoutSeconds=1)

As you can see, the exec field is missing from the liveness probe in Hikaru.

Here is the complete code to reproduce for convenience:

from kubernetes import client, config
import hikaru
from hikaru import *
from hikaru.model.rel_1_25 import *

config.load_kube_config()

p = Pod().read(name='order-processor', namespace='default')
print(p.spec.containers[0].livenessProbe)
haxsaw commented 5 months ago

So the obvious things seem ok at first glance: the objects for this path are all defined properly, and certainly when you call Probe.get_empty_instance(), Probe knows of the optional exec attribute and ExecAction (the type of exec) knows it contains a list of strings called 'command', so the models are correct. There isn't any special processing for this attribute in either the build or hikaru itself, so an error in special processing seems unlikely. Do you have some specimen YAML for a Pod with a livenessProbe defined that doesn't come back that you can share? It would help narrow things down. At this point, I'd be wanting to verify that K8s was actually returning the strings for this value; what version of K8s is running in this test? Have you checked it with any other versions? I'm going to put together a Pod with some dumb test like '/bin/sleep 1' to see if I can get the Pod read back, but any other observations or examples would help.

aantn commented 5 months ago

Try this YAML to reproduce: https://raw.githubusercontent.com/robusta-dev/kubernetes-demos/main/liveness_probe_fail/failing_liveness_probe.yaml

Kubernetes version is 1.25:

$ kubectl version --short
Flag --short has been deprecated, and will be removed in the future. The --short output will become the default.
Client Version: v1.26.13-dispatcher
Kustomize Version: v4.5.7
Server Version: v1.25.16-gke.1460000
haxsaw commented 5 months ago

I found it; it turns out that the field is coming out of K8s as "_exec", not "exec", and hence we don't find it since 'exec' is optional and we never find that. I think there's some provision in the code for this kind of thing, but I'm going to have to dig around a bit later one as I haven't touched that bit in a while.

aantn commented 5 months ago

Makes sense, thanks.

On Tue, Apr 2, 2024 at 12:05 PM Tom Carroll @.***> wrote:

I found it; it turns out that the field is coming out of K8s as "_exec", not "exec", and hence we don't find it since 'exec' is optional and we never find that. I think there's some provision in the code for this kind of thing, but I'm going to have to dig around a bit later one as I haven't touched that bit in a while.

— Reply to this email directly, view it on GitHub https://github.com/haxsaw/hikaru/issues/39#issuecomment-2031460712, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYUBZ56GFYCGGWFCWSNI3Y3JYG3AVCNFSM6AAAAABFMGJTC2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZRGQ3DANZRGI . You are receiving this because you authored the thread.Message ID: @.***>

haxsaw commented 5 months ago

OK, some news and some questions.

It seems that although the OpenAPI spec names a field in the Probe object 'exec', it actually comes back from the underlying K8s libraries as '_exec'. It isn't clear whether it is actually coming out of the cluster this way, or it just gets translated to this value in the underlying K8s libraries. Regardless, I have a facility for these kind of odd-man-out fields (there is at least one other) and so it's a pretty straightforward change. In the process of identifying the root cause and the fix, a small performance enhancement has made it self obvious so that will be part of the fix.

However, I have a vague impression that there used to be some other special code to handle this exact case and I deleted it without knowing the impact. This is what leads me to the questions I have:

  1. Did this previously work for you?
  2. If it did, what was the last version of Hikaru you used where this worked as you expected?

I'm going to put in the fix that I've identified as it's a better solution (especially if I really had code like I'm remembering). But I would like to track down the older code where this may have lived before just so I can see what was going on, and it would be helpful to narrow the search down to a version where it may have worked before.

This isn't critical, but it would be helpful to me as I'd like to get some idea where/when this stopped working right. In the meantime I'll get a patch release going.

aantn commented 5 months ago

We only noticed this now, so its hard for me to say if it worked in the past or not. We discovered it when looking at something new related to the exec field that we had never looked at before. Sorry I can't be more useful!

haxsaw commented 5 months ago

OK, so this has been fixed and is available now in hikaru-core v1.1.2.

aantn commented 5 months ago

Wonderful, thanks for the update.