Support exec readiness probes for sidecar containers

[flomedja]

Fixes #15484

Proposed Changes

Delegate the exec readiness probes to kubernetes for sidecar containers. The controller previsouly return an error when a exec readiness probes is defined on a sidecar container. We now support the exec readiness probes for all the containers.

Release Note

Support exec readiness probes for sidecar containers

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: flomedja / name: Florian MEDJA (c14663c, 16cb6b2, a2f121f, 7a16d95, 4209bac)

[knative-prow]

Welcome @flomedja! It looks like this is your first PR to knative/serving 🎉

[knative-prow]

Hi @flomedja. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[dprotaso]

/ok-to-test

[codecov]

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 80.91%. Comparing base (e193904) to head (16cb6b2).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/activator/net/revision_backends.go	40.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #15773   +/-   ##
=======================================
  Coverage   80.90%   80.91%           
=======================================
  Files         210      210           
  Lines       16698    16706    +8     
=======================================
+ Hits        13510    13518    +8     
  Misses       2838     2838           
  Partials      350      350           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[flomedja]

@dprotaso @skonto A e2e test failed for v1.30.x but succeeded for v1.31.x. I don't see an option to rerun the test. How can I rerun it ?

Also I see a MR removing the support for v.1.30.x, was there some issues with this version ?

[skonto]

To re-test you can do:

[skonto]

The failure was due to kind not being setup correctly. I will re-run.

[flomedja]

@skonto I don't see the re-run button on my side. I suspect that you need to be a special group member to be able to re-run tests. Thanks for the help.

@dprotaso All the tests passed. You can take a look to the implementation when you have a moment :). Thanks

[flomedja]

@skonto @dprotaso Just a reminder for the MR :)

[flomedja]

@skonto @dprotaso Just a reminder for the review :)

[dprotaso]

thanks for the change @flomedja looks good.

We'll need to also disable some probe optimizations in the activator when the sidecar and an exec probe

serving/pkg/activator/net/revision_backends.go

Lines 536 to 544 in 3d830a0

	enableProbeOptimisation := true
	if rp := rev.Spec.GetContainer().ReadinessProbe; rp != nil && rp.Exec != nil {
	enableProbeOptimisation = false
	}
	// Startup probes are executed by Kubelet, so we can only mark the container as ready
	// once K8s sees it as ready.
	if sp := rev.Spec.GetContainer().StartupProbe; sp != nil {
	enableProbeOptimisation = false
	}

[flomedja]

@dprotaso Thanks for the comment. I added the changes to disabel the probe optimization when a sidecar container have a exec probe.

[dprotaso]

/lgtm
/approve

thanks @flomedja

[flomedja]

@dprotaso My pleasure! I'd be happy to contribute to other issues as well, just let me know where I can help!

[dprotaso]

If you see any issues you want to work on just message me in the CNCF slack (https://communityinviter.com/apps/cloud-native/cncf) and I can walk through stuff

[dprotaso]

/lgtm
/approve

[dprotaso]

/hold cancel

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, flomedja

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [dprotaso]

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