Skip to content

Commit 74c4f3b

Browse files
committed
Move NRI cleanup from StopContainer to StateChange REMOVE_CONTAINER event
- Minimize event subscription to CREATE_CONTAINER and REMOVE_CONTAINER with correct 1-indexed bit positions (bit = event_value - 1, not 1 << event) - Move hardlink farm volume cleanup from unused StopContainer handler to StateChange listening for REMOVE_CONTAINER events - Keep StopContainer as skeleton logger handler for type checking (matches pattern of other unused handlers like UpdatePodSandbox) - Cleanup happens after container namespace is destroyed, when it's safe Verified working: CreateContainer injection works with minimal subscription. Previous bug was in bit position calculation - NRI spec uses 1-indexed positions, not direct event values as shifts.
1 parent a4c2927 commit 74c4f3b

1 file changed

Lines changed: 28 additions & 12 deletions

File tree

pkgs/nixkube/src/nri/server.py

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,26 @@
4141
from .mount import mount_in_container
4242
from .zmq import ZeroMQServer
4343

44-
# Subscribe to all valid NRI events (containerd may not send CreateContainer
45-
# unless we also subscribe to pod events and other related event types)
46-
_SUBSCRIBED_EVENTS = (1 << (nri_pb2.Event.Value("LAST") - 1)) - 1
44+
# Subscribe only to events we actually need for store injection and cleanup.
45+
#
46+
# NRI Event Subscription Bitmask Encoding:
47+
# Event enum values use 1-based indexing for bit positions: bit = (event_value - 1)
48+
# - Event 0 (UNKNOWN) → bit -1 (invalid, not used)
49+
# - Event 1 (RUN_POD_SANDBOX) → bit 0
50+
# - Event 4 (CREATE_CONTAINER) → bit 3: (1 << 3) = 8
51+
# - Event 11 (REMOVE_CONTAINER) → bit 10: (1 << 10) = 1024
52+
# - Event 15 (LAST, sentinel) → bit 14
53+
#
54+
# Previous Bug: Used (1 << (LAST - 1)) - 1 which set bits 0-13 (missing bit 14)
55+
# The fix: Calculate each event's bit position as (event_value - 1) before shifting
56+
#
57+
_SUBSCRIBED_EVENTS = sum(
58+
1 << (event - 1)
59+
for event in [
60+
nri_pb2.Event.CREATE_CONTAINER, # Inject stores into containers
61+
nri_pb2.Event.REMOVE_CONTAINER, # Cleanup hardlink farm volumes
62+
]
63+
)
4764

4865

4966
class NriPlugin(nri_grpc.PluginBase):
@@ -245,15 +262,7 @@ async def StopContainer(self, stream) -> None:
245262
req: nri_pb2.StopContainerRequest | None = await stream.recv_message()
246263
assert req is not None
247264
logger.info(f"container={req.container.name!r}")
248-
249-
container_id = req.container.id
250-
251-
# Phase 1: Cleanup volume directory for this container
252-
await cleanup_container_volume(container_id)
253-
254-
# Phase 2: Garbage collect stale volumes from containers no longer in CRI
255-
await garbage_collect_stale_volumes(self.cri_socket)
256-
265+
# Cleanup is handled in StateChange on REMOVE_CONTAINER event
257266
await stream.send_message(nri_pb2.StopContainerResponse())
258267

259268
async def UpdatePodSandbox(self, stream) -> None:
@@ -283,6 +292,13 @@ async def StateChange(self, stream) -> None:
283292
parts.append(container_info)
284293

285294
logger.info(" ".join(parts))
295+
296+
# Cleanup hardlink farm volumes when container is removed
297+
if event.event == nri_pb2.Event.REMOVE_CONTAINER:
298+
container_id = event.container.id
299+
await cleanup_container_volume(container_id)
300+
await garbage_collect_stale_volumes(self.cri_socket)
301+
286302
await stream.send_message(nri_pb2.Empty())
287303

288304
async def ValidateContainerAdjustment(self, stream) -> None:

0 commit comments

Comments
 (0)