Add missing probes

[skonto]

Changes

• Adds missing probes for net-kourier-controller and the gateway,

[codecov]

Codecov Report

Merging #1029 (a5fa3a1) into main (a6297c1) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1029   +/-   ##
=======================================
  Coverage   80.78%   80.78%           
=======================================
  Files          18       18           
  Lines        1343     1343           
=======================================
  Hits         1085     1085           
  Misses        205      205           
  Partials       53       53           

Impacted Files	Coverage Δ
pkg/generator/status_vhost.go	100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[skonto]

/retest

[ReToCode]

/lgtm
/hold for @nak3 to review.

I'd assume the same comments apply like in knative-extensions/net-istio#1095 (review)?

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ReToCode, skonto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [skonto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[nak3]

Should we use /probe, /kprobe, /health or /khealth or something else for both readiness and liveness?
Using the root path should be fine as long as the header matching is used but I am not 100% confident if it is no side effect at all. (e.g. the total number of access to / cannot be counted anymore.)

[skonto]

Initially I had it like that. There is no other path to use for liveness other than ready. Right now this is using the one that envoy provide /ready.

[skonto]

@nak3 I update the path. Now both readiness and liveness probes use the /ready path at the admin interface.
I dont think there is anything else to use there (it is the same path to check envoy liveness that you get at the admin ui).

I tried to use a different path like "/health" by doing:

• add the subpath to the regex in the boostrap.yaml
• create a new route with:

eroute.Action = &route.Route_Redirect{
  Redirect: &route.RedirectAction{
    PathRewriteSpecifier: &route.RedirectAction_PathRedirect{
      PathRedirect: "/ready",
    },
  },
}

and although K8s allows redirects: https://github.com/kubernetes/kubernetes/blob/master/pkg/probe/http/http.go#L113
I get a warning from that line:

$ k get events -n kourier-system | grep warning
4m46s       Warning   ProbeWarning                   pod/3scale-kourier-gateway-6c9ddb7b68-kgjhd      Liveness probe warning: Probe terminated redirects, Response body:

So although it works due to the warning I decided not to go that path because users may not be aware of what codes K8s allows for probes.

I have also fixed the settings for the readiness, liveness so they are not the same to avoid the known issues.

[nak3]

Thank you. My intention was not used the same path (/) with applications so /ready should be fine. Let's wait for serving's PR if there is any additional feedback if we can apply.

[dprotaso]

FYI Serving's PR merged

[nak3]

/lgtm
/hold cancel

Thank you!