Skip to content

define readinessProbe on statefulSet #1825

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

Merged
merged 9 commits into from
Mar 30, 2022
Merged

define readinessProbe on statefulSet #1825

merged 9 commits into from
Mar 30, 2022

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Mar 28, 2022

With #1760 merged, we should add a readinessProbe to the statefultSet because the ConfigMaps option defines a selector on the master service. Label selectors incl. readiness probes. We will add it either way.

#1760 also introduced a new suffix for deleting Patroni cluster objects which will result in an error for the new "leader" suffix if endpoints are used. Instead, the operator should just l log a warning and try to delete as much as it can. Another bug was introduced by not deleting all services if endpoints are used.

Other changes:

  • user variables when specifying container ports
  • check configmaps option when syncing ClusterStatus not omit endpoints
  • remove endpoints from RBAC (provide RBAC manifest for OpenShift)

@FxKu FxKu added this to the 1.8 milestone Mar 28, 2022
@FxKu FxKu added the zalando label Mar 28, 2022
@FxKu FxKu changed the title define readinessProbe on statefulSet when Patroni uses ConfigMaps define readinessProbe on statefulSet Mar 28, 2022
@jopadi
Copy link
Member

jopadi commented Mar 30, 2022

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Mar 30, 2022

👍

@FxKu FxKu merged commit 60e0685 into master Mar 30, 2022
Handler: v1.Handler{
HTTPGet: &v1.HTTPGetAction{
Path: "/readiness",
Port: intstr.IntOrString{IntVal: patroni.ApiPort},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the commit!
I have a question: are you sure of the port? Container is never ready with this probe, HTTP 503. While if we try the same path with port 8080, we get HTTP 200.

root@acid-grafana-postgres-0:/home/postgres# curl -vvvv $POD_IP:8008/readiness
*   Trying 10.0.2.66...
* TCP_NODELAY set
* Connected to 10.0.2.66 (10.0.2.66) port 8008 (#0)
> GET /readiness HTTP/1.1
> Host: 10.0.2.66:8008
> User-Agent: curl/7.58.0
> Accept: */*
> 
* HTTP 1.0, assume close after body
< HTTP/1.0 503 Service Unavailable
< Server: BaseHTTP/0.6 Python/3.6.7
< Date: Tue, 31 May 2022 15:43:49 GMT
< Content-Type: application/json
< 
* Closing connection 0
<json>

vs port 8080:

root@acid-grafana-postgres-0:/home/postgres# curl -vvvv $POD_IP:8080/readiness
*   Trying 10.0.2.66...
* TCP_NODELAY set
* Connected to 10.0.2.66 (10.0.2.66) port 8080 (#0)
> GET /readiness HTTP/1.1
> Host: 10.0.2.66:8080
> User-Agent: curl/7.58.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Tue, 31 May 2022 15:43:44 GMT
< Content-Length: 3375
<json>

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants