Right-size native sidecar containers (+ bump kubernetes client to 29)

README.md

@@ -483,6 +483,10 @@ By default, we use a _simple_ strategy to calculate resource recommendations. It
 
 - For memory, we take the maximum value over the past week and add a 15% buffer.
 
+### Native sidecars
+
+KRR right-sizes [native sidecar containers](https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/) — init containers with `restartPolicy: Always` (GA in Kubernetes 1.29) — alongside regular containers, since they run for the whole pod lifetime and reserve real requests. They appear as ordinary container rows in the report. One-shot init containers are intentionally **not** right-sized: they run only briefly at startup, so their usage is not representative of steady state. (Reading the container-level `restartPolicy` requires the `kubernetes` client `>= 28.1.0`.)
+
 ### Prometheus connection
 
 Find about how KRR tries to find the default Prometheus to connect <a href="#prometheus-victoria-metrics-and-thanos-auto-discovery">here</a>.

pyproject.toml

@@ -26,7 +26,7 @@ krr = "robusta_krr.main:run"
 python = ">=3.10,<=3.12.9"
 typer = { extras = ["all"], version = "^0.7.0" }
 pydantic = "^1.10.7"
-kubernetes = "^26.1.0"
+kubernetes = "^29.0.0"
 prometheus-api-client = "0.5.3"
 numpy = ">=1.26.4,<1.27.0"
 alive-progress = "^3.1.2"

requirements.txt

@@ -18,7 +18,7 @@ httmock==1.4.0 ; python_version >= "3.10" and python_full_version < "3.13"
 idna==3.7 ; python_version >= "3.10" and python_full_version < "3.13"
 jmespath==1.0.1 ; python_version >= "3.10" and python_full_version < "3.13"
 kiwisolver==1.4.5 ; python_version >= "3.10" and python_full_version < "3.13"
-kubernetes==26.1.0 ; python_version >= "3.10" and python_full_version < "3.13"
+kubernetes==29.0.0 ; python_version >= "3.10" and python_full_version < "3.13"
 matplotlib==3.8.3 ; python_version >= "3.10" and python_full_version < "3.13"
 numpy==1.26.4 ; python_version >= "3.10" and python_full_version < "3.13"
 oauthlib==3.2.2 ; python_version >= "3.10" and python_full_version < "3.13"

robusta_krr/core/integrations/kubernetes/__init__.py

@@ -40,6 +40,39 @@ def __init__(self, name: str, namespace: str):
 HPAKey = tuple[str, str, str]
 
 
+def _native_sidecars(pod_spec: Any) -> list[Any]:
+    """Return init containers that are native sidecars (``restartPolicy: Always``).
+
+    Native sidecars (GA in Kubernetes 1.29) run for the whole pod lifetime and
+    reserve real resource requests, so they are right-sized like normal containers.
+    One-shot init containers are intentionally excluded: their startup-only usage is
+    not representative of steady-state consumption.
+
+    Handles both typed clients (snake_case ``init_containers`` / ``restart_policy``)
+    and custom-resource workloads wrapped in :class:`ObjectLikeDict` (camelCase
+    ``initContainers`` / ``restartPolicy``). ``getattr(..., None)`` means an older
+    kubernetes client that lacks the container-level ``restart_policy`` field simply
+    yields nothing, so the feature degrades to a no-op instead of raising.
+    """
+    if pod_spec is None:
+        return []
+    init_containers = getattr(pod_spec, "init_containers", None) or getattr(pod_spec, "initContainers", None) or []
+    return [
+        container
+        for container in init_containers
+        if (getattr(container, "restart_policy", None) or getattr(container, "restartPolicy", None)) == "Always"
+    ]
+
+
+def _scannable_containers(pod_spec: Any) -> list[Any]:
+    """Regular containers plus native sidecars for a pod spec.
+
+    Single point where the two container classes KRR right-sizes are combined,
+    so every workload discovery path stays consistent.
+    """
+    return list(pod_spec.containers) + _native_sidecars(pod_spec)
+
+
 class ClusterLoader:
     def __init__(self, cluster: Optional[str] = None):
         self.cluster = cluster
@@ -421,13 +454,13 @@ def _list_deployments(self) -> list[K8sObjectData]:
             kind="Deployment",
             all_namespaces_request=self.apps.list_deployment_for_all_namespaces,
             namespaced_request=self.apps.list_namespaced_deployment,
-            extract_containers=lambda item: item.spec.template.spec.containers,
+            extract_containers=lambda item: _scannable_containers(item.spec.template.spec),
         )
 
     def _list_rollouts(self) -> list[K8sObjectData]:
         async def _extract_containers(item: Any) -> list[V1Container]:
             if item.spec.template is not None:
-                return item.spec.template.spec.containers
+                return _scannable_containers(item.spec.template.spec)
 
             loop = asyncio.get_running_loop()
 
@@ -444,7 +477,7 @@ async def _extract_containers(item: Any) -> list[V1Container]:
                         namespace=item.metadata.namespace, name=workloadRef.name
                     ),
                 )
-                return ret.spec.template.spec.containers
+                return _scannable_containers(ret.spec.template.spec)
 
             return []
 
@@ -492,7 +525,7 @@ def _list_strimzipodsets(self) -> list[K8sObjectData]:
                     **kwargs,
                 )
             ),
-            extract_containers=lambda item: item.spec.pods[0].spec.containers,
+            extract_containers=lambda item: _scannable_containers(item.spec.pods[0].spec),
         )
 
     def _list_deploymentconfig(self) -> list[K8sObjectData]:
@@ -516,23 +549,23 @@ def _list_deploymentconfig(self) -> list[K8sObjectData]:
                     **kwargs,
                 )
             ),
-            extract_containers=lambda item: item.spec.template.spec.containers,
+            extract_containers=lambda item: _scannable_containers(item.spec.template.spec),
         )
 
     def _list_all_statefulsets(self) -> list[K8sObjectData]:
         return self._list_scannable_objects(
             kind="StatefulSet",
             all_namespaces_request=self.apps.list_stateful_set_for_all_namespaces,
             namespaced_request=self.apps.list_namespaced_stateful_set,
-            extract_containers=lambda item: item.spec.template.spec.containers,
+            extract_containers=lambda item: _scannable_containers(item.spec.template.spec),
         )
 
     def _list_all_daemon_set(self) -> list[K8sObjectData]:
         return self._list_scannable_objects(
             kind="DaemonSet",
             all_namespaces_request=self.apps.list_daemon_set_for_all_namespaces,
             namespaced_request=self.apps.list_namespaced_daemon_set,
-            extract_containers=lambda item: item.spec.template.spec.containers,
+            extract_containers=lambda item: _scannable_containers(item.spec.template.spec),
         )
 
     async def _list_all_jobs(self) -> list[K8sObjectData]:
@@ -571,7 +604,7 @@ async def _list_all_jobs(self) -> list[K8sObjectData]:
                             continue
                         if self._is_job_grouped(job):
                             continue
-                        for container in job.spec.template.spec.containers:
+                        for container in _scannable_containers(job.spec.template.spec):
                             all_jobs.append(self.__build_scannable_object(job, container, "Job"))
                     if not continue_ref:
                         break
@@ -591,7 +624,7 @@ def _list_all_cronjobs(self) -> list[K8sObjectData]:
             kind="CronJob",
             all_namespaces_request=self.batch.list_cron_job_for_all_namespaces,
             namespaced_request=self.batch.list_namespaced_cron_job,
-            extract_containers=lambda item: item.spec.job_template.spec.template.spec.containers,
+            extract_containers=lambda item: _scannable_containers(item.spec.job_template.spec.template.spec),
         )
 
     async def _list_all_groupedjobs(self) -> list[K8sObjectData]:
@@ -678,13 +711,14 @@ async def _list_all_groupedjobs(self) -> list[K8sObjectData]:
             for namespace, namespace_jobs in jobs_by_namespace.items():
                 limited_jobs = namespace_jobs[: settings.job_grouping_limit]
 
+                template_containers = _scannable_containers(template_job.spec.template.spec)
                 container_names = set()
-                for container in template_job.spec.template.spec.containers:
+                for container in template_containers:
                     container_names.add(container.name)
 
                 for container_name in container_names:
                     template_container = None
-                    for container in template_job.spec.template.spec.containers:
+                    for container in template_containers:
                         if container.name == container_name:
                             template_container = container
                             break

tests/test_native_sidecars.py

@@ -0,0 +1,193 @@
+"""Tests for native-sidecar discovery.
+
+KRR right-sizes native sidecars (init containers with ``restartPolicy: Always``,
+GA in Kubernetes 1.29) like normal containers, while intentionally ignoring
+one-shot init containers. The core logic lives in the module-level helper
+``_native_sidecars``; these tests cover it directly (it is pure) plus two
+discovery paths that use it (the Deployment extractor lambda and the Job loop).
+
+Typed kubernetes-client models are simulated with ``SimpleNamespace`` rather than
+``MagicMock`` on purpose: ``MagicMock`` auto-creates any attribute, so
+``getattr(mock, "initContainers", None)`` would return a truthy mock and defeat
+the snake_case/camelCase fallback under test. CRD workloads are simulated with a
+real ``ObjectLikeDict`` (raw camelCase keys), exactly as the custom-objects API
+returns them.
+"""
+
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from robusta_krr.core.integrations.kubernetes import ClusterLoader, _native_sidecars
+from robusta_krr.core.models.config import Config
+from robusta_krr.utils.object_like_dict import ObjectLikeDict
+
+
+def typed_container(name: str, **kwargs) -> SimpleNamespace:
+    """A typed V1Container-like object. Pass restart_policy=... to set it;
+    omit it entirely to simulate an old client where the field does not exist."""
+    return SimpleNamespace(name=name, **kwargs)
+
+
+def typed_spec(containers, init_containers=None) -> SimpleNamespace:
+    """A typed V1PodSpec-like object (snake_case attributes)."""
+    return SimpleNamespace(containers=containers, init_containers=init_containers)
+
+
+# --- pure helper tests -------------------------------------------------------
+
+
+def test_native_sidecar_discovered():
+    spec = typed_spec(
+        containers=[typed_container("main")],
+        init_containers=[typed_container("proxy", restart_policy="Always")],
+    )
+    sidecars = _native_sidecars(spec)
+    assert [c.name for c in sidecars] == ["proxy"]
+
+
+def test_oneshot_init_container_excluded():
+    # A one-shot init container has restart_policy unset (None) -> not a sidecar.
+    spec = typed_spec(
+        containers=[typed_container("main")],
+        init_containers=[typed_container("db-migrate", restart_policy=None)],
+    )
+    assert _native_sidecars(spec) == []
+
+
+def test_no_init_containers():
+    spec = typed_spec(containers=[typed_container("main")], init_containers=None)
+    assert _native_sidecars(spec) == []
+
+
+def test_none_pod_spec():
+    # Guards the Rollout workloadRef branch and any spec-less edge.
+    assert _native_sidecars(None) == []
+
+
+def test_old_client_no_restart_policy_field():
+    # On kubernetes < 28.1.0 the container model has no restart_policy attribute
+    # at all: the helper must degrade to a no-op, not crash.
+    spec = typed_spec(
+        containers=[typed_container("main")],
+        init_containers=[typed_container("maybe-sidecar")],  # no restart_policy attr
+    )
+    assert _native_sidecars(spec) == []
+
+
+def test_crd_camelcase_sidecar():
+    # Custom-objects API (Rollout/StrimziPodSet/DeploymentConfig) returns raw
+    # camelCase keys wrapped in ObjectLikeDict.
+    spec = ObjectLikeDict(
+        {
+            "containers": [{"name": "main"}],
+            "initContainers": [
+                {"name": "proxy", "restartPolicy": "Always"},
+                {"name": "warmup", "restartPolicy": None},
+            ],
+        }
+    )
+    sidecars = _native_sidecars(spec)
+    assert [c.name for c in sidecars] == ["proxy"]
+
+
+def test_mixed_init_containers():
+    spec = typed_spec(
+        containers=[typed_container("main")],
+        init_containers=[
+            typed_container("db-migrate", restart_policy=None),
+            typed_container("proxy", restart_policy="Always"),
+            typed_container("logshipper", restart_policy="Always"),
+        ],
+    )
+    assert [c.name for c in _native_sidecars(spec)] == ["proxy", "logshipper"]
+
+
+# --- integration tests through real discovery paths --------------------------
+
+
+@pytest.fixture
+def mock_config():
+    config = MagicMock(spec=Config)
+    config.resources = "*"
+    config.selector = None
+    config.namespaces = "*"
+    config.max_workers = 4
+    config.get_kube_client = MagicMock()
+    # Job discovery settings
+    config.discovery_job_batch_size = 1000
+    config.discovery_job_max_batches = 50
+    return config
+
+
+@pytest.fixture
+def loader(mock_config):
+    with patch("robusta_krr.core.integrations.kubernetes.settings", mock_config):
+        loader = ClusterLoader()
+        loader.apps = MagicMock()
+        loader.batch = MagicMock()
+        loader.core = MagicMock()
+
+        # __build_scannable_object is name-mangled; stub it to expose container name.
+        def build(item, container, kind):
+            obj = MagicMock()
+            obj.container = container.name
+            obj.kind = kind
+            return obj
+
+        loader._ClusterLoader__build_scannable_object = build  # type: ignore
+        return loader
+
+
+@pytest.mark.asyncio
+async def test_deployment_extractor_includes_sidecar(loader, mock_config):
+    """The real Deployment extractor lambda + _list_scannable_objects yields the
+    main container and the native sidecar, but not the one-shot init container."""
+    deployment = SimpleNamespace(
+        spec=SimpleNamespace(
+            template=SimpleNamespace(
+                spec=typed_spec(
+                    containers=[typed_container("web")],
+                    init_containers=[
+                        typed_container("php-fpm", restart_policy="Always"),
+                        typed_container("db-migrate", restart_policy=None),
+                    ],
+                )
+            )
+        )
+    )
+    loader._list_namespaced_or_global_objects = AsyncMock(return_value=[deployment])
+
+    with patch("robusta_krr.core.integrations.kubernetes.settings", mock_config):
+        result = await loader._list_deployments()
+
+    assert {obj.container for obj in result} == {"web", "php-fpm"}
+
+
+@pytest.mark.asyncio
+async def test_job_includes_native_sidecar(loader, mock_config):
+    """The Job loop (which bypasses _list_scannable_objects) also picks up the
+    native sidecar and skips the one-shot init container."""
+    job = MagicMock()
+    job.metadata.owner_references = []
+    job.metadata.labels = {}
+    job.spec.template.spec = typed_spec(
+        containers=[typed_container("batch")],
+        init_containers=[
+            typed_container("metrics", restart_policy="Always"),
+            typed_container("seed", restart_policy=None),
+        ],
+    )
+
+    async def batched(*args, **kwargs):
+        return ([job], None)
+
+    loader._list_namespaced_or_global_objects_batched = batched
+    loader._is_job_owned_by_cronjob = MagicMock(return_value=False)
+    loader._is_job_grouped = MagicMock(return_value=False)
+
+    with patch("robusta_krr.core.integrations.kubernetes.settings", mock_config):
+        result = await loader._list_all_jobs()
+
+    assert {obj.container for obj in result} == {"batch", "metrics"}