-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add curl timeout to the rgw-probe.sh healtchcheck script #14143
Comments
I'll take a look at this in the coming days. I want to be sure that the best solution is indeed the curl |
@kayrus what do you mean by "accumulate" here?
Can you provide logs that show what you mean? |
there are no logs, there are just dozens of |
How are you determining that to be true? I would like to see some kind of output of proof that this is happening. My understanding of the kubernetes probes is that when they time out, kubernetes kills the process (bash), which should also kill the curl process in the script. |
The dilemma I face is thus:
Because of the risk of introducing errors by making the proposed change, I want to make sure we are certain that there is an issue present. Rook has gotten the pod probes wrong in the past, and it has taken us a lot of effort to develop probes that are useful and bug-free, and to make sure that we are not implementing probes or probe features that risk causing cascading system failures. We have also had Rook users in the past submit issues based on theory rather than on observed behavior. Since you are not providing evidence via observed behavior, I am still concerned that the reasoning behind the creation of this issue could be theoretical and not evidence-based. When users have reported theoretical issues in the past, it has led to us introducing unnecessary complexity into Rook, and it has sometimes caused bugs. I did some in-depth investigation today, and I have findings to share. FindingsResearchThis is the information I can find on the topic of probe "accumulation" (from June 2022): In summary, cri-o and containerd runtimes will terminate probe processes if they pass the timeout threshold, preventing process "accumulation". The dockershim runtime does not have this ability, but since dockershim is not a supported K8s configuration, k8s will not fix the behavior. Because dockershim was already an unsupported runtime 2 years ago, I don't see any compelling reason that we should consider this a Rook bug in 2024. TestingTo confirm that there isn't a bash issue present, I looked into how the script behaves in isolation when it receives a SIGTERM signal. When the SIGTERM signal is received while ConclusionBecause this involves liveness probes, I am taking this issue seriously. That also means that I am doing my own due diligence to be certain that Rook is doing the best thing for all its users. And because my research and testing didn't find anything wrong, it means that I must put the burden of proof on you to show that the issue is real. For us to move forward with the bug fix, please show us via some sort of console output that the process accumulation is definitely happening in your environment and that the container runtime is something other than dockershim. |
@BlaineEXE thanks a lot for your extra research. I'll try to provide more evidences next week. |
Hi @kayrus I wanted to check in to see how things are progressing on your side. I'm also happy to try to see if I can repro, if you can help me understand what you mean by "network hiccup." I'm not sure what would cause networking between a probe and pod to stall, but I might be able to simulate it if I understood more about the situation you're describing. |
@BlaineEXE apologies, I was having other more important issues to fix, like keystone auth, etc. Regarding the hanged |
I reproduced an issue, and here is some additional info. By default kubelet sets the readiness check timeout based on the When the context timeout is reached, the running
Pod config to reproduce the issue: apiVersion: v1
kind: Pod
metadata:
name: sleep-pod
spec:
nodeSelector:
kubernetes.io/hostname: some-node
containers:
- name: sleeper
image: nginx:latest
args:
- sleep
- "3600" # Sleep for 1 hour
readinessProbe:
exec:
command:
- /bin/sh
- -c
- curl 8.8.8.8
initialDelaySeconds: 0
periodSeconds: 1
timeoutSeconds: 1
successThreshold: 1
failureThreshold: 1 kubelet config:
P.S. Pay attention that it's impossible to set the |
Fascinating and frustrating. It looks like only the I did some reading and found some people mentioning that I'd like to make sure that we also take a look at Rook's other |
Interestingly, I can't reproduce this exactly using your sample pod on minikube with containerd runtime. Runtime request timeout is a crazy 15 minutes in the minikube config. I'm seeing I doubt this has anything to do with processor architecture (I'm on arm64). However, also interesting, is that those defunct processes don't go away when I use
|
I'm using flatcar amd64 with containerd. In my case if the |
I am noticing with my local testing that |
Sorry to spam here, but I have some new info. I am trying to test using this probe, which is conceptually most similar to how our probes are configured. readinessProbe:
exec:
command:
- bash
- -c
- |
out="$(curl 8.8.8.8)"
echo "$out" I am not seeing a bunch of zombies in my case, but I am seeing that the I believe the underlying issue here is that k8s doesn't do process reaping. It does properly send the kill signal to In your case, it seems that k8s is starting a new probe process even when the old children are still around. I can't explain the discrepancy in behavior, but in both cases, the behavior is very non-ideal. This issue can be trivially solved by setting a curl timeout, but the more generalized issue seems to be a compound one:
In theory, one of the trivial things that Rook could do is enable container pid sharing in pods. However, in my testing, the curl process isn't getting reaped even with Given this, I think it still makes sense to continue with your PR since it can fix the issue for curl, and I'd like to keep looking to see if there is something Rook can do more broadly to not have this situation come up for other probes. |
@kayrus I have been researching and playing around with your example, because I really want to find a generalized way to resolve this for the rest of our Rook probes so we don't have more issues like this elsewhere. I didn't have any luck trying to use However, I have found that this version (below) doesn't result in zombie curl commands in my env. apiVersion: v1
kind: Pod
metadata:
name: sleep-pod
spec:
nodeSelector:
kubernetes.io/hostname: minikube
# shareProcessNamespace: true
containers:
- name: sleeper
image: quay.io/ceph/ceph:v18
args:
- sleep
- "3600" # Sleep for 1 hour
readinessProbe:
exec:
command:
- bash
- -c
- |
set -e
out="$(curl --max-time 30 8.8.8.8)"
echo "$out"
initialDelaySeconds: 0
periodSeconds: 10
timeoutSeconds: 5
successThreshold: 1
failureThreshold: 1
resources: {} Would you mind testing this to see if it also resolves things in your env? It seems much easier to use And, for what it's worth, I see the same thing if I replace the curl command with something like |
Is this a bug report or feature request?
Deviation from expected behavior:
When there is a network hiccup, the bash scripts with a curl probe start to accumulate. By default curl has a high connect timeout, e.g.
And the probes are:
Expected behavior:
Curl probe should have about ~3 sec connect timeout. E.g. use
--connect-timeout 3
or--max-time 3
.How to reproduce it (minimal and precise):
n/a
File(s) to submit:
cluster.yaml
, if necessaryLogs to submit:
Operator's logs, if necessary
Crashing pod(s) logs, if necessary
To get logs, use
kubectl -n <namespace> logs <pod name>
When pasting logs, always surround them with backticks or use the
insert code
button from the Github UI.Read GitHub documentation if you need help.
Cluster Status to submit:
Output of kubectl commands, if necessary
To get the health of the cluster, use
kubectl rook-ceph health
To get the status of the cluster, use
kubectl rook-ceph ceph status
For more details, see the Rook kubectl Plugin
Environment:
uname -a
):rook version
inside of a Rook Pod):ceph -v
):kubectl version
):ceph health
in the Rook Ceph toolbox):The text was updated successfully, but these errors were encountered: