serving icon indicating copy to clipboard operation
serving copied to clipboard

Add configurable probe timeout and frequency for activator

Open bindrad opened this issue 1 month ago • 10 comments

Fixes #16249

Proposed Changes

  • Add PROBE_TIMEOUT environment variable to configure activator health check timeout (default: 300ms)
  • Add PROBE_FREQUENCY environment variable to configure activator probe frequency (default: 200ms)

Release Note

Activator probe timeout and frequency are now configurable via PROBE_TIMEOUT and PROBE_FREQUENCY environment variables.

bindrad avatar Nov 20 '25 10:11 bindrad

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: bindrad / name: Dhruv Bindra (45e141bae50885fa71a0ca48300716a19bd8c891, 51b3c60685a8c505d87e9bdf167871b75e10217f)

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bindrad Once this PR has been reviewed and has the lgtm label, please assign skonto for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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

knative-prow[bot] avatar Nov 20 '25 10:11 knative-prow[bot]

Hi @bindrad. Thanks for your PR.

I'm waiting for a github.com 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.

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.

knative-prow[bot] avatar Nov 20 '25 10:11 knative-prow[bot]

/ok-to-test

linkvt avatar Nov 21 '25 09:11 linkvt

/test unit-tests

bindrad avatar Nov 22 '25 02:11 bindrad

I'm not sure if istio-latest-no-mesh-tls_serving_main is failing due to my changes. Please suggest.

bindrad avatar Nov 22 '25 03:11 bindrad

It's likely a flaky test indicated by a webhook deployment not being ready in the E2E test setup. Let's do a simple retest.

/retest

linkvt avatar Nov 24 '25 07:11 linkvt

@bindrad Seems like the flaky test finished without errors.

I just reviewed the code and think it might make sense to only set/overwrite these values through the config-network ConfigMap instead of allowing an overwrite via environment variables. This would seem more in line with how we configure other parameters, I guess the two fields above (MaxIdleProxyConns, ...) are rather outliers. Another idea would be a new ConfigMap config-activator, this is IMO too much overhead so I wouldn't do it.

I'm not a long-term contributor/approver though so someone else might have a better idea.

linkvt avatar Nov 24 '25 13:11 linkvt

@linkvt Thanks for the feedback. I thought to follow the same pattern as MaxIdleProxyConns and MaxIdleProxyConnsPerHost, which are also set via environment variables. I'm fine with either approach, but I think it would be useful to make these values configurable in some form.

bindrad avatar Nov 25 '25 03:11 bindrad

Codecov Report

:x: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 80.10%. Comparing base (a94c607) to head (45e141b). :warning: Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
pkg/activator/net/throttler.go 0.00% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16250      +/-   ##
==========================================
+ Coverage   80.02%   80.10%   +0.07%     
==========================================
  Files         215      215              
  Lines       13327    13328       +1     
==========================================
+ Hits        10665    10676      +11     
+ Misses       2299     2293       -6     
+ Partials      363      359       -4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Dec 04 '25 02:12 codecov[bot]