Add initContainer support (#426) #491
Conversation
|
|
WalkthroughAdds support for reading Kubernetes pod init containers (both snake_case and camelCase) and includes them alongside regular containers in ClusterLoader extraction, grouping, and listing paths; introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/test_initcontainers.py (1)
65-65: Update type hints to use explicitOptionalor union syntax.The
init_containersparameter should use explicit optional typing per PEP 484.Apply this diff:
-def create_mock_deployment(name: str, namespace: str, containers: list, init_containers: list = None): +def create_mock_deployment(name: str, namespace: str, containers: list, init_containers: list | None = None):-def create_mock_job(name: str, namespace: str, containers: list, init_containers: list = None): +def create_mock_job(name: str, namespace: str, containers: list, init_containers: list | None = None):-def create_mock_cronjob(name: str, namespace: str, containers: list, init_containers: list = None): +def create_mock_cronjob(name: str, namespace: str, containers: list, init_containers: list | None = None):Also applies to: 80-80, 93-93
robusta_krr/core/integrations/kubernetes/__init__.py (1)
691-691: Minor cleanup opportunity: unused exception variable.The exception variable
eis assigned but never used sinceexc_info=Truealready logs the exception details.Apply this diff:
- except Exception as e: + except Exception: logger.error( "Failed to run grouped jobs discovery", exc_info=True, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
robusta_krr/core/integrations/kubernetes/__init__.py(25 hunks)tests/test_initcontainers.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_initcontainers.py (4)
robusta_krr/core/integrations/kubernetes/__init__.py (3)
ClusterLoader(60-829)namespaces(80-113)_list_namespaced_or_global_objects_batched(301-363)robusta_krr/core/models/config.py (2)
Config(20-211)get_kube_client(183-193)robusta_krr/utils/object_like_dict.py (1)
ObjectLikeDict(1-29)robusta_krr/core/models/objects.py (1)
selector(76-83)
robusta_krr/core/integrations/kubernetes/__init__.py (2)
robusta_krr/utils/object_like_dict.py (1)
items(28-29)robusta_krr/core/models/objects.py (2)
selector(76-83)K8sObjectData(38-107)
🪛 Ruff (0.14.8)
tests/test_initcontainers.py
65-65: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
80-80: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
93-93: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
229-229: Unused function argument: args
(ARG001)
229-229: Unused function argument: kwargs
(ARG001)
254-254: Unused function argument: args
(ARG001)
254-254: Unused function argument: kwargs
(ARG001)
282-282: Unused function argument: args
(ARG001)
282-282: Unused function argument: kwargs
(ARG001)
robusta_krr/core/integrations/kubernetes/__init__.py
234-234: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
238-238: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
348-348: Consider moving this statement to an else block
(TRY300)
607-607: Consider moving this statement to an else block
(TRY300)
691-691: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🔇 Additional comments (14)
tests/test_initcontainers.py (6)
1-14: LGTM!Clear module documentation and appropriate imports for a comprehensive test suite.
17-52: LGTM!Well-structured fixtures that properly mock the configuration and ClusterLoader dependencies needed for testing.
126-214: LGTM!Comprehensive test coverage for Deployment and CronJob init container extraction, including edge cases like None, empty lists, and multiple init containers.
217-296: LGTM!The async tests correctly verify Job and GroupedJob init container handling. The unused
argsandkwargsparameters in the mock functions (lines 229, 254, 282) are acceptable patterns for matching function signatures in tests.
299-390: LGTM!Excellent test coverage for the
_get_init_containershelper function, including precedence rules, empty vs None handling, and both naming conventions.
392-487: LGTM!Thorough validation of CustomObjectsApi workloads with camelCase
initContainerssupport. The tests correctly simulate real-world ObjectLikeDict behavior and verify all three custom resource types (Rollout, StrimziPodSet, DeploymentConfig).robusta_krr/core/integrations/kubernetes/__init__.py (8)
27-42: LGTM!Excellent helper function that elegantly handles the dual snake_case/camelCase naming convention discrepancy between the standard Kubernetes Python client and CustomObjectsApi. The precedence logic (snake_case first) is correct, and the docstring clearly explains the rationale.
441-442: LGTM!Consistent implementation of init container support for Deployment, StatefulSet, and DaemonSet workloads using the standard Python client pattern with proper None handling.
Also applies to: 550-551, 559-560
446-491: LGTM!Correct implementation of init container support for CustomObjectsApi workloads (Rollout, StrimziPodSet, DeploymentConfig) using the new helper function. The Rollout implementation properly handles both the template path (CustomObjectsApi with camelCase) and the workloadRef path (standard API with snake_case).
Also applies to: 514-516, 540-542
621-622: LGTM!Correct implementation for CronJob init containers, properly navigating the nested
job_template.spec.template.specstructure.
599-602: LGTM!Correct inline loop implementation for Jobs that processes both regular containers and init containers, with proper None handling.
709-725: LGTM!Comprehensive GroupedJob implementation that correctly handles init containers by:
- Collecting all container names (regular + init) from the template job
- Searching both regular and init containers when looking up the template container by name
This ensures init containers are properly represented in grouped job results.
177-190: LGTM!The GroupedJob pod listing correctly applies the
job_grouping_limit, and the selector building logic appropriately usesgetattrto handle optional camelCase attributes from CRDs (despite the static analysis warning, this usage is correct for attributes that may not exist on all objects).Also applies to: 230-239
350-363: LGTM!Proper error handling for expired continue tokens (HTTP 410), extracting the new token from the error response body and continuing the batched loading.
|
The docstring coverage warning is pre-existing (the main kubernetes/init.py has 27.5% coverage in the existing codebase). Our new code (_get_init_containers helper and all test functions) includes proper docstrings. Happy to add more if needed, |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/test_initcontainers.py (2)
217-266: LGTM! Job extraction tests validate full integration flow.The async tests properly mock the batched listing method and verify that both regular and init containers are extracted through the full
_list_all_jobs()flow.Optional: To satisfy the linter warnings about unused parameters in the mock async functions (lines 229 and 254), you could prefix them with underscores:
- async def mock_batched_method(*args, **kwargs): + async def mock_batched_method(*_args, **_kwargs):This preserves the signature while indicating the parameters are intentionally unused.
268-297: LGTM! GroupedJob extraction tests validate grouping with init containers.The test correctly validates that init containers are extracted and grouped separately when job grouping is enabled, resulting in distinct GroupedJob objects per container.
Optional: Same linter warning as above—consider prefixing unused parameters with underscores on line 282:
- async def mock_batched_method(*args, **kwargs): + async def mock_batched_method(*_args, **_kwargs):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_initcontainers.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_initcontainers.py (3)
robusta_krr/core/integrations/kubernetes/__init__.py (1)
_get_init_containers(27-41)robusta_krr/core/models/config.py (1)
get_kube_client(183-193)robusta_krr/utils/object_like_dict.py (1)
ObjectLikeDict(1-29)
🪛 Ruff (0.14.8)
tests/test_initcontainers.py
229-229: Unused function argument: args
(ARG001)
229-229: Unused function argument: kwargs
(ARG001)
254-254: Unused function argument: args
(ARG001)
254-254: Unused function argument: kwargs
(ARG001)
282-282: Unused function argument: args
(ARG001)
282-282: Unused function argument: kwargs
(ARG001)
🔇 Additional comments (7)
tests/test_initcontainers.py (7)
1-52: LGTM! Well-structured test fixtures.The imports, module documentation, and fixtures are well-organized. The
mock_cluster_loaderfixture appropriately mocks all necessary Kubernetes API clients and the executor for async operations.
55-123: LGTM! Excellent helper functions.The helper functions reduce duplication and make tests readable. The extract container helpers correctly mirror the lambda patterns used in the production code, properly handling
Nonevalues with theor []idiom.
126-186: LGTM! Comprehensive Deployment extraction tests.The tests thoroughly cover Deployment container extraction scenarios: with init containers, without (None), empty list, and multiple init containers. The assertions correctly verify both count and container names.
188-215: LGTM! CronJob extraction tests cover the job_template path.The tests correctly validate init container extraction through the CronJob-specific
job_templatepath, covering both presence and absence scenarios.
299-338: LGTM! Focused unit tests for extraction lambda patterns.The tests validate the container extraction logic for different workload types independently, covering both the standard pattern and specialized paths for CronJob and StrimziPodSet.
340-390: LGTM! Excellent tests for the dual-case helper function.The tests comprehensively validate
_get_init_containersbehavior across all scenarios. The test on lines 382-389 is particularly valuable—it ensures that an explicit empty list frominit_containers(snake_case) is returned without falling back toinitContainers(camelCase), which correctly matches theis not Nonecheck in the implementation.
392-487: LGTM! Thorough CustomObjectsApi camelCase tests.The tests validate init container extraction for CustomObjectsApi workloads (Rollout, StrimziPodSet, DeploymentConfig) using
ObjectLikeDictto simulate the raw JSON response with camelCase fields. The final test (lines 479-487) provides valuable documentation ofObjectLikeDictbehavior—returningNonefor missing attributes—which explains why the dual-case_get_init_containershelper is necessary.
2 participants
Adds support for analyzing initContainers alongside regular containers for all workload types.
Changes
_get_init_containers()helper to handle bothinit_containers(standard API) andinitContainers(CustomObjectsApi)Testing
Fixes #426