feat: add /status/ready function #454
Conversation
|
|
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #454 +/- ##
==========================================
- Coverage 59.61% 59.58% -0.04%
==========================================
Files 71 71
Lines 4408 4449 +41
==========================================
+ Hits 2628 2651 +23
- Misses 1166 1180 +14
- Partials 614 618 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
tao12345666333
force-pushed
the
feat-add-status-ready
branch
3 times, most recently
from
c0ba631
to
a8b4e6e
Compare
Signed-off-by: Jintao Zhang <[email protected]>
tao12345666333
force-pushed
the
feat-add-status-ready
branch
from
a8b4e6e
to
f154725
Compare
| } | ||
|
|
||
| func parseStatusListen(listen string) string { | ||
| re := regexp.MustCompile(`^([\w\.:]+)\s*(.*)?`) |
There was a problem hiding this comment.
Can we extract the regex to not compile it on every call?
| address := matches[1] | ||
| extraParams := matches[2] |
There was a problem hiding this comment.
What if matches is 1 or 2 entries long? This will blow up then. Can we check for that as well here?
| @@ -391,6 +455,21 @@ func (c *Client) Status(ctx context.Context) (*Status, error) { | |||
| return &s, nil | |||
| } | |||
|
|
|||
| // Ready returns 200 only after the Kong node has configured itself and is ready to start proxying traffic. | |||
There was a problem hiding this comment.
Can we link to the official docs here? Specifically to https://docs.konghq.com/gateway/latest/production/monitoring/healthcheck-probes/#types-of-health-checks
I'd consider mentioning that this function uses Kong's "Readiness" endpoint.
|
|
||
| sm, err := client.Ready(defaultCtx) | ||
| if err != nil { | ||
| // for dbless mode, the ready endpoint returns 503 |
There was a problem hiding this comment.
WDYT about moving this message to assertion so that it's logged when the test fails?
| assert.Equal("HTTP status 503 (message: \"no configuration available (empty configuration present)\")", err.Error()) | ||
| assert.Nil(sm) | ||
| } else { | ||
| // for db-mode, the ready endpoint returns 200 |
There was a problem hiding this comment.
WDYT about moving this message to assertion so that it's logged when the test fails?
| assert.NotNil(client) | ||
|
|
||
| sm, err := client.Ready(defaultCtx) | ||
| if err != nil { |
There was a problem hiding this comment.
This can produce false positives when the err != nil and we use db-mode. Can we use a different method here to inspect which flavor of Kong we're running against? e.g. inspecting the configuration?
Co-authored-by: Patryk Małek <[email protected]>
ref: Kong/kubernetes-ingress-controller#5068
https://docs.konghq.com/gateway/api/status/latest/#/default/get_status_ready