refactor(dynamic mount): introduce dynamic server manager

[imeoer]

We found that when a pod container is recreated, there's a chance the dynamically mounted CSI directory gets unmounted. Since this directory is shared among multiple pod containers via mount propagation from /var/run/model-csi (a tmpfs), it introduces instability and security isolation risks, for example, if one pod learns another pod's volume name, it could maliciously call APIs to mount or unmount that submount of volume.

This pull request refactors the management of dynamic HTTP servers for CSI mounts, improving security and stability by introducing the DynamicServerManager and deprecating the previous shared dynamic CSI endpoint approach. It also restructures the codebase for better maintainability by moving HTTP server logic from pkg/server/http.go to new service-layer files and updating handlers accordingly. Additionally, it enhances resource cleanup and recovery logic for dynamic servers, and updates mount/unmount workflows to leverage the new manager.

Dynamic Server Management and API Refactor:

• Introduced DynamicServerManager in pkg/service/dynamic_server.go to handle lifecycle (creation, closing, recovery) of per-volume dynamic HTTP servers, replacing the old shared socket approach. This improves isolation and avoids mount propagation issues.
• Deprecated the use of a single dynamic_csi_endpoint in favor of individual csi.sock sockets managed per dynamic mount directory, as documented in pkg/config/config.go.
• Removed the old HTTP server implementation from pkg/server/http.go and replaced it with dynamic server logic in the service layer. [1] [2]

Handler and API Changes:

• Renamed and refactored HTTP handler code from HttpHandler in pkg/server/http_handler.go to DynamicServerHandler in pkg/service/dynamic_server_handler.go, updating method names and request types for clarity and consistency. [1] [2] [3] [4] [5] [6] [7]

Mount/Unmount Workflow Updates:

• Updated dynamic volume mount/unmount logic in pkg/service/node_dynamic.go to use DynamicServerManager for server lifecycle management, and added device checks to determine the correct cleanup path for backward compatibility. [1] [2]

Resource Management and Utilities:

• Moved socket file existence checks and cleanup logic to a shared utility (EnsureSockNotExists), replacing the previous inline implementation for better reuse. [1] [2]
• Added utility function for generating dynamic CSI socket paths in pkg/config/config.go.

Test and Import Cleanup:

• Removed obsolete HTTP server and handler tests from pkg/server/server_test.go and cleaned up unused imports related to the old HTTP server code. [1] [2]

This refactor lays the groundwork for more robust and secure dynamic mount management, while maintaining backward compatibility for legacy setups.

[imeoer]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request introduces a significant and well-motivated refactoring to improve the management of dynamic CSI mounts. The introduction of DynamicServerManager to handle per-volume HTTP servers is a great step towards better security and stability. The code is well-structured, and backward compatibility for the old shared socket approach is thoughtfully maintained. My review includes a few suggestions to enhance robustness, particularly around context propagation for graceful shutdowns, ensuring proper logging severity for error conditions, and maintaining test coverage for the refactored code.

[gemini-code-assist]

Code Review

This pull request is a significant refactoring to improve the management of dynamic CSI mounts. It introduces a DynamicServerManager to create per-volume HTTP servers, which enhances security and stability by isolating volumes from each other and resolving issues with mount propagation. The changes are well-structured, moving logic into a new service layer and cleaning up the old implementation.

My review focuses on a few critical areas:

• Concurrency safety: I've identified a critical issue with the use of os.Chdir in a concurrent environment, which can lead to race conditions.
• Context propagation: There are places where context.Background() is used instead of propagating the request-scoped context, which can affect graceful shutdown.
• Error handling: I've pointed out a couple of spots where error handling could be more robust to prevent unexpected behavior.

Overall, this is a great improvement. Addressing these points will make the implementation even more robust.

[aftersnow]

lgtm

[chlins]

lgtm