kbuild: make network readiness checks bounded (#3197) - #3202
Conversation
Signed-off-by: aniket1260 <anket1260@gmail.com>
| for _ in range(10): | ||
| try: | ||
| socket.gethostbyname('www.google.com') | ||
| socket.gethostbyname(hostname) |
There was a problem hiding this comment.
socket.gethostbyname() has no deadline, so the ten-iteration loop cannot interrupt a stuck DNS lookup
| while retries > 0: | ||
| try: | ||
| r = requests.get("https://google.com") | ||
| r = requests.get(target_url, timeout=timeout) |
There was a problem hiding this comment.
timeout=10 is an inactivity timeout, not a total wall-clock deadline
| """Verify network connectivity""" | ||
| # TBD: Different URL? pool of urls? | ||
| retries = 10 | ||
| def _verify_network(self, url=None, max_retries=5, retry_delay=5, timeout=10): |
There was a problem hiding this comment.
_verify_network() still catches every Exception; malformed URLs and programming/configuration errors become misleading network-readiness failures
| def is_dtb_artifact(artifact): | ||
| return artifact.startswith("dtbs/") and artifact.endswith(".dtb") | ||
| posix_art = artifact.replace("\\", "/") | ||
| return posix_art.startswith("dtbs/") and posix_art.endswith(".dtb") |
There was a problem hiding this comment.
The DTB path-normalization changes are unrelated to #3197 and are not explained in the PR description. Please remove them from this PR or submit them separately with their motivation and tests.
Their inclusion also raises concern that the complete patch may not have been carefully reviewed before submission. KernelCI’s contribution policy requires contributors to understand and take responsibility for all submitted changes, and to disclose meaningful AI assistance with an Assisted-by tag. Please review the full diff and disclose any applicable assistance.
|
@nuclearcat before submitting my next commit, I'll observe the changes more carefully. Thanks for your valuable insights |
2 participants
This PR addresses #3197 by making Kubernetes network readiness checks bounded and independent of unrelated third-party services.
Problem
KernelCI had two generic startup checks that used Google connectivity as an indication that the Kubernetes node network was ready.
KBuild._verify_network() performed an HTTP request to google.com without a request timeout. Additionally, non-200 HTTP responses did not decrement the retry counter, which could cause the readiness check to loop indefinitely.
The generic Python runtime template also attempted to resolve www.google.com before starting a job. Besides introducing an unnecessary dependency on an external service, this does not verify connectivity to the KernelCI services that the job actually needs.
Solution
This PR updates the network readiness handling to use the KernelCI API as the readiness target.
For KBuild._verify_network():
Added a configurable target URL.
Use the configured KernelCI API URL when available, with https://api.kernelci.org as the fallback.
Added configurable retry count, retry delay, and request timeout.
Pass the timeout directly to requests.get().
Decrement the retry counter for every failed attempt, including non-200 HTTP responses.
Avoid sleeping after the final attempt.
Report the target service and elapsed time when readiness checks are exhausted.
Raise a RuntimeError instead of terminating the process directly with sys.exit().
For the generic Python runtime template:
Removed the hard-coded dependency on www.google.com.
Resolve the hostname of the configured KernelCI API instead.
Fall back to api.kernelci.org when the API URL cannot be determined.
Bound the DNS readiness check to a fixed number of attempts.
Report the actual hostname when DNS resolution fails.
Validation
The readiness logic was tested with mocked network conditions, including:
Immediate HTTP 200 success.
Repeated non-200 responses.
Temporary failures followed by a successful response.
Connection failures.
Request timeouts.
Retry exhaustion.
Closes #3197