-
Notifications
You must be signed in to change notification settings - Fork 545
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
Understand the semantics of ssh client in case the command fails #1343
Comments
@wojtek-t I did some more experiments on this and also posted a question on #kubernetes-dev slack channel. the response i got is more or less in line with what I had found previously:
1 suggestion was to use -f option with curl. The man page mentions the following:
I tested by throwing 500 error and accessing an invalid url in a web server running in a pod. on both occasions the error code returned is 22. I believe this is better than checking stdout and stderr as we might see differing behaviour. And hopefully, response codes 401 and 407 shouldn't be something that we should be worrying about. |
Thanks for digging into it - it's really useful. |
Closing this issue. |
While working on : #1096 , the code tries to access the /healthz end point SSHing into the a master node:
perf-tests/clusterloader2/pkg/measurement/common/api_availability_measurement.go
Line 62 in d26909c
The semantics that we need to understand is what happens if the command fails internally with error 500 for the Curl Request. In that case, what corrective actions need to be taken?
Currently, only err and sshResult.Code is checked:
https://github.com/kubernetes/perf-tests/blob/master/clusterloader2/pkg/measurement/common/scheduler_latency.go#L317-L322
However, in some of my local testing, it was found that when the Curl request for /healthz end point fails, then Stderr in SshResult would be populated. In case of a proper response, Stderr would be empty and Stdout would be populated. The findings have been noted here:
#1162 (comment)
The intention is to find the semantics to verify this behaviour and document it accordingly.
The text was updated successfully, but these errors were encountered: