feat(ISV-7560): internal-request uses k8s instead of kubectl - #1016
feat(ISV-7560): internal-request uses k8s instead of kubectl#1016mavaras wants to merge 1 commit into
Conversation
mavaras
commented
Sep 8, 2026
mavaras
commented
- internal-request uses python library for k8s to replace the use of kubectl
- kubernetes dependency added to pyproject
- tests updated accordingly
|
@kosciCZ please take a look here |
PR Summary by QodoUse Kubernetes client for InternalRequest operations
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Retries can overlap old pipelines
|
![qodo-app-for-konflux-ci[bot]](https://avatars.githubusercontent.com/u/155583288?s=60&v=4){kind=small}
| @pytest.fixture() | ||
| def k8s_api(): |
There was a problem hiding this comment.
1. Maintainers lose fixture type checks 📘 Rule violation ⚙ Maintainability
The new k8s_api fixture declares no return type annotation. Type checking therefore cannot validate the yielded mock client or detect incompatible fixture changes reaching every modified test that consumes it.
Agent Prompt
## Issue description
The new `k8s_api` fixture is missing an explicit return type annotation.
## Issue Context
The fixture yields a `MagicMock`, and the compliance checklist requires annotations for all parameters and return types in newly added functions.
## Fix Focus Areas
- src/helpers/internal_request/tests/test_internal_request.py[22-27]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if k8s_api is None: | ||
| k8s_api = _default_k8s_api() |
There was a problem hiding this comment.
2. Invalid requests report setup errors 🐞 Bug ≡ Correctness
create() constructs the default Kubernetes client before validating pipeline and the required Git resolver parameters. When a caller supplies invalid arguments outside a configured cluster environment, configuration loading fails before the intended ValueError is raised.
Agent Prompt
## Issue description
Argument validation should run before Kubernetes configuration is loaded so invalid calls consistently raise the documented validation errors without requiring cluster credentials or a kubeconfig.
## Issue Context
Move default-client construction until after validation of `pipeline`, required Git parameters, and timeout values. Retain the injected-client behavior used by callers and tests.
## Fix Focus Areas
- src/helpers/internal_request/internal_request.py[537-560]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if k8s_api is None: | ||
| k8s_api = _default_k8s_api() |
There was a problem hiding this comment.
3. Invalid waits report setup errors 🐞 Bug ≡ Correctness
wait_for_completion() initializes the default Kubernetes client before checking that exactly one selector was provided. Calls with neither or both selectors now require usable Kubernetes configuration before they can receive the function's intended ValueError.
Agent Prompt
## Issue description
Selector validation should run before Kubernetes configuration is loaded so invalid wait calls consistently raise the declared `ValueError` without requiring cluster credentials or a kubeconfig.
## Issue Context
Perform the exactly-one-selector check first, then create the default client only for valid polling requests. Preserve support for an explicitly injected client.
## Fix Focus Areas
- src/helpers/internal_request/internal_request.py[339-346]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| k8s_api.delete_namespaced_custom_object( | ||
| group=_IR_GROUP, | ||
| version=_IR_VERSION, | ||
| namespace=namespace, |
There was a problem hiding this comment.
4. Retries can overlap old pipelines 🔗 Cross-repo conflict ☼ Reliability
cleanup_existing_requests treats delete_namespaced_custom_object returning as completed deletion, replacing kubectl delete --wait=true --timeout=60s with an asynchronous API call followed only by a fixed five-second sleep even though internal-services retains the InternalRequest while its finalizer cancels the associated PipelineRun. When finalizer or PipelineRun reconciliation takes longer than five seconds, create submits the replacement while the prior request is still terminating and driving operator work, potentially before its PipelineRun has been patched to Cancelled.
Agent Prompt
## Issue description
Restore synchronous cleanup semantics after deleting an InternalRequest. The Kubernetes deletion API only requests deletion and can return while the resource remains terminating and the internal-services finalizer has not yet cancelled the associated PipelineRun, so cleanup must not allow a replacement request to be created at that point.
## Issue Context
The previous `kubectl delete --wait=true --timeout=60s` invocation waited for resource deletion, whereas the new `delete_namespaced_custom_object` call is followed only by a fixed five-second propagation delay. Poll each deleted InternalRequest until `get_namespaced_custom_object` returns a not-found response, or use an equivalent API-based wait, with a bounded timeout comparable to the former 60-second timeout. Retain the existing propagation delay only after deletion has completed, and ensure the subsequent create path cannot run while cleanup is still waiting. Update or extend the cleanup tests to cover asynchronous deletion completion and timeout behavior.
## Fix Focus Areas
- src/helpers/internal_request/internal_request.py[421-465]
- src/helpers/internal_request/internal_request.py[561-575]
- src/helpers/internal_request/tests/test_internal_request.py[107-128]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
- internal-request uses python library for k8s to replace kubectl - kubernetes dependency added to pyproject - tests updated accordingly Signed-off-by: mvaras <mvaras@redhat.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1016 +/- ##
==========================================
- Coverage 97.24% 95.80% -1.45%
==========================================
Files 205 257 +52
Lines 12815 13199 +384
==========================================
+ Hits 12462 12645 +183
- Misses 353 554 +201
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 63 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
4 participants
