Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Panic in liveness endpoint #1065

Closed
vikstrous2 opened this issue Jan 9, 2022 · 2 comments · Fixed by #1068
Closed

Panic in liveness endpoint #1065

vikstrous2 opened this issue Jan 9, 2022 · 2 comments · Fixed by #1068
Assignees
@enocom
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@vikstrous2
Copy link

Bug Description

When this google cloud incident started https://status.cloud.google.com/incidents/NMcnk6aE8xMHHwRGmyry, our cloudsql proxy started being killed by liveness probes. They are configured according to this recommendation https://github.com/GoogleCloudPlatform/cloudsql-proxy/tree/main/examples/k8s-health-check#cloud-sql-proxy-health-checks. That's fine and expected, but the error message was very confusing:

[Health Check] Liveness failed: %!v(PANIC=Error method: runtime error: invalid memory address or nil pointer dereference)

There seems to be a panic somewhere in cloudsql proxy, but no more information was printed.

Environment

  1. OS type and version: GKE
  2. Cloud SQL Proxy version (./cloud_sql_proxy -version): gcr.io/cloudsql-docker/gce-proxy:1.27.0-buster@sha256:5d3d9bb25542b5f786cdb983d052b5f9145c9abd0b7128688f545ae9ce060fca
@vikstrous2 vikstrous2 added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Jan 9, 2022
@enocom enocom added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Jan 10, 2022
@enocom
Copy link
Member

enocom commented Jan 10, 2022

Thanks for the report @vikstrous2. I'll take a closer look and report back here.

@enocom enocom added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Jan 10, 2022
@enocom
Copy link
Member

enocom commented Jan 10, 2022

@vikstrous2 out of curiosity, what logging do you see immediately before this line?

enocom added a commit that referenced this issue Jan 10, 2022
@enocom
fix: invalidated config should retain error
f7bc077 
When a TLS handshake fails, the client would invalidate the
configuration without recording the associated error. When the liveness
check runs, it would panic when trying to print the invalidated
configuration's error because the error was nil. This commit ensures
that when the proxy invalidates a configuration, it includes the error.

Fixes #1065.
enocom added a commit that referenced this issue Jan 10, 2022
@enocom
fix: invalidated config should retain error
3904508 
When a TLS handshake fails, the client would invalidate the
configuration without recording the associated error. When the liveness
check runs, it would panic when trying to print the invalidated
configuration's error because the error was nil. This commit ensures
that when the proxy invalidates a configuration, it includes the error.

Fixes #1065.
@enocom enocom mentioned this issue Jan 10, 2022
Merged
enocom added a commit that referenced this issue Jan 13, 2022
@enocom
fix: invalidated config should retain error
087a59f 
When a TLS handshake fails, the client would invalidate the
configuration without recording the associated error. When the liveness
check runs, it would panic when trying to print the invalidated
configuration's error because the error was nil. This commit ensures
that when the proxy invalidates a configuration, it includes the error.

Fixes #1065.
enocom added a commit that referenced this issue Jan 20, 2022
@enocom
fix: invalidated config should retain error (#1068)
49d3003 
When a TLS handshake fails, the client would invalidate the
configuration without recording the associated error. When the liveness
check runs, it would panic when trying to print the invalidated
configuration's error because the error was nil. This commit ensures
that when the proxy invalidates a configuration, it includes the error.

Fixes #1065.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@enocom enocom

Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: invalidated config should retain error GoogleCloudPlatform/cloud-sql-proxy
2 participants
@enocom @vikstrous2