Skip to content

fix: add support for Strimzi v1.0+ api#534

Open
mzglinski wants to merge 2 commits into
robusta-dev:mainfrom
mzglinski:fix/strimzi-v1-support
Open

fix: add support for Strimzi v1.0+ api#534
mzglinski wants to merge 2 commits into
robusta-dev:mainfrom
mzglinski:fix/strimzi-v1-support

Conversation

@mzglinski

Copy link
Copy Markdown

KRR’s StrimziPodSet support was hardcoded to core.strimzi.io/v1beta2. That worked when the feature landed, but Strimzi 1.0 dropped v1beta2.

This probes v1 first, falls back to v1beta2 for older operators, and caches whichever version works for the rest of the scan.

@CLAassistant

CLAassistant commented Jul 1, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b36d4ec0-2d10-4610-97b1-a8303cdb7416

📥 Commits

Reviewing files that changed from the base of the PR and between 12f4956 and fb86f0d.

📒 Files selected for processing (2)
  • robusta_krr/core/integrations/kubernetes/__init__.py
  • tests/test_strimzipodset_api_version.py

Walkthrough

ClusterLoader now probes and caches the StrimziPodSet API version before listing custom resources, using v1 when available and falling back to v1beta2 when needed. The listing path uses the detected version, and tests cover resolution, fallback, namespace probing, and retry behavior.

Changes

StrimziPodSet API version detection

Layer / File(s) Summary
Cached state and version resolver
robusta_krr/core/integrations/kubernetes/__init__.py
Adds cached API-version fields to ClusterLoader and a resolver that probes v1 then v1beta2 custom-object listings across cluster-wide or namespaced paths.
StrimziPodSet listing wiring
robusta_krr/core/integrations/kubernetes/__init__.py
Updates _list_strimzipodsets() to use the resolved version for listing requests and return an empty result when no version is detected.
Resolver success and fallback tests
tests/test_strimzipodset_api_version.py
Adds fixtures and tests covering v1 success, 404 fallback to v1beta2, unresolved availability, and cached repeat calls.
Namespace and retry tests
tests/test_strimzipodset_api_version.py
Adds tests covering namespaced probing, non-404 fallback behavior, transient error retry semantics, and multi-namespace probing until success.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main change: adding support for Strimzi v1.0+ APIs.
Description check ✅ Passed The description accurately matches the change by explaining the v1 probe, v1beta2 fallback, and caching behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mzglinski mzglinski force-pushed the fix/strimzi-v1-support branch from 2af9ffd to 12f4956 Compare July 1, 2026 12:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
tests/test_strimzipodset_api_version.py (1)

27-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Good coverage of the core fallback logic; consider adding a caching/memoization test.

The three tests cover v1 success, v1→v1beta2 fallback, and both-unavailable→None well. Missing: a test asserting that calling _resolve_strimzipodset_api_version() twice on the same loader only performs the underlying API calls once (verifying _strimzipodset_api_version_checked memoization actually takes effect), and a test for the namespaced (config.namespaces = ["ns1"]) code path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_strimzipodset_api_version.py` around lines 27 - 74, Add tests
around loader._resolve_strimzipodset_api_version() to cover the memoized cache
and the namespaced path. Specifically, verify that calling the method twice on
the same loader only triggers the underlying custom_objects
list_cluster_custom_object calls once by asserting
_strimzipodset_api_version_checked prevents повторных API requests, and add a
case with config.namespaces set (for example a single namespace) that exercises
the list_namespaced_custom_object branch instead of the cluster-scoped call.
robusta_krr/core/integrations/kubernetes/__init__.py (1)

492-501: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Minor: probe only checks the first configured namespace; synthetic exception is a bit indirect.

Only self.namespaces[0] is probed when namespaces are explicitly configured — if that particular namespace is inaccessible via RBAC while others aren't, version detection could fail unnecessarily. Also, raising a synthetic ApiException(status=404, ...) just to reuse the continue branch (line 493) works but reads a bit indirectly; an explicit continue with its own log line would be clearer.

Low priority given this is a narrow edge case with a safe fallback (mark kind unavailable).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@robusta_krr/core/integrations/kubernetes/__init__.py` around lines 492 - 501,
The version probe in the Kubernetes integration only checks self.namespaces[0]
and uses a synthetic ApiException to flow into the fallback path, which is too
indirect. Update the probing logic in the relevant namespace loop so it iterates
through all configured namespaces until one succeeds, and on the no-namespaces
case use an explicit continue/fallback path instead of raising a fake exception.
Keep the behavior in the Kubernetes custom object probe code (the
list_namespaced_custom_object call) the same otherwise, and add a clear log
message when a namespace is skipped or unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@robusta_krr/core/integrations/kubernetes/__init__.py`:
- Around line 476-515: In _resolve_strimzipodset_api_version, non-404 probe
failures are being cached as None or can escape and fail the scan; update the
probing logic for the StrimziPodSet API versions so transient non-404
ApiException cases still try the next version instead of breaking, and broaden
handling to catch client/network exceptions raised by the Kubernetes client.
Keep the discovery behavior aligned with __list_hpa / _try_list_hpa so only
missing resources are treated as unavailable while other errors are logged and
safely degraded without poisoning _strimzipodset_api_version_checked for the
rest of the run.
- Around line 476-527: The StrimziPodSet API probe in
_resolve_strimzipodset_api_version() is being executed synchronously on the
event loop via _list_strimzipodsets() and list_scannable_objects(), which can
block other work. Make _list_strimzipodsets() async and run the custom_objects
list_cluster_custom_object/list_namespaced_custom_object probe inside the
executor, following the same pattern used by _list_namespaced_or_global_objects,
_list_jobs_for_cronjobs, and list_pods. Keep
_resolve_strimzipodset_api_version() synchronous so callers and tests that
invoke it directly still work.

---

Nitpick comments:
In `@robusta_krr/core/integrations/kubernetes/__init__.py`:
- Around line 492-501: The version probe in the Kubernetes integration only
checks self.namespaces[0] and uses a synthetic ApiException to flow into the
fallback path, which is too indirect. Update the probing logic in the relevant
namespace loop so it iterates through all configured namespaces until one
succeeds, and on the no-namespaces case use an explicit continue/fallback path
instead of raising a fake exception. Keep the behavior in the Kubernetes custom
object probe code (the list_namespaced_custom_object call) the same otherwise,
and add a clear log message when a namespace is skipped or unavailable.

In `@tests/test_strimzipodset_api_version.py`:
- Around line 27-74: Add tests around
loader._resolve_strimzipodset_api_version() to cover the memoized cache and the
namespaced path. Specifically, verify that calling the method twice on the same
loader only triggers the underlying custom_objects list_cluster_custom_object
calls once by asserting _strimzipodset_api_version_checked prevents повторных
API requests, and add a case with config.namespaces set (for example a single
namespace) that exercises the list_namespaced_custom_object branch instead of
the cluster-scoped call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7513a6de-80dc-47e1-a9f7-70a7fae7a98a

📥 Commits

Reviewing files that changed from the base of the PR and between 8ca245f and 2af9ffd.

📒 Files selected for processing (2)
  • robusta_krr/core/integrations/kubernetes/__init__.py
  • tests/test_strimzipodset_api_version.py

Comment thread robusta_krr/core/integrations/kubernetes/__init__.py
Comment thread robusta_krr/core/integrations/kubernetes/__init__.py Outdated
@mzglinski mzglinski force-pushed the fix/strimzi-v1-support branch from 12f4956 to fb86f0d Compare July 1, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants