feat(BA-2290): Add Privileged Storage Worker #5779
Conversation
github-actions
bot
added
size:XL
fregataa
force-pushed
the
feat/bgtask-vfolder-delete
branch
from
a7e1c41 to
0650b00
Compare
fregataa
force-pushed
the
feat/add-privileged-worker
branch
2 times, most recently
from
1e32ad6 to
dacdba5
Compare
fregataa
force-pushed
the
feat/bgtask-vfolder-delete
branch
from
0650b00 to
67e2fa4
Compare
fregataa
force-pushed
the
feat/add-privileged-worker
branch
from
dacdba5 to
0dc54fb
Compare
fregataa
force-pushed
the
feat/add-privileged-worker
branch
from
0dc54fb to
ff4a5cc
Compare
fregataa
force-pushed
the
feat/add-privileged-worker
branch
from
ff4a5cc to
b35b5b3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Privileged Storage Worker component that runs with elevated privileges to handle sensitive storage operations. The implementation includes comprehensive bootstrap stages, configuration management, and integration with the existing storage system.
- Add complete privileged storage worker infrastructure with configurable root privilege validation
- Implement bootstrap stages for logger, monitor, etcd, Redis, event handling, and background task management
- Integrate privileged worker into the storage CLI with a new entry point
Reviewed Changes
Copilot reviewed 20 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/storage/privileged/server.py | Main server entry point with multiprocess worker management |
| src/ai/backend/storage/privileged/config.py | Configuration validation with root privilege checking |
| src/ai/backend/storage/privileged/bootstrap/ | Bootstrap stages for component initialization |
| src/ai/backend/storage/bgtask/tags.py | Renamed constant for privileged worker tagging |
| src/ai/backend/storage/BUILD | Added CLI entry point for privileged worker |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| log_config = logging.getLogger("ai.backend.storage.config") | ||
| if local_config.debug.enabled: | ||
| log_config.debug("debug mode enabled.") | ||
| if local_config.debug.enabled: |
There was a problem hiding this comment.
The debug condition is checked twice on lines 128 and 130. Consider combining these checks into a single block to reduce duplication.
| if local_config.debug.enabled: |
|
|
||
| @override | ||
| async def teardown(self, resource: LoggerResult) -> None: | ||
| resource.logger.__exit__() |
There was a problem hiding this comment.
The logger teardown calls __exit__() without the required arguments. Context managers require three parameters (exc_type, exc_val, exc_tb). Use resource.logger.__exit__(None, None, None) or consider using a more explicit teardown method.
| resource.logger.__exit__() | |
| resource.logger.__exit__(None, None, None) |
| @property | ||
| @override | ||
| def name(self) -> str: | ||
| return "storage-worker-redis-config" |
There was a problem hiding this comment.
The provisioner name is incorrect. It should be 'storage-worker-message-queue' to match the actual functionality of this class.
| return "storage-worker-redis-config" | |
| return "storage-worker-message-queue" |
| def create(self) -> BackgroundTaskHandlerRegistry: | ||
| registry = BackgroundTaskHandlerRegistry() | ||
| registry.register(VFolderDeleteTaskHandler(self._volume_pool, self._event_producer)) | ||
|
|
||
| return registry |
There was a problem hiding this comment.
The method creates a registry and registers a handler but doesn't return it on line 16. The return statement should come after line 15.
resolves #5776 (BA-2290)
Checklist: (if applicable)
ai.backend.testdocsdirectory